Merge pull request #1190 from LittleHuba/fix_audio

Fix crackling in audio playback

src/util/audio/AudioPlayer.cpp

@@ -5,8 +5,8 @@ AudioPlayer::AudioPlayer(Control* control, Settings* settings) : control(control
 {
 	XOJ_INIT_TYPE(AudioPlayer);
 
-	this->audioQueue = new AudioQueue<int>();
-	this->portAudioConsumer = new PortAudioConsumer(settings, this->audioQueue);
+	this->audioQueue = new AudioQueue<float>();
+	this->portAudioConsumer = new PortAudioConsumer(this, this->audioQueue);
 	this->vorbisProducer = new VorbisProducer(this->audioQueue);
 }
 
@@ -73,41 +73,28 @@ bool AudioPlayer::play()
 		return false;
 	}
 
-	bool status = this->portAudioConsumer->startPlaying();
+	return this->portAudioConsumer->startPlaying();
+}
 
-	if (status)
+void AudioPlayer::disableAudioPlaybackButtons()
+{
+	XOJ_CHECK_TYPE(AudioPlayer);
+
+	if (this->audioQueue->hasStreamEnded())
 	{
-		// Clean up after audio is played
-		stopThread = std::thread(
-				[&]
-				{
-					while (isPlaying())
-					{
-						Pa_Sleep(100);
-					}
-					this->portAudioConsumer->stopPlaying();
-
-					// If the stream is played completely update the UI elements accordingly
-					if (this->audioQueue->hasStreamEnded())
-					{
-						this->control->getWindow()->disableAudioPlaybackButtons();
-					}
-				});
-		stopThread.detach();
+		this->control->getWindow()->disableAudioPlaybackButtons();
 	}
-
-	return status;
 }
 
 void AudioPlayer::stop()
 {
 	XOJ_CHECK_TYPE(AudioPlayer);
 
-	this->audioQueue->signalEndOfStream();
-
 	// Stop playing audio
 	this->portAudioConsumer->stopPlaying();
 
+	this->audioQueue->signalEndOfStream();
+
 	// Abort libsox
 	this->vorbisProducer->abort();
 
@@ -123,3 +110,8 @@ vector<DeviceInfo> AudioPlayer::getOutputDevices()
 	return vector<DeviceInfo>{std::make_move_iterator(std::begin(deviceList)),
 							  std::make_move_iterator(std::end(deviceList))};
 }
+
+Settings* AudioPlayer::getSettings()
+{
+	return this->settings;
+}

src/util/audio/AudioPlayer.h

@@ -18,6 +18,7 @@
 #include "VorbisProducer.h"
 
 #include <control/settings/Settings.h>
+#include <control/Control.h>
 
 class AudioPlayer
 {
@@ -32,6 +33,8 @@ public:
 
 	vector<DeviceInfo> getOutputDevices();
 
+	Settings* getSettings();
+	void disableAudioPlaybackButtons();
 private:
 	XOJ_TYPE_ATTRIB;
 
@@ -39,7 +42,7 @@ protected:
 	Settings* settings = nullptr;
 	Control* control = nullptr;
 
-	AudioQueue<int>* audioQueue = nullptr;
+	AudioQueue<float>* audioQueue = nullptr;
 	PortAudioConsumer* portAudioConsumer = nullptr;
 	VorbisProducer* vorbisProducer = nullptr;
 	std::thread stopThread;

src/util/audio/AudioRecorder.cpp

@@ -7,7 +7,7 @@ AudioRecorder::AudioRecorder(Settings* settings)
 {
 	XOJ_INIT_TYPE(AudioRecorder);
 
-	this->audioQueue = new AudioQueue<int>();
+	this->audioQueue = new AudioQueue<float>();
 	this->portAudioProducer = new PortAudioProducer(settings, this->audioQueue);
 	this->vorbisConsumer = new VorbisConsumer(settings, this->audioQueue);
 }

src/util/audio/AudioRecorder.h

@@ -37,7 +37,7 @@ private:
 protected:
 	Settings* settings = nullptr;
 
-	AudioQueue<int>* audioQueue = nullptr;
+	AudioQueue<float>* audioQueue = nullptr;
 	PortAudioProducer* portAudioProducer = nullptr;
 	VorbisConsumer* vorbisConsumer = nullptr;
 };

src/util/audio/PortAudioConsumer.cpp

@@ -1,6 +1,7 @@
 #include "PortAudioConsumer.h"
+#include "AudioPlayer.h"
 
-PortAudioConsumer::PortAudioConsumer(Settings* settings, AudioQueue<int>* audioQueue) : sys(portaudio::System::instance()), settings(settings), audioQueue(audioQueue)
+PortAudioConsumer::PortAudioConsumer(AudioPlayer* audioPlayer, AudioQueue<float>* audioQueue) : sys(portaudio::System::instance()), audioPlayer(audioPlayer), audioQueue(audioQueue)
 {
 	XOJ_INIT_TYPE(PortAudioConsumer);
 }
