From 1bca75aa4eac948e4ed19c574f7491f8fc53f26b Mon Sep 17 00:00:00 2001 From: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com> Date: Mon, 26 May 2025 17:18:50 -0400 Subject: [PATCH] portaudiolayer: fix stop-stream routine The return value of Pa_IsStreamStopped() was not checked correctly, and the stream pointers were not reset to nullptr. This could theoretically lead to a crash if we were to try to stop an non-null stream pointer. Gitlab: #1130 Change-Id: If2cf3a05b76c8c2d9c7d1ba53606c80e86516b77 --- src/media/audio/portaudio/portaudiolayer.cpp | 56 ++++++++++++-------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/src/media/audio/portaudio/portaudiolayer.cpp b/src/media/audio/portaudio/portaudiolayer.cpp index 054df62eb..639f4d79c 100644 --- a/src/media/audio/portaudio/portaudiolayer.cpp +++ b/src/media/audio/portaudio/portaudiolayer.cpp @@ -61,6 +61,7 @@ struct PortAudioLayer::PortAudioLayerImpl bool outputInitialized_ {false}; std::array<PaStream*, static_cast<int>(Direction::End)> streams_; + bool paStopStream(Direction streamDirection); AudioDeviceNotificationClientPtr audioDeviceNotificationClient_; // The following flag used to debounce the device state changes, @@ -213,31 +214,15 @@ PortAudioLayer::startStream(AudioDeviceType stream) void PortAudioLayer::stopStream(AudioDeviceType stream) { - auto stopPaStream = [](PaStream* stream) -> bool { - if (!stream || Pa_IsStreamStopped(stream) != paNoError) - return false; - auto err = Pa_StopStream(stream); - if (err != paNoError) { - JAMI_ERR("Pa_StopStream error: %s", Pa_GetErrorText(err)); - return false; - } - err = Pa_CloseStream(stream); - if (err != paNoError) { - JAMI_ERR("Pa_CloseStream error: %s", Pa_GetErrorText(err)); - return false; - } - return true; - }; - - auto stopPlayback = [this, &stopPaStream](bool fullDuplexMode = false) -> bool { + auto stopPlayback = [this](bool fullDuplexMode = false) -> bool { std::lock_guard lock(mutex_); if (status_.load() != Status::Started) return false; bool stopped = false; if (fullDuplexMode) - stopped = stopPaStream(pimpl_->streams_[Direction::IO]); + stopped = pimpl_->paStopStream(Direction::IO); else - stopped = stopPaStream(pimpl_->streams_[Direction::Output]); + stopped = pimpl_->paStopStream(Direction::Output); if (stopped) status_.store(Status::Idle); return stopped; @@ -249,7 +234,7 @@ PortAudioLayer::stopStream(AudioDeviceType stream) if (pimpl_->streams_[Direction::IO]) { stopped = stopPlayback(true); } else { - stopped = stopPaStream(pimpl_->streams_[Direction::Input]) && stopPlayback(); + stopped = pimpl_->paStopStream(Direction::Input) && stopPlayback(); } if (stopped) { recordChanged(false); @@ -259,7 +244,7 @@ PortAudioLayer::stopStream(AudioDeviceType stream) return; break; case AudioDeviceType::CAPTURE: - if (stopPaStream(pimpl_->streams_[Direction::Input])) { + if (pimpl_->paStopStream(Direction::Input)) { recordChanged(false); JAMI_DBG("PortAudioLayer input stream stopped"); } else @@ -740,6 +725,35 @@ PortAudioLayer::PortAudioLayerImpl::initFullDuplexStream(PortAudioLayer& parent) return true; } +bool +PortAudioLayer::PortAudioLayerImpl::paStopStream(Direction streamDirection) +{ + PaStream* paStream = streams_[streamDirection]; + if (!paStream) + return false; + auto ret = Pa_IsStreamStopped(paStream); + if (ret == 1) { + JAMI_DBG("PortAudioLayer stream %d already stopped", streamDirection); + return true; + } else if (ret < 0) { + JAMI_ERR("Pa_IsStreamStopped error: %s", Pa_GetErrorText(ret)); + return false; + } + auto err = Pa_StopStream(paStream); + if (err != paNoError) { + JAMI_ERR("Pa_StopStream error: %s", Pa_GetErrorText(err)); + return false; + } + err = Pa_CloseStream(paStream); + if (err != paNoError) { + JAMI_ERR("Pa_CloseStream error: %s", Pa_GetErrorText(err)); + return false; + } + JAMI_DBG("PortAudioLayer stream %d stopped", streamDirection); + streams_[streamDirection] = nullptr; + return true; +}; + int PortAudioLayer::PortAudioLayerImpl::paOutputCallback(PortAudioLayer& parent, const int16_t* inputBuffer, -- GitLab