From 0ac202078c7c6998fa04ba02f33f446d1f5d6b69 Mon Sep 17 00:00:00 2001 From: itsmattkc Date: Fri, 3 Jan 2020 02:29:39 +1100 Subject: [PATCH] rewrite of audio output manager for vastly improved operation Previously the audio output manager was a "hybrid device" that acted as a pull device for audio devices and as a push device and device proxy for the rest of the application. This approach turned out to be flawed, particularly in the frequent opening and closing of the audio device (extremely slow). Since we use both pulling (for constant playback) and pushing (audio scrubbing, short bursts of sound), it needed a similar but different approach. This approach will switch the output device from push mode (default) to pull mode (during playback) only when necessary resulting in far less UI lag (basically unnoticeable now) than the previous approach. --- app/audio/CMakeLists.txt | 4 + app/audio/audiobufferaverage.cpp | 20 +++ app/audio/audiobufferaverage.h | 14 ++ app/audio/audiohybriddevice.cpp | 213 ++++++++++----------------- app/audio/audiohybriddevice.h | 94 ++---------- app/audio/audiomanager.cpp | 26 +--- app/audio/audiooutputdeviceproxy.cpp | 39 +++++ app/audio/audiooutputdeviceproxy.h | 31 ++++ 8 files changed, 203 insertions(+), 238 deletions(-) create mode 100644 app/audio/audiobufferaverage.cpp create mode 100644 app/audio/audiobufferaverage.h create mode 100644 app/audio/audiooutputdeviceproxy.cpp create mode 100644 app/audio/audiooutputdeviceproxy.h diff --git a/app/audio/CMakeLists.txt b/app/audio/CMakeLists.txt index 419f30cdb..99a1f9bff 100644 --- a/app/audio/CMakeLists.txt +++ b/app/audio/CMakeLists.txt @@ -16,10 +16,14 @@ set(OLIVE_SOURCES ${OLIVE_SOURCES} + audio/audiobufferaverage.h + audio/audiobufferaverage.cpp audio/audiohybriddevice.h audio/audiohybriddevice.cpp audio/audiomanager.h audio/audiomanager.cpp + audio/audiooutputdeviceproxy.h + audio/audiooutputdeviceproxy.cpp audio/sampleformat.h audio/sampleformat.cpp PARENT_SCOPE diff --git a/app/audio/audiobufferaverage.cpp b/app/audio/audiobufferaverage.cpp new file mode 100644 index 000000000..1bfd952f2 --- /dev/null +++ b/app/audio/audiobufferaverage.cpp @@ -0,0 +1,20 @@ +#include "audiobufferaverage.h" + +QVector AudioBufferAverage::ProcessAverages(const char *data, int length) +{ + // FIXME: Assumes float and stereo + const float* samples = reinterpret_cast(data); + int sample_count = static_cast(length / static_cast(sizeof(float))); + int channels = 2; + + // Create array of samples to send + QVector averages(channels); + averages.fill(0); + + // Add all samples together + for (int i=0;i(qAbs(samples[i]))); + } + + return averages; +} diff --git a/app/audio/audiobufferaverage.h b/app/audio/audiobufferaverage.h new file mode 100644 index 000000000..62d07066c --- /dev/null +++ b/app/audio/audiobufferaverage.h @@ -0,0 +1,14 @@ +#ifndef AUDIOBUFFERAVERAGE_H +#define AUDIOBUFFERAVERAGE_H + +#include + +class AudioBufferAverage +{ +public: + AudioBufferAverage() = default; + + static QVector ProcessAverages(const char* data, int length); +}; + +#endif // AUDIOBUFFERAVERAGE_H diff --git a/app/audio/audiohybriddevice.cpp b/app/audio/audiohybriddevice.cpp index 85d981143..86420d709 100644 --- a/app/audio/audiohybriddevice.cpp +++ b/app/audio/audiohybriddevice.cpp @@ -23,182 +23,125 @@ #include #include +#include "audiobufferaverage.h" + AudioHybridDevice::AudioHybridDevice(QObject *parent) : - QIODevice(parent), + QObject(parent), output_(nullptr), - device_(nullptr), - sample_index_(0), + push_device_(nullptr), enable_sending_samples_(false) { - + connect(&device_proxy_, &AudioOutputDeviceProxy::ProcessedAverages, this, &AudioHybridDevice::SentSamples); } bool AudioHybridDevice::OutputIsSet() { - qDebug() << "OIS"; - - output_set_lock_.lock(); - - bool output_is_set = (output_.get()); - - output_set_lock_.unlock(); - - return output_is_set; + return (output_.get()); } void AudioHybridDevice::Push(const QByteArray& samples) { - qDebug() << "push"; - - Stop(); - - pushed_samples_ = samples; - sample_index_ = 0; - - if (!pushed_samples_.isEmpty()) { - WakeOutputDevice(); - } -} - -void AudioHybridDevice::Stop() -{ - qDebug() << "Stop"; - - // Whatever is happening, stop it - pushed_samples_.clear(); - - if (device_ != nullptr) { - device_->close(); - device_ = nullptr; - } - - if (output_) { - output_->stop(); - } -} - -void AudioHybridDevice::ConnectDevice(QIODevice *device) -{ - qDebug() << "CD"; - + // If no output device, nothing to be done if (!output_) { return; } - // Clear any previous device or pushed sample - Stop(); + // Replace sample buffer with this one + pushed_samples_ = samples; + pushed_sample_index_ = 0; - device_ = device; + // If we had another device connected, disconnect it now + ResetToPushMode(); - if (device_ != nullptr) { - WakeOutputDevice(); + // Start pushing samples to the output + OutputNotified(); +} + +void AudioHybridDevice::ResetToPushMode() +{ + // If we have a null push device, then we currently have the output in pull mode. We restore it to push mode here. + if (output_ && !push_device_) { + output_->stop(); + + device_proxy_.close(); + + // Put QAudioOutput back into push mode + push_device_ = output_->start(); } } -bool AudioHybridDevice::IsIdle() +void AudioHybridDevice::PullFromDevice(QIODevice *device) { - return device_ == nullptr && pushed_samples_.isEmpty(); + if (!output_ || !device) { + return; + } + + // Stop any current output and disable push mode + output_->stop(); + push_device_ = nullptr; + pushed_samples_.clear(); + + // Pull from the device + device_proxy_.SetDevice(device); + device_proxy_.open(QIODevice::ReadOnly); + output_->start(&device_proxy_); } -void AudioHybridDevice::WakeOutputDevice() +void AudioHybridDevice::OutputNotified() { - if (output_ != nullptr && output_->state() != QAudio::ActiveState) { - output_->start(this); + // Check if we're currently in push mode and if we have samples to push + if (!push_device_ || pushed_samples_.isEmpty()) { + return; + } + + const char* read_ptr = pushed_samples_.constData() + pushed_sample_index_; + + // Push the bytes we have to the audio output + qint64 write_count = push_device_->write(read_ptr, + pushed_samples_.size() - pushed_sample_index_); + + // Emit the samples we just sent + ProcessAverages(read_ptr, static_cast(write_count)); + + // Increment sample buffer index (faster than shift the bytes up) + pushed_sample_index_ += static_cast(write_count); + + // If we've pushed all samples, we can clear this array + if (pushed_sample_index_ == pushed_samples_.size()) { + pushed_samples_.clear(); } } void AudioHybridDevice::SetEnableSendingSamples(bool e) { enable_sending_samples_ = e; + + device_proxy_.SetSendAverages(e); } void AudioHybridDevice::SetOutputDevice(QAudioDeviceInfo info, QAudioFormat format) { - output_set_lock_.lock(); + // Whatever the output is doing right now, stop it + if (output_) { + output_->stop(); + if (device_proxy_.isOpen()) { + device_proxy_.close(); + } + } + + // Create a new output device and start it in push mode output_ = std::unique_ptr(new QAudioOutput(info, format, this)); - - output_set_lock_.unlock(); - + output_->setNotifyInterval(1); + push_device_ = output_->start(); connect(output_.get(), &QAudioOutput::notify, this, &AudioHybridDevice::OutputNotified); } -void AudioHybridDevice::OutputNotified() +void AudioHybridDevice::ProcessAverages(const char *data, int length) { - if (IsIdle()) { - static_cast(sender())->stop(); - } -} - -qint64 AudioHybridDevice::readData(char *data, qint64 maxSize) -{ - qint64 read_size = read_internal(data, maxSize); - - if (enable_sending_samples_ && read_size > 0) { - // FIXME: Assumes float and stereo - float* samples = reinterpret_cast(data); - int sample_count = static_cast(read_size / static_cast(sizeof(float))); - int channels = 2; - - // Create array of samples to send - QVector averages(channels); - averages.fill(0); - - // Add all samples together - for (int i=0;i(qAbs(samples[i]))); - } - - emit SentSamples(averages); + if (!enable_sending_samples_ || length == 0) { + return; } - return read_size; -} - -qint64 AudioHybridDevice::writeData(const char *data, qint64 maxSize) -{ - Q_UNUSED(data) - Q_UNUSED(maxSize) - - // This device doesn't support writing - - return -1; -} - -qint64 AudioHybridDevice::read_internal(char *data, qint64 maxSize) -{ - // If a device is connected, passthrough to it - if (device_ != nullptr) { - qint64 read_count = device_->read(data, maxSize); - - // Stop reading this device - if (device_->atEnd()) { - device_->close(); - device_ = nullptr; - } - - return read_count; - } - - // If there are samples to push, push those - if (!pushed_samples_.isEmpty()) { - qint64 read_count = qMin(maxSize, pushed_samples_.size() - sample_index_); - - memcpy(data, pushed_samples_.data()+sample_index_, static_cast(read_count)); - - sample_index_ += read_count; - - if (sample_index_ == pushed_samples_.size()) { - pushed_samples_.clear(); - } - - if (read_count < maxSize) { - memset(data + read_count, 0, static_cast(maxSize - read_count)); - } - - return maxSize; - } - - memset(data, 0, static_cast(maxSize)); - return maxSize; + emit SentSamples(AudioBufferAverage::ProcessAverages(data, length)); } diff --git a/app/audio/audiohybriddevice.h b/app/audio/audiohybriddevice.h index 56623394a..fc77366ad 100644 --- a/app/audio/audiohybriddevice.h +++ b/app/audio/audiohybriddevice.h @@ -22,25 +22,13 @@ #define AUDIOHYBRIDDEVICE_H #include +#include #include #include -/** - * @brief A device that can be connected to QAudioOutput and provides both "push" and "pull" functionality - * - * By default, a QAudioOutput works in either "push" or "pull" mode. Either it can automatically "pull" from a - * QIODevice (reading from it whenever it requires samples), or we can "push" samples to it constantly (on a timer and - * often in another thread) to make sure the buffer never underruns. - * - * This class functions as a "pull" device for the QAudioOutput, so the audio device automatically reads from it, but - * then provides both "pull" and "push" functionality to the rest of the system. A QIODevice (like a raw PCM file) - * can be connected and pulled from (good for continuous audio playback), and any amount of samples can also be pushed - * (good for short bursts of sound, e.g. audio scrubbing). - * - * This does not support any "queuing", running ConnectDevice() or Push() will immediately discard anything that's - * currently being sent to the device and replace it. - */ -class AudioHybridDevice : public QIODevice +#include "audiooutputdeviceproxy.h" + +class AudioHybridDevice : public QObject { Q_OBJECT public: @@ -53,32 +41,19 @@ public: */ void SetEnableSendingSamples(bool e); -public slots: void SetOutputDevice(QAudioDeviceInfo info, QAudioFormat format); void Push(const QByteArray &samples); - /** - * @brief Stop all audio output - * - * Whatever is being done (pulling from QIODevice or samples), it is stopped and cleared placing this into - * "idle" state. - * - * Note that audio playback may not stop immediately after calling this function as the audio output may still have - * samples in its buffer to output (QAudioOutput::stop() should be called as well for more immediate feedback). This - * will prevent any further samples from being sent however. - * - * \see IsIdle() - */ - void Stop(); - /** * @brief Connect a QIODevice (e.g. QFile) to start sending to the audio output * * This will clear any pushed samples or QIODevices currently being read and will start reading from this next time * the audio output requests data. */ - void ConnectDevice(QIODevice* device); + void PullFromDevice(QIODevice* device); + + void ResetToPushMode(); signals: /** @@ -91,64 +66,21 @@ signals: */ void SentSamples(QVector averages); -protected: - /** - * @brief Internal QIODevice function for reading - * - * Returns either QIODevice::read() from the connected device, or samples from the pushed sample buffer if there is - * none. - * - * If there is neither, this function returns a complete buffer of zeroes (i.e. silence). This is to allow - * pushed samples that don't fulfill the QAudioOutput's internal buffer size. If it cannot fill its internal buffer - * with the samples we send, it won't play anything until it receives more samples (enough to fill the buffer) - * in an attempt to prevent buffer underrun). - * - * Since we don't care about buffer underrun with short bursts of sound, we return silent samples if the QAudioOutput - * requests it. However this does mean the QAudioOutput never returns to QAudio::IdleState (since we never know if a - * read is just to fill the "remainder" of a buffer - which needs zeroes - or just a general read - which technically - * doesn't). It's recommended to check IsIdle() in tandem with the QAudioOutput::notify() signal to - * determine whether the QAudioOutput is effectively idle and can be stopped. - */ - virtual qint64 readData(char *data, qint64 maxSize) override; - - /** - * @brief Internal QIODevice function for writing - * - * This class does not support writing, so this always returns -1 (QIODevice's documented error code for writing) - */ - virtual qint64 writeData(const char *data, qint64 maxSize) override; - private: - /** - * @brief Returns true if there are no more samples to be sent - * - * This is true if a device was connected or samples were pushed but we reached the end and no more data is available - * to be sent. - * - * In this state, this device will continue returning an empty buffer (all zeroes) to the device to workaround - * QAudioOutput's buffer underrun prevention with pushed samples. Therefore the QAudioOutput will never be put into - * QAudio::IdleState. Therefore only way to determine whether the output is no longer receiving usable audio is to - * check this function. - */ - bool IsIdle(); + void ProcessAverages(const char* data, int length); - void WakeOutputDevice(); - - qint64 read_internal(char *data, qint64 maxSize); - - QMutex output_set_lock_; std::unique_ptr output_; - - QIODevice* device_; + QIODevice* push_device_; QByteArray pushed_samples_; - qint64 sample_index_; + int pushed_sample_index_; - QAtomicInt enable_sending_samples_; + bool enable_sending_samples_; + + AudioOutputDeviceProxy device_proxy_; private slots: void OutputNotified(); - }; #endif // AUDIOHYBRIDDEVICE_H diff --git a/app/audio/audiomanager.cpp b/app/audio/audiomanager.cpp index e37a42187..3b538855b 100644 --- a/app/audio/audiomanager.cpp +++ b/app/audio/audiomanager.cpp @@ -76,25 +76,17 @@ bool AudioManager::IsRefreshing() void AudioManager::PushToOutput(const QByteArray &samples) { - QMetaObject::invokeMethod(&output_manager_, - "Push", - Qt::QueuedConnection, - Q_ARG(QByteArray, samples)); + output_manager_.Push(samples); } void AudioManager::StartOutput(QIODevice *device) { - QMetaObject::invokeMethod(&output_manager_, - "ConnectDevice", - Qt::QueuedConnection, - Q_ARG(QIODevice*, device)); + output_manager_.PullFromDevice(device); } void AudioManager::StopOutput() { - QMetaObject::invokeMethod(&output_manager_, - "Stop", - Qt::QueuedConnection); + output_manager_.ResetToPushMode(); } void AudioManager::SetOutputDevice(const QAudioDeviceInfo &info) @@ -142,11 +134,7 @@ void AudioManager::SetOutputDevice(const QAudioDeviceInfo &info) abort(); } - QMetaObject::invokeMethod(&output_manager_, - "SetOutputDevice", - Qt::QueuedConnection, - Q_ARG(QAudioDeviceInfo, info), - Q_ARG(QAudioFormat, format)); + output_manager_.SetOutputDevice(info, format); } // Un-comment this to get debug information about what the audio output is doing @@ -189,12 +177,6 @@ AudioManager::AudioManager() : connect(&output_manager_, &AudioHybridDevice::SentSamples, this, &AudioManager::SentSamples); output_manager_.SetEnableSendingSamples(true); - output_manager_.open(AudioHybridDevice::ReadOnly); - - QThread* output_thread = new QThread(); - output_thread->start(); - - output_manager_.moveToThread(output_thread); } void AudioManager::RefreshThreadDone() diff --git a/app/audio/audiooutputdeviceproxy.cpp b/app/audio/audiooutputdeviceproxy.cpp new file mode 100644 index 000000000..21801d45b --- /dev/null +++ b/app/audio/audiooutputdeviceproxy.cpp @@ -0,0 +1,39 @@ +#include "audiooutputdeviceproxy.h" + +#include "audiobufferaverage.h" + +AudioOutputDeviceProxy::AudioOutputDeviceProxy() : + device_(nullptr), + send_averages_(false) +{ +} + +void AudioOutputDeviceProxy::SetDevice(QIODevice *device) +{ + device_ = device; +} + +void AudioOutputDeviceProxy::SetSendAverages(bool send) +{ + send_averages_ = send; +} + +qint64 AudioOutputDeviceProxy::readData(char *data, qint64 maxlen) +{ + if (device_) { + qint64 read_count = device_->read(data, maxlen); + + if (send_averages_ && read_count > 0) { + emit ProcessedAverages(AudioBufferAverage::ProcessAverages(data, static_cast(read_count))); + } + + return read_count; + } + + return 0; +} + +qint64 AudioOutputDeviceProxy::writeData(const char *data, qint64 maxSize) +{ + return -1; +} diff --git a/app/audio/audiooutputdeviceproxy.h b/app/audio/audiooutputdeviceproxy.h new file mode 100644 index 000000000..2490c8aef --- /dev/null +++ b/app/audio/audiooutputdeviceproxy.h @@ -0,0 +1,31 @@ +#ifndef AUDIOOUTPUTDEVICEPROXY_H +#define AUDIOOUTPUTDEVICEPROXY_H + +#include + +class AudioOutputDeviceProxy : public QIODevice +{ + Q_OBJECT +public: + AudioOutputDeviceProxy(); + + void SetDevice(QIODevice* device); + + void SetSendAverages(bool send); + +signals: + void ProcessedAverages(QVector averages); + +protected: + virtual qint64 readData(char *data, qint64 maxlen) override; + + virtual qint64 writeData(const char *data, qint64 maxSize) override; + +private: + QIODevice* device_; + + bool send_averages_; + +}; + +#endif // AUDIOOUTPUTDEVICEPROXY_H