@@ -28,7 +29,7 @@ std::list<DeviceInfo> PortAudioConsumer::getOutputDevices()
 
 		if (i->isFullDuplexDevice() || i->isOutputOnlyDevice())
 		{
-			DeviceInfo deviceInfo(&(*i), this->settings->getAudioOutputDevice() == i->index());
+			DeviceInfo deviceInfo(&(*i), this->audioPlayer->getSettings()->getAudioOutputDevice() == i->index());
 			deviceList.push_back(deviceInfo);
 		}
 
@@ -40,7 +41,7 @@ const DeviceInfo PortAudioConsumer::getSelectedOutputDevice()
 {
 	try
 	{
-		return DeviceInfo(&sys.deviceByIndex(this->settings->getAudioOutputDevice()), true);
+		return DeviceInfo(&sys.deviceByIndex(this->audioPlayer->getSettings()->getAudioOutputDevice()), true);
 	}
 	catch (portaudio::PaException& e)
 	{
@@ -60,10 +61,10 @@ bool PortAudioConsumer::startPlaying()
 {
 	XOJ_CHECK_TYPE(PortAudioConsumer);
 
-	// Check if there already is a recording
-	if (this->outputStream != nullptr)
+	// Abort a playback stream if one is currently active
+	if (this->outputStream != nullptr && this->outputStream->isActive())
 	{
-		return false;
+		this->outputStream->abort();
 	}
 
 	double sampleRate;
@@ -96,7 +97,7 @@ bool PortAudioConsumer::startPlaying()
 	}
 
 	this->outputChannels = channels;
-	portaudio::DirectionSpecificStreamParameters outParams(*device, channels, portaudio::INT32, true, device->defaultLowOutputLatency(), nullptr);
+	portaudio::DirectionSpecificStreamParameters outParams(*device, channels, portaudio::FLOAT32, true, device->defaultLowOutputLatency(), nullptr);
 	portaudio::StreamParameters params(portaudio::DirectionSpecificStreamParameters::null(), outParams, sampleRate, this->framesPerBuffer, paNoFlag);
 
 	try
@@ -106,7 +107,7 @@ bool PortAudioConsumer::startPlaying()
 	catch (portaudio::PaException& e)
 	{
 		this->audioQueue->signalEndOfStream();
-		g_warning("PortAudioConsumer: Unable to open stream to device");
+		g_warning("PortAudioConsumer: Unable to open stream to device\nCaused by: %s", e.what());
 		return false;
 	}
 	// Start the recording
@@ -117,7 +118,7 @@ bool PortAudioConsumer::startPlaying()
 	catch (portaudio::PaException& e)
 	{
 		this->audioQueue->signalEndOfStream();
-		g_warning("PortAudioConsumer: Unable to start stream");
+		g_warning("PortAudioConsumer: Unable to start stream\nCaused by: %s", e.what());
 		return false;
 	}
 	return true;
@@ -130,34 +131,60 @@ int PortAudioConsumer::playCallback(const void* inputBuffer, void* outputBuffer,
 
 	if (statusFlags)
 	{
-		g_warning("PortAudioConsumer: statusFlag: %s", std::to_string(statusFlags).c_str());
+		g_warning("PortAudioConsumer: PortAudio reported a stream warning: %s", std::to_string(statusFlags).c_str());
 	}
 
 	if (outputBuffer != nullptr)
 	{
 		unsigned long outputBufferLength;
-		this->audioQueue->pop(((int*) outputBuffer), outputBufferLength, framesPerBuffer * this->outputChannels);
+		this->audioQueue->pop(((float*) outputBuffer), outputBufferLength, framesPerBuffer * this->outputChannels);
 
 		// Fill buffer to requested length if necessary
 
 		if (outputBufferLength < framesPerBuffer * this->outputChannels)
 		{
-			g_warning("PortAudioConsumer: Frame underflow");
+			// Show frame underflow warning if there are not enough samples and the stream is not yet finished
+			if (!this->audioQueue->hasStreamEnded())
+			{
+				g_warning("PortAudioConsumer: Not enough audio samples available to fill requested frame");
+			}
 
-			auto outputBufferImpl = (int*) outputBuffer;
-			for (auto i = outputBufferLength; i < framesPerBuffer * this->outputChannels; ++i)
+			auto outputBufferImpl = (float*) outputBuffer;
+
+			if (outputBufferLength > this->outputChannels)
+			{
+				// If there is previous audio data use this data to ramp down the audio samples
+				for (auto i = outputBufferLength; i < framesPerBuffer * this->outputChannels; ++i)
+				{
+					outputBufferImpl[i] = outputBufferImpl[i - this->outputChannels] / 2;
+				}
+			}
+			else
 			{
-				outputBufferImpl[i] = 0;
+				// If there is no data that could be used to ramp down just output silence
+				for (auto i = outputBufferLength; i < framesPerBuffer * this->outputChannels; ++i)
+				{
+					outputBufferImpl[i] = 0;
+				}
 			}
 		}
 
-
-		if (!this->audioQueue->hasStreamEnded() || !this->audioQueue->empty())
+		// Continue playback if there is still data available
+		if (this->audioQueue->hasStreamEnded() && this->audioQueue->empty())
+		{
+			this->audioPlayer->disableAudioPlaybackButtons();
+			return paComplete;
+		}
+		else
 		{
 			return paContinue;
 		}
 	}
-	return paComplete;
+
+	// The output buffer is no longer available - Abort!
+	this->audioQueue->signalEndOfStream();
+	this->audioPlayer->disableAudioPlaybackButtons();
+	return paAbort;
 }
 
 void PortAudioConsumer::stopPlaying()
@@ -173,18 +200,13 @@ void PortAudioConsumer::stopPlaying()
 			{
 				this->outputStream->stop();
 			}
-			if (this->outputStream->isOpen())
-			{
-				this->outputStream->close();
-			}
 		}
 		catch (portaudio::PaException& e)
 		{
-			g_warning("PortAudioConsumer: Closing stream failed");
+			/*
+			 * We try closing the stream but this->outputStream might be an invalid object at this time if the stream was previously closed by the backend.
+			 * Just ignore this as the stream is closed either way.
+			 */
 		}
 	}
