From 40bf381eca9c7146f205a4b5dd3f369dd2b0e317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Smoczy=C5=84ski?= Date: Wed, 6 Jan 2021 23:02:35 +0100 Subject: [PATCH] [EGD-5086] Fix voice not starting when calling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to a race condition between source and sink voice is not always starting when calling. Introduce audio stream connections to avoid race condition and improve handling of audio start and stop operations. Signed-off-by: Marcin SmoczyƄski --- module-audio/Audio/BluetoothProxyAudio.cpp | 18 +++ module-audio/Audio/BluetoothProxyAudio.hpp | 7 ++ module-audio/Audio/Endpoint.cpp | 88 ++++++++++--- module-audio/Audio/Endpoint.hpp | 81 ++++++++++-- .../Audio/Operation/PlaybackOperation.cpp | 119 ++++++++---------- .../Audio/Operation/PlaybackOperation.hpp | 1 + .../Audio/Operation/RouterOperation.cpp | 96 ++++++++------ .../Audio/Operation/RouterOperation.hpp | 5 +- .../Profiles/ProfilePlaybackBluetoothA2DP.hpp | 1 - module-audio/Audio/Stream.cpp | 28 +++-- module-audio/Audio/decoder/Decoder.cpp | 20 ++- module-audio/Audio/decoder/Decoder.hpp | 11 +- module-audio/Audio/decoder/DecoderWorker.cpp | 81 +++++++++--- module-audio/Audio/decoder/DecoderWorker.hpp | 25 +++- module-audio/Audio/decoder/decoderWAV.hpp | 1 - .../board/linux/audio/LinuxCellularAudio.cpp | 23 +++- .../board/linux/audio/LinuxCellularAudio.hpp | 7 ++ .../board/linux/audio/linux_audiocodec.cpp | 23 +++- .../board/linux/audio/linux_audiocodec.hpp | 7 ++ .../rt1051/bsp/audio/RT1051Audiocodec.cpp | 63 +--------- .../rt1051/bsp/audio/RT1051Audiocodec.hpp | 4 +- .../rt1051/bsp/audio/RT1051BluetoothAudio.cpp | 104 --------------- .../rt1051/bsp/audio/RT1051BluetoothAudio.hpp | 42 ------- .../rt1051/bsp/audio/RT1051CellularAudio.cpp | 62 +-------- .../rt1051/bsp/audio/RT1051CellularAudio.hpp | 4 +- .../board/rt1051/bsp/audio/SAIAudioDevice.cpp | 97 ++++++++++++++ .../board/rt1051/bsp/audio/SAIAudioDevice.hpp | 38 ++++++ module-bsp/bsp/audio/bsp_audio.cpp | 52 ++++---- module-bsp/bsp/audio/bsp_audio.hpp | 5 +- module-bsp/targets/Target_RT1051.cmake | 2 +- module-sys/Service/Worker.cpp | 5 + module-sys/Service/Worker.hpp | 4 +- 32 files changed, 647 insertions(+), 477 deletions(-) delete mode 100644 module-bsp/board/rt1051/bsp/audio/RT1051BluetoothAudio.cpp delete mode 100644 module-bsp/board/rt1051/bsp/audio/RT1051BluetoothAudio.hpp create mode 100644 module-bsp/board/rt1051/bsp/audio/SAIAudioDevice.cpp create mode 100644 module-bsp/board/rt1051/bsp/audio/SAIAudioDevice.hpp diff --git a/module-audio/Audio/BluetoothProxyAudio.cpp b/module-audio/Audio/BluetoothProxyAudio.cpp index ddf04ac60..a7084eddf 100644 --- a/module-audio/Audio/BluetoothProxyAudio.cpp +++ b/module-audio/Audio/BluetoothProxyAudio.cpp @@ -72,4 +72,22 @@ namespace bsp { Stop(); } + + void BluetoothProxyAudio::onDataReceive() + {} + + void BluetoothProxyAudio::onDataSend() + {} + + void BluetoothProxyAudio::enableInput() + {} + + void BluetoothProxyAudio::enableOutput() + {} + + void BluetoothProxyAudio::disableInput() + {} + + void BluetoothProxyAudio::disableOutput() + {} } // namespace bsp diff --git a/module-audio/Audio/BluetoothProxyAudio.hpp b/module-audio/Audio/BluetoothProxyAudio.hpp index 2be1222f8..de9308651 100644 --- a/module-audio/Audio/BluetoothProxyAudio.hpp +++ b/module-audio/Audio/BluetoothProxyAudio.hpp @@ -28,6 +28,13 @@ namespace bsp AudioDevice::RetCode InputPathCtrl(InputPath inputPath) final; bool IsFormatSupported(const Format &format) final; + void onDataReceive() final; + void onDataSend() final; + void enableInput() final; + void enableOutput() final; + void disableInput() final; + void disableOutput() final; + private: audio::Stream &dataStreamOut; audio::Stream &dataStreamIn; diff --git a/module-audio/Audio/Endpoint.cpp b/module-audio/Audio/Endpoint.cpp index f45b3a617..9f6c6d2a8 100644 --- a/module-audio/Audio/Endpoint.cpp +++ b/module-audio/Audio/Endpoint.cpp @@ -7,18 +7,21 @@ using namespace audio; -void Endpoint::setStream(Stream &stream) +Endpoint::Endpoint(const Capabilities &caps) : _caps(caps) +{} + +const Endpoint::Capabilities &Endpoint::getCapabilities() const noexcept +{ + return _caps; +} + +void Endpoint::connectStream(Stream &stream) { assert(_stream == nullptr); _stream = &stream; } -Stream *Endpoint::getStream() const noexcept -{ - return _stream; -} - -void Endpoint::unsetStream() +void Endpoint::disconnectStream() { assert(_stream != nullptr); _stream = nullptr; @@ -29,16 +32,71 @@ bool Endpoint::isConnected() const noexcept return _stream != nullptr; } -void Source::connect(Sink &sink, Stream &stream) +StreamConnection::StreamConnection(Source *source, Sink *sink, Stream *stream) + : _sink(sink), _source(source), _stream(stream) { - connectedSink = &sink; - connectedSink->setStream(stream); - setStream(stream); + assert(_sink != nullptr); + assert(_source != nullptr); + assert(_stream != nullptr); + + _sink->connectStream(*_stream); + _source->connectStream(*_stream); } -void Source::disconnectStream() +StreamConnection::~StreamConnection() { - unsetStream(); - connectedSink->unsetStream(); - connectedSink = nullptr; + destroy(); +} + +void StreamConnection::destroy() +{ + disable(); + _sink->disconnectStream(); + _source->disconnectStream(); +} + +void StreamConnection::enable() +{ + if (enabled) { + return; + } + + _stream->reset(); + _sink->enableOutput(); + _source->enableInput(); + + enabled = true; +} + +void StreamConnection::disable() +{ + if (!enabled) { + return; + } + + _source->disableInput(); + _sink->disableOutput(); + _stream->reset(); + + enabled = false; +} + +bool StreamConnection::isEnabled() const noexcept +{ + return enabled; +} + +Source *StreamConnection::getSource() const noexcept +{ + return _source; +} + +Sink *StreamConnection::getSink() const noexcept +{ + return _sink; +} + +Stream *StreamConnection::getStream() const noexcept +{ + return _stream; } diff --git a/module-audio/Audio/Endpoint.hpp b/module-audio/Audio/Endpoint.hpp index 858c31f9b..b291e83b0 100644 --- a/module-audio/Audio/Endpoint.hpp +++ b/module-audio/Audio/Endpoint.hpp @@ -10,26 +10,87 @@ namespace audio class Endpoint { public: - void setStream(Stream &stream); - Stream *getStream() const noexcept; - void unsetStream(); + struct Capabilities + { + bool usesDMA = false; + std::size_t maxBlockSize = 0; + }; + + Endpoint() = default; + Endpoint(const Capabilities &caps); + + void connectStream(Stream &stream); + void disconnectStream(); bool isConnected() const noexcept; - private: + [[nodiscard]] const Capabilities &getCapabilities() const noexcept; + + protected: + Capabilities _caps; Stream *_stream = nullptr; }; class Sink : public Endpoint - {}; + { + public: + virtual void onDataSend() = 0; + virtual void enableOutput() = 0; + virtual void disableOutput() = 0; + }; class Source : public Endpoint { public: - void connect(Sink &sink, Stream &stream); - void disconnectStream(); - - private: - Sink *connectedSink = nullptr; + virtual void onDataReceive() = 0; + virtual void enableInput() = 0; + virtual void disableInput() = 0; }; + class IOProxy : public Sink, public Source + { + public: + inline bool isSinkConnected() const noexcept + { + return Sink::isConnected(); + } + + inline bool isSourceConnected() const noexcept + { + return Source::isConnected(); + } + + inline void connectOutputStream(Stream &stream) + { + Sink::connectStream(stream); + } + + inline void connectInputStream(Stream &stream) + { + Source::connectStream(stream); + } + }; + + class StreamConnection + { + public: + StreamConnection() = default; + StreamConnection(Source *source, Sink *sink, Stream *stream); + ~StreamConnection(); + + void enable(); + void disable(); + void destroy(); + + [[nodiscard]] Source *getSource() const noexcept; + [[nodiscard]] Sink *getSink() const noexcept; + [[nodiscard]] Stream *getStream() const noexcept; + + [[nodiscard]] bool isEnabled() const noexcept; + + private: + bool enabled = false; + Sink *_sink = nullptr; + Source *_source = nullptr; + Stream *_stream = nullptr; + }; }; // namespace audio diff --git a/module-audio/Audio/Operation/PlaybackOperation.cpp b/module-audio/Audio/Operation/PlaybackOperation.cpp index 92edb433c..30fe261c4 100644 --- a/module-audio/Audio/Operation/PlaybackOperation.cpp +++ b/module-audio/Audio/Operation/PlaybackOperation.cpp @@ -16,8 +16,6 @@ namespace audio using namespace AudioServiceMessage; using namespace utils; -#define PERF_STATS_ON 0 - PlaybackOperation::PlaybackOperation(const char *file, const audio::PlaybackType &playbackType, Callback callback) : Operation(callback, playbackType), dec(nullptr) { @@ -26,6 +24,13 @@ namespace audio AddProfile(Profile::Type::PlaybackHeadphones, playbackType, false); AddProfile(Profile::Type::PlaybackLoudspeaker, playbackType, true); + endOfFileCallback = [this]() { + state = State::Idle; + const auto req = AudioServiceMessage::EndOfFile(operationToken); + serviceCallback(&req); + return std::string(); + }; + auto defaultProfile = GetProfile(Profile::Type::PlaybackLoudspeaker); if (!defaultProfile) { throw AudioInitException("Error during initializing profile", RetCode::ProfileNotSet); @@ -36,18 +41,12 @@ namespace audio if (dec == nullptr) { throw AudioInitException("Error during initializing decoder", RetCode::FileDoesntExist); } + tags = dec->fetchTags(); auto retCode = SwitchToPriorityProfile(); if (retCode != RetCode::Success) { throw AudioInitException("Failed to switch audio profile", retCode); } - - endOfFileCallback = [this]() { - state = State::Idle; - const auto req = AudioServiceMessage::EndOfFile(operationToken); - serviceCallback(&req); - return std::string(); - }; } audio::RetCode PlaybackOperation::Start(audio::Token token) @@ -55,31 +54,21 @@ namespace audio if (state == State::Active || state == State::Paused) { return RetCode::InvokedInIncorrectState; } + + // create audio connection + outputConnection = std::make_unique(dec.get(), audioDevice.get(), dataStreamOut); + + // decoder worker soft start - must be called after connection setup + dec->startDecodingWorker(endOfFileCallback); + + // start output device and enable audio connection + auto ret = audioDevice->Start(currentProfile->GetAudioFormat()); + outputConnection->enable(); + + // update state and token + state = State::Active; operationToken = token; - assert(dataStreamOut != nullptr); - - dec->startDecodingWorker(*dataStreamOut, endOfFileCallback); - - if (!tags) { - tags = dec->fetchTags(); - } - - state = State::Active; - - if (tags->num_channel == channel::stereoSound) { - currentProfile->SetInOutFlags(static_cast(bsp::AudioDevice::Flags::OutputStereo)); - } - else { - currentProfile->SetInOutFlags(static_cast(bsp::AudioDevice::Flags::OutputMono)); - if (currentProfile->GetOutputPath() == bsp::AudioDevice::OutputPath::Headphones) { - currentProfile->SetOutputPath(bsp::AudioDevice::OutputPath::HeadphonesMono); - } - } - - currentProfile->SetSampleRate(tags->sample_rate); - - auto ret = audioDevice->Start(currentProfile->GetAudioFormat()); return GetDeviceError(ret); } @@ -90,7 +79,10 @@ namespace audio return audio::RetCode::DeviceFailure; } + // stop playback by destroying audio connection + outputConnection.reset(); dec->stopDecodingWorker(); + return GetDeviceError(audioDevice->Stop()); } @@ -100,23 +92,18 @@ namespace audio return RetCode::InvokedInIncorrectState; } state = State::Paused; - - dec->stopDecodingWorker(); - return GetDeviceError(audioDevice->Stop()); + outputConnection->disable(); + return audio::RetCode::Success; } audio::RetCode PlaybackOperation::Resume() { - if (state == State::Active || state == State::Idle) { return RetCode::InvokedInIncorrectState; } - state = State::Active; - - assert(dataStreamOut != nullptr); - dec->startDecodingWorker(*dataStreamOut, endOfFileCallback); - auto ret = audioDevice->Start(currentProfile->GetAudioFormat()); - return GetDeviceError(ret); + state = State::Active; + outputConnection->enable(); + return audio::RetCode::Success; } audio::RetCode PlaybackOperation::SetOutputVolume(float vol) @@ -159,41 +146,40 @@ namespace audio audio::RetCode PlaybackOperation::SwitchProfile(const Profile::Type type) { - uint32_t currentSampleRate = currentProfile->GetSampleRate(); - uint32_t currentInOutFlags = currentProfile->GetInOutFlags(); - - auto ret = GetProfile(type); - if (ret) { - currentProfile = ret; - } - else { + auto newProfile = GetProfile(type); + if (newProfile == nullptr) { return RetCode::UnsupportedProfile; } - if (dec->isConnected()) { - dec->disconnectStream(); - } - - audioDevice = CreateDevice(currentProfile->GetAudioDeviceType(), audioCallback).value_or(nullptr); + /// profile change - (re)create output device; stop audio first by + /// killing audio connection + outputConnection.reset(); + audioDevice.reset(); + audioDevice = CreateDevice(newProfile->GetAudioDeviceType(), audioCallback).value_or(nullptr); if (audioDevice == nullptr) { LOG_ERROR("Error creating AudioDevice"); return RetCode::Failed; } - dec->connect(audioDevice->sink, *dataStreamOut); + // adjust new profile with information from file's tags + newProfile->SetSampleRate(tags->sample_rate); + if (tags->num_channel == channel::stereoSound) { + newProfile->SetInOutFlags(static_cast(bsp::AudioDevice::Flags::OutputStereo)); + } + else { + newProfile->SetInOutFlags(static_cast(bsp::AudioDevice::Flags::OutputMono)); + if (newProfile->GetOutputPath() == bsp::AudioDevice::OutputPath::Headphones) { + newProfile->SetOutputPath(bsp::AudioDevice::OutputPath::HeadphonesMono); + } + } - currentProfile->SetSampleRate(currentSampleRate); - currentProfile->SetInOutFlags(currentInOutFlags); + // store profile + currentProfile = newProfile; - switch (state) { - case State::Idle: - case State::Paused: - break; - - case State::Active: + if (state == State::Active) { + // playback in progress, restart state = State::Idle; Start(operationToken); - break; } return audio::RetCode::Success; @@ -201,10 +187,7 @@ namespace audio PlaybackOperation::~PlaybackOperation() { - dec->stopDecodingWorker(); Stop(); - dataStreamOut->reset(); - dataStreamIn->reset(); } } // namespace audio diff --git a/module-audio/Audio/Operation/PlaybackOperation.hpp b/module-audio/Audio/Operation/PlaybackOperation.hpp index 376e0c15c..de0f1abfe 100644 --- a/module-audio/Audio/Operation/PlaybackOperation.hpp +++ b/module-audio/Audio/Operation/PlaybackOperation.hpp @@ -41,6 +41,7 @@ namespace audio private: std::unique_ptr dec; std::unique_ptr tags; + std::unique_ptr outputConnection = nullptr; DecoderWorker::EndOfFileCallback endOfFileCallback; }; diff --git a/module-audio/Audio/Operation/RouterOperation.cpp b/module-audio/Audio/Operation/RouterOperation.cpp index 8297c79b0..051bb7808 100644 --- a/module-audio/Audio/Operation/RouterOperation.cpp +++ b/module-audio/Audio/Operation/RouterOperation.cpp @@ -49,23 +49,37 @@ namespace audio operationToken = token; state = State::Active; - if (audioDevice->IsFormatSupported(currentProfile->GetAudioFormat())) { - auto ret = audioDevice->Start(currentProfile->GetAudioFormat()); - if (ret != bsp::AudioDevice::RetCode::Success) { - LOG_ERROR("Start error: %s", audio::str(audio::RetCode::DeviceFailure).c_str()); - } - } - else { + // check if audio devices support desired audio format + if (!audioDevice->IsFormatSupported(currentProfile->GetAudioFormat())) { return RetCode::InvalidFormat; } - if (audioDeviceCellular->IsFormatSupported(currentProfile->GetAudioFormat())) { - auto ret = audioDeviceCellular->Start(currentProfile->GetAudioFormat()); - return GetDeviceError(ret); - } - else { + if (!audioDeviceCellular->IsFormatSupported(currentProfile->GetAudioFormat())) { return RetCode::InvalidFormat; } + + // try to run devices with the format + if (auto ret = audioDevice->Start(currentProfile->GetAudioFormat()); + ret != bsp::AudioDevice::RetCode::Success) { + return GetDeviceError(ret); + } + + if (auto ret = audioDeviceCellular->Start(currentProfile->GetAudioFormat()); + ret != bsp::AudioDevice::RetCode::Success) { + return GetDeviceError(ret); + } + + // create audio connections + inputConnection = + std::make_unique(audioDeviceCellular.get(), audioDevice.get(), dataStreamIn); + outputConnection = + std::make_unique(audioDevice.get(), audioDeviceCellular.get(), dataStreamOut); + + // enable audio connections + inputConnection->enable(); + outputConnection->enable(); + + return audio::RetCode::Success; } audio::RetCode RouterOperation::Stop() @@ -75,10 +89,14 @@ namespace audio } state = State::Idle; + outputConnection.reset(); + inputConnection.reset(); + audioDevice->Stop(); audioDeviceCellular->Stop(); - dataStreamOut->reset(); - dataStreamIn->reset(); + + audioDevice.reset(); + audioDeviceCellular.reset(); return RetCode::Success; } @@ -90,8 +108,8 @@ namespace audio } state = State::Paused; - audioDevice->Stop(); - audioDeviceCellular->Stop(); + outputConnection->disable(); + inputConnection->disable(); return RetCode::Success; } @@ -102,8 +120,8 @@ namespace audio } state = State::Active; - audioDevice->Start(currentProfile->GetAudioFormat()); - audioDeviceCellular->Start(currentProfile->GetAudioFormat()); + inputConnection->enable(); + outputConnection->enable(); return RetCode::Success; } @@ -141,41 +159,38 @@ namespace audio audio::RetCode RouterOperation::SwitchProfile(const audio::Profile::Type type) { - auto ret = GetProfile(type); + auto newProfile = GetProfile(type); + auto callInProgress = state == State::Active; - if (ret) { - if (currentProfile && currentProfile->GetType() == ret->GetType()) { - return RetCode::Success; - } - currentProfile = ret; - } - else { + if (newProfile == nullptr) { return RetCode::UnsupportedProfile; } - audioDevice = CreateDevice(currentProfile->GetAudioDeviceType(), nullptr).value_or(nullptr); + if (currentProfile && currentProfile->GetType() == newProfile->GetType()) { + return RetCode::Success; + } + + if (callInProgress) { + Stop(); + } + + audioDevice = CreateDevice(newProfile->GetAudioDeviceType(), nullptr).value_or(nullptr); if (audioDevice == nullptr) { LOG_ERROR("Error creating AudioDevice"); return RetCode::Failed; } + audioDeviceCellular = CreateDevice(bsp::AudioDevice::Type::Cellular, nullptr).value_or(nullptr); if (audioDeviceCellular == nullptr) { LOG_ERROR("Error creating AudioDeviceCellular"); return RetCode::Failed; } - audioDevice->source.connect(audioDeviceCellular->sink, *dataStreamIn); - audioDeviceCellular->source.connect(audioDevice->sink, *dataStreamOut); + // store new profile + currentProfile = newProfile; - switch (state) { - case State::Idle: - case State::Paused: - break; - - case State::Active: - state = State::Idle; - Start(operationToken); - break; + if (callInProgress) { + return Start(operationToken); } return RetCode::Success; @@ -192,4 +207,9 @@ namespace audio return 0.0; } + RouterOperation::~RouterOperation() + { + Stop(); + } + } // namespace audio diff --git a/module-audio/Audio/Operation/RouterOperation.hpp b/module-audio/Audio/Operation/RouterOperation.hpp index 1365202e5..d6f88440a 100644 --- a/module-audio/Audio/Operation/RouterOperation.hpp +++ b/module-audio/Audio/Operation/RouterOperation.hpp @@ -8,6 +8,7 @@ #include