-
-	// Allow new playback by removing the old one
-	delete this->outputStream;
-	this->outputStream = nullptr;
 }

src/util/audio/PortAudioConsumer.h

@@ -22,10 +22,12 @@
 
 #include <list>
 
+class AudioPlayer;
+
 class PortAudioConsumer
 {
 public:
-	explicit PortAudioConsumer(Settings* settings, AudioQueue<int>* audioQueue);
+	explicit PortAudioConsumer(AudioPlayer* audioPlayer, AudioQueue<float>* audioQueue);
 	~PortAudioConsumer();
 
 public:
@@ -44,8 +46,8 @@ protected:
 
 	portaudio::AutoSystem autoSys;
 	portaudio::System& sys;
-	Settings* settings = nullptr;
-	AudioQueue<int>* audioQueue = nullptr;
+	AudioPlayer* audioPlayer;
+	AudioQueue<float>* audioQueue = nullptr;
 
 	int outputChannels = 0;
 

src/util/audio/PortAudioProducer.cpp

@@ -1,6 +1,6 @@
 #include "PortAudioProducer.h"
 
-PortAudioProducer::PortAudioProducer(Settings* settings, AudioQueue<int>* audioQueue)
+PortAudioProducer::PortAudioProducer(Settings* settings, AudioQueue<float>* audioQueue)
 		: sys(portaudio::System::instance()),
 		  settings(settings),
 		  audioQueue(audioQueue)
@@ -85,7 +85,7 @@ bool PortAudioProducer::startRecording()
 
 	// Restrict recording channels to 2 as playback devices should have 2 channels at least
 	this->inputChannels = std::min(2, device->maxInputChannels());
-	portaudio::DirectionSpecificStreamParameters inParams(*device, this->inputChannels, portaudio::INT32, true, device->defaultLowInputLatency(), nullptr);
+	portaudio::DirectionSpecificStreamParameters inParams(*device, this->inputChannels, portaudio::FLOAT32, true, device->defaultLowInputLatency(), nullptr);
 	portaudio::StreamParameters params(inParams, portaudio::DirectionSpecificStreamParameters::null(), this->settings->getAudioSampleRate(), this->framesPerBuffer, paNoFlag);
 
 	this->audioQueue->setAudioAttributes(this->settings->getAudioSampleRate(), static_cast<unsigned int>(this->inputChannels));
@@ -130,7 +130,7 @@ int PortAudioProducer::recordCallback(const void* inputBuffer, void* outputBuffe
 	{
 		unsigned long providedFrames = framesPerBuffer * this->inputChannels;
 
-		this->audioQueue->push((int*) inputBuffer, providedFrames);
+		this->audioQueue->push((float*) inputBuffer, providedFrames);
 	}
 	return paContinue;
 }

src/util/audio/PortAudioProducer.h

@@ -25,7 +25,7 @@
 class PortAudioProducer
 {
 public:
-	explicit PortAudioProducer(Settings* settings, AudioQueue<int>* audioQueue);
+	explicit PortAudioProducer(Settings* settings, AudioQueue<float>* audioQueue);
 	~PortAudioProducer();
 
 	std::list<DeviceInfo> getInputDevices();
@@ -49,7 +49,7 @@ protected:
 	portaudio::AutoSystem autoSys;
 	portaudio::System& sys;
 	Settings* settings;
-	AudioQueue<int>* audioQueue;
+	AudioQueue<float>* audioQueue;
 
 	int inputChannels = 0;
 

src/util/audio/VorbisConsumer.cpp

@@ -1,7 +1,7 @@
 #include <cmath>
 #include "VorbisConsumer.h"
 
-VorbisConsumer::VorbisConsumer(Settings* settings, AudioQueue<int>* audioQueue)
+VorbisConsumer::VorbisConsumer(Settings* settings, AudioQueue<float>* audioQueue)
 		: settings(settings),
 		  audioQueue(audioQueue)
 {
@@ -46,7 +46,7 @@ bool VorbisConsumer::start(string filename)
 			{
 				std::unique_lock<std::mutex> lock(audioQueue->syncMutex());
 
-				int buffer[64 * channels];
+				float buffer[64 * channels];
 				unsigned long bufferLength;
 				double audioGain = this->settings->getAudioGain();
 
@@ -63,25 +63,11 @@ bool VorbisConsumer::start(string filename)
 						{
 							for (unsigned int i = 0; i < 64 * channels; ++i)
 							{
-								// check for overflow
-								if (std::abs(buffer[i]) < std::floor(INT_MAX / audioGain))
-								{
-									buffer[i] = static_cast<int>(buffer[i] * audioGain);
-								} else
-								{
-									// clip audio
-									if (buffer[i] > 0)
-									{
-										buffer[i] = INT_MAX;
-									} else
-									{
-										buffer[i] = INT_MIN;
-									}
-								}
+								buffer[i] = buffer[i] * audioGain;
 							}
 						}
 
-						sf_writef_int(sfFile, buffer, 64);
+						sf_writef_float(sfFile, buffer, 64);
 					}
 				}
 

src/util/audio/VorbisConsumer.h

@@ -26,7 +26,7 @@
 class VorbisConsumer
 {
 public:
-	explicit VorbisConsumer(Settings* settings, AudioQueue<int>* audioQueue);
+	explicit VorbisConsumer(Settings* settings, AudioQueue<float>* audioQueue);
 	~VorbisConsumer();
 
 public:
@@ -41,6 +41,6 @@ protected:
 	bool stopConsumer = false;
 
 	Settings* settings = nullptr;
-	AudioQueue<int>* audioQueue = nullptr;
+	AudioQueue<float>* audioQueue = nullptr;
 	std::thread* consumerThread = nullptr;
 };

src/util/audio/VorbisProducer.cpp

@@ -1,6 +1,7 @@
 #include "VorbisProducer.h"
 
-VorbisProducer::VorbisProducer(AudioQueue<int>* audioQueue) : audioQueue(audioQueue)
+
+VorbisProducer::VorbisProducer(AudioQueue<float>* audioQueue) : audioQueue(audioQueue)
 {
 	XOJ_INIT_TYPE(VorbisProducer);
 }
@@ -40,13 +41,13 @@ bool VorbisProducer::start(std::string filename, unsigned int timestamp)
 			[&, filename]
 			{
 				long numSamples = 1;
-				auto sampleBuffer = new int[1024 * this->sfInfo.channels];
+				auto sampleBuffer = new float[1024 * this->sfInfo.channels];
 
 				while (!this->stopProducer && numSamples > 0 && !this->audioQueue->hasStreamEnded())
 				{
-					numSamples = sf_readf_int(this->sfFile, sampleBuffer, 1024);
+					numSamples = sf_readf_float(this->sfFile, sampleBuffer, 1024);
 
-					while (this->audioQueue->size() > 4096 && !this->audioQueue->hasStreamEnded() && !this->stopProducer)
+					while (this->audioQueue->size() >= this->sample_buffer_size && !this->audioQueue->hasStreamEnded() && !this->stopProducer)
 					{
 						std::this_thread::sleep_for(std::chrono::microseconds(100));
 					}

src/util/audio/VorbisProducer.h

@@ -24,7 +24,7 @@
 class VorbisProducer
 {
 public:
-	explicit VorbisProducer(AudioQueue<int>* audioQueue);
+	explicit VorbisProducer(AudioQueue<float>* audioQueue);
 	~VorbisProducer();
 
 public:
@@ -33,6 +33,7 @@ public:
 	void stop();
 
 private:
+	const int sample_buffer_size = 16384;
 	XOJ_TYPE_ATTRIB;
 
 protected:
@@ -40,6 +41,6 @@ protected:
 	SF_INFO sfInfo;
 	SNDFILE_tag* sfFile = nullptr;
 
-	AudioQueue<int>* audioQueue = nullptr;
+	AudioQueue<float>* audioQueue = nullptr;
 	std::thread* producerThread = nullptr;
 };