From 6aeafbd0fa2ebd48ef3da1d2fa779307ccbdc4bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 15 Sep 2015 16:44:02 -0400
Subject: [PATCH] audiolayer: prevent double-init
Issue: #80355
Change-Id: I8d0a1c0391aa33b9c01e3cd1f1ed315e1a68fe14
---
src/media/audio/alsa/alsalayer.cpp | 20 +++++++++---
src/media/audio/audiolayer.cpp | 3 --
src/media/audio/audiolayer.h | 21 ++++++++----
src/media/audio/coreaudio/corelayer.cpp | 32 ++++++++----------
src/media/audio/coreaudio/corelayer.h | 3 --
src/media/audio/jack/jacklayer.cpp | 34 +++++++++-----------
src/media/audio/jack/jacklayer.h | 1 -
src/media/audio/opensl/opensllayer.cpp | 30 ++++++++++-------
src/media/audio/portaudio/portaudiolayer.cpp | 22 +++++++------
src/media/audio/portaudio/portaudiolayer.h | 2 --
src/media/audio/pulseaudio/pulselayer.cpp | 3 +-
11 files changed, 92 insertions(+), 79 deletions(-)
diff --git a/src/media/audio/alsa/alsalayer.cpp b/src/media/audio/alsa/alsalayer.cpp
index b2ddf9df32..c44d5ba1a0 100644
--- a/src/media/audio/alsa/alsalayer.cpp
+++ b/src/media/audio/alsa/alsalayer.cpp
@@ -136,9 +136,13 @@ void AlsaThread::initAudioLayer(void)
void AlsaThread::run()
{
initAudioLayer();
- alsa_->isStarted_ = true;
+ {
+ std::lock_guard<std::mutex> lock(alsa_->mutex_);
+ alsa_->status_ = AudioLayer::Status::Started;
+ }
+ alsa_->startedCv_.notify_all();
- while (alsa_->isStarted_ and running_) {
+ while (alsa_->status_ == AudioLayer::Status::Started and running_) {
alsa_->audioCallback();
}
}
@@ -168,7 +172,6 @@ AlsaLayer::AlsaLayer(const AudioPreference &pref)
AlsaLayer::~AlsaLayer()
{
- isStarted_ = false;
audioThread_.reset();
/* Then close the audio devices */
@@ -212,6 +215,13 @@ bool AlsaLayer::openDevice(snd_pcm_t **pcm, const std::string &dev, snd_pcm_stre
void
AlsaLayer::startStream()
{
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Idle)
+ return;
+ status_ = Status::Starting;
+ }
+
dcblocker_.reset();
if (is_playback_running_ and is_capture_running_)
@@ -228,8 +238,6 @@ AlsaLayer::startStream()
void
AlsaLayer::stopStream()
{
- isStarted_ = false;
-
audioThread_.reset();
closeCaptureStream();
@@ -242,6 +250,8 @@ AlsaLayer::stopStream()
/* Flush the ring buffers */
flushUrgent();
flushMain();
+
+ status_ = Status::Idle;
}
/*
diff --git a/src/media/audio/audiolayer.cpp b/src/media/audio/audiolayer.cpp
index fb38538dbe..0823ca3a16 100644
--- a/src/media/audio/audiolayer.cpp
+++ b/src/media/audio/audiolayer.cpp
@@ -45,12 +45,9 @@ AudioLayer::AudioLayer(const AudioPreference &pref)
, isPlaybackMuted_(pref.getPlaybackMuted())
, captureGain_(pref.getVolumemic())
, playbackGain_(pref.getVolumespkr())
- , isStarted_(false)
, audioFormat_(Manager::instance().getRingBufferPool().getInternalAudioFormat())
, audioInputFormat_(Manager::instance().getRingBufferPool().getInternalAudioFormat())
, urgentRingBuffer_("urgentRingBuffer_id", SIZEBUF, audioFormat_)
- , mutex_()
- , dcblocker_()
, resampler_(new Resampler{audioFormat_.sample_rate})
, inputResampler_(new Resampler{audioInputFormat_.sample_rate})
, lastNotificationTime_(0)
diff --git a/src/media/audio/audiolayer.h b/src/media/audio/audiolayer.h
index 310bf80a1a..611ddc77ef 100644
--- a/src/media/audio/audiolayer.h
+++ b/src/media/audio/audiolayer.h
@@ -77,6 +77,13 @@ class AudioLayer {
private:
NON_COPYABLE(AudioLayer);
+ protected:
+ enum class Status {
+ Idle,
+ Starting,
+ Started
+ };
+
public:
AudioLayer(const AudioPreference &);
@@ -108,15 +115,15 @@ class AudioLayer {
/**
* Determine wether or not the audio layer is active (i.e. stream opened)
*/
- bool isStarted() const {
- return isStarted_;
+ inline bool isStarted() const {
+ return status_ == Status::Started;
}
template< class Rep, class Period >
bool waitForStart(const std::chrono::duration<Rep, Period>& rel_time) const {
std::unique_lock<std::mutex> lk(mutex_);
- startedCv_.wait_for(lk, rel_time, [&]{return isStarted_.load();});
- return isStarted_;
+ startedCv_.wait_for(lk, rel_time, [&]{return isStarted();});
+ return isStarted();
}
/**
@@ -245,7 +252,7 @@ class AudioLayer {
/**
* Whether or not the audio layer stream is started
*/
- std::atomic_bool isStarted_;
+ Status status_ {Status::Idle};
mutable std::condition_variable startedCv_;
/**
@@ -266,12 +273,12 @@ class AudioLayer {
/**
* Lock for the entire audio layer
*/
- mutable std::mutex mutex_;
+ mutable std::mutex mutex_ {};
/**
* Remove audio offset that can be introduced by certain cheap audio device
*/
- DcBlocker dcblocker_;
+ DcBlocker dcblocker_ {};
/**
* Manage sampling rate conversion
diff --git a/src/media/audio/coreaudio/corelayer.cpp b/src/media/audio/coreaudio/corelayer.cpp
index beb40b7e72..2bbe088056 100644
--- a/src/media/audio/coreaudio/corelayer.cpp
+++ b/src/media/audio/coreaudio/corelayer.cpp
@@ -50,15 +50,11 @@ CoreLayer::CoreLayer(const AudioPreference &pref)
, indexRing_(pref.getAlsaCardring())
, playbackBuff_(0, audioFormat_)
, captureBuff_(0)
- , is_playback_running_(false)
- , is_capture_running_(false)
, mainRingBuffer_(Manager::instance().getRingBufferPool().getRingBuffer(RingBufferPool::DEFAULT_ID))
{}
CoreLayer::~CoreLayer()
{
- isStarted_ = false;
-
if (captureBuff_) {
for (UInt32 i = 0; i < captureBuff_->mNumberBuffers; ++i)
free(captureBuff_->mBuffers[i].mData);
@@ -166,11 +162,6 @@ void CoreLayer::initAudioLayerPlayback()
// Initialize
checkErr(AudioUnitInitialize(outputUnit_));
checkErr(AudioOutputUnitStart(outputUnit_));
-
- is_playback_running_ = true;
- is_capture_running_ = true;
-
- initAudioFormat();
}
void CoreLayer::initAudioLayerCapture()
@@ -304,10 +295,15 @@ void CoreLayer::initAudioLayerCapture()
void CoreLayer::startStream()
{
RING_DBG("START STREAM");
- dcblocker_.reset();
- if (is_playback_running_ and is_capture_running_)
- return;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Idle)
+ return;
+ status_ = Status::Started;
+ }
+
+ dcblocker_.reset();
initAudioLayerPlayback();
initAudioLayerCapture();
@@ -328,7 +324,12 @@ void CoreLayer::stopStream()
{
RING_DBG("STOP STREAM");
- isStarted_ = is_playback_running_ = is_capture_running_ = false;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Started)
+ return;
+ status_ = Status::Idle;
+ }
destroyAudioLayer();
@@ -341,11 +342,6 @@ void CoreLayer::stopStream()
//// PRIVATE /////
-void CoreLayer::initAudioFormat()
-{
-}
-
-
OSStatus CoreLayer::outputCallback(void* inRefCon,
AudioUnitRenderActionFlags* ioActionFlags,
const AudioTimeStamp* inTimeStamp,
diff --git a/src/media/audio/coreaudio/corelayer.h b/src/media/audio/coreaudio/corelayer.h
index d5b39493b8..732348ba32 100644
--- a/src/media/audio/coreaudio/corelayer.h
+++ b/src/media/audio/coreaudio/corelayer.h
@@ -178,9 +178,6 @@ class CoreLayer : public AudioLayer {
AudioUnit inputUnit_;
std::shared_ptr<RingBuffer> mainRingBuffer_;
- bool is_playback_running_;
- bool is_capture_running_;
-
std::vector<AudioDevice> getDeviceList(bool getCapture) const;
};
diff --git a/src/media/audio/jack/jacklayer.cpp b/src/media/audio/jack/jacklayer.cpp
index 3ace332b5c..10ad67b86f 100644
--- a/src/media/audio/jack/jacklayer.cpp
+++ b/src/media/audio/jack/jacklayer.cpp
@@ -257,7 +257,7 @@ JackLayer::ringbuffer_worker()
std::unique_lock<std::mutex> lock(ringbuffer_thread_mutex_);
// may have changed, we don't want to wait for a notification we won't get
- if (not workerAlive_)
+ if (status_ != Status::Started)
return;
// FIXME this is all kinds of evil
@@ -273,7 +273,7 @@ JackLayer::ringbuffer_worker()
data_ready_.wait(lock, [&] {
// Note: lock is released while waiting, and held when woken
// up, so this predicate is called while holding the lock
- return not workerAlive_
+ return status_ != Status::Started
or ringbuffer_ready_for_read(in_ringbuffers_[0]);
});
}
@@ -320,7 +320,7 @@ JackLayer::JackLayer(const AudioPreference &p) :
out_ringbuffers_(),
in_ringbuffers_(),
ringbuffer_thread_(),
- workerAlive_(false),
+ //workerAlive_(false),
ringbuffer_thread_mutex_(),
data_ready_(),
playbackBuffer_(0, audioFormat_),
@@ -477,26 +477,22 @@ JackLayer::process_playback(jack_nframes_t frames, void *arg)
void
JackLayer::startStream()
{
- if (isStarted_)
- return;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Idle)
+ return;
+ status_ = Status::Started;
+ }
dcblocker_.reset();
const auto hardwareFormat = AudioFormat(playbackBuffer_.getSampleRate(), out_ports_.size());
hardwareFormatAvailable(hardwareFormat);
- workerAlive_ = true;
- assert(not ringbuffer_thread_.joinable());
- ringbuffer_thread_ = std::thread(&JackLayer::ringbuffer_worker, this);
-
if (jack_activate(playbackClient_) or jack_activate(captureClient_)) {
RING_ERR("Could not activate JACK client");
- workerAlive_ = false;
- ringbuffer_thread_.join();
- isStarted_ = false;
return;
- } else {
- isStarted_ = true;
}
+ ringbuffer_thread_ = std::thread(&JackLayer::ringbuffer_worker, this);
connectPorts(playbackClient_, JackPortIsInput, out_ports_);
connectPorts(captureClient_, JackPortIsOutput, in_ports_);
@@ -516,17 +512,17 @@ void
JackLayer::stopStream()
{
{
- std::lock_guard<std::mutex> lock(ringbuffer_thread_mutex_);
- workerAlive_ = false;
- data_ready_.notify_one();
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Started)
+ return;
+ status_ = Status::Idle;
}
+ data_ready_.notify_one();
if (jack_deactivate(playbackClient_) or jack_deactivate(captureClient_)) {
RING_ERR("JACK client could not deactivate");
}
- isStarted_ = false;
-
if (ringbuffer_thread_.joinable())
ringbuffer_thread_.join();
diff --git a/src/media/audio/jack/jacklayer.h b/src/media/audio/jack/jacklayer.h
index 8b18920735..1e2f1a5a3b 100644
--- a/src/media/audio/jack/jacklayer.h
+++ b/src/media/audio/jack/jacklayer.h
@@ -57,7 +57,6 @@ class JackLayer : public AudioLayer {
std::vector<jack_ringbuffer_t *> out_ringbuffers_;
std::vector<jack_ringbuffer_t *> in_ringbuffers_;
std::thread ringbuffer_thread_;
- bool workerAlive_;
std::mutex ringbuffer_thread_mutex_;
std::condition_variable data_ready_;
AudioBuffer playbackBuffer_;
diff --git a/src/media/audio/opensl/opensllayer.cpp b/src/media/audio/opensl/opensllayer.cpp
index 2fd1411e2c..f179bafba0 100644
--- a/src/media/audio/opensl/opensllayer.cpp
+++ b/src/media/audio/opensl/opensllayer.cpp
@@ -87,9 +87,6 @@ OpenSLLayer::OpenSLLayer(const AudioPreference &pref)
// Destructor
OpenSLLayer::~OpenSLLayer()
{
- isStarted_ = false;
-
- /* Then close the audio devices */
stopAudioPlayback();
stopAudioCapture();
}
@@ -110,12 +107,17 @@ OpenSLLayer::startStream()
{
dcblocker_.reset();
- if (isStarted_)
- return;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Idle)
+ return;
+ status_ = Status::Starting;
+ }
RING_DBG("Start OpenSL audio layer");
std::vector<int32_t> hw_infos;
+ hw_infos.reserve(4);
emitSignal<DRing::ConfigurationSignal::GetHardwareAudioFormat>(&hw_infos);
hardwareFormat_ = AudioFormat(hw_infos[0], 1); // Mono on Android
hardwareBuffSize_ = hw_infos[1];
@@ -131,7 +133,11 @@ OpenSLLayer::startStream()
init();
startAudioPlayback();
startAudioCapture();
- isStarted_ = true;
+ RING_WARN("OpenSL audio layer started");
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ status_ = Status::Started;
+ }
startedCv_.notify_all();
});
launcher.detach();
@@ -140,16 +146,18 @@ OpenSLLayer::startStream()
void
OpenSLLayer::stopStream()
{
- if (not isStarted_)
- return;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Started)
+ return;
+ status_ = Status::Idle;
+ }
- RING_DBG("Stop OpenSL audio layer");
+ RING_WARN("Stop OpenSL audio layer");
stopAudioPlayback();
stopAudioCapture();
- isStarted_ = false;
-
flushMain();
flushUrgent();
}
diff --git a/src/media/audio/portaudio/portaudiolayer.cpp b/src/media/audio/portaudio/portaudiolayer.cpp
index ed4a73de06..cb943ff1c7 100644
--- a/src/media/audio/portaudio/portaudiolayer.cpp
+++ b/src/media/audio/portaudio/portaudiolayer.cpp
@@ -45,13 +45,12 @@ PortAudioLayer::PortAudioLayer(const AudioPreference &pref)
, playbackBuff_(0, audioFormat_)
, mainRingBuffer_(Manager::instance().getRingBufferPool().getRingBuffer(RingBufferPool::DEFAULT_ID))
{
- isStarted_ = false;
- this->init();
+ init();
}
PortAudioLayer::~PortAudioLayer()
{
- this->terminate();
+ terminate();
}
std::vector<std::string>
@@ -118,17 +117,19 @@ PortAudioLayer::getIndexRingtone() const
void
PortAudioLayer::startStream()
{
- if (isRunning_)
- return;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ if (status_ != Status::Idle)
+ return;
+ status_ = Status::Started;
+ }
this->initStream();
- isRunning_ = isStarted_ = true;
}
void
PortAudioLayer::stopStream()
{
-
- if (!isRunning_)
+ if (status_ != Status::Started)
return;
RING_DBG("Stop PortAudio Streams");
@@ -143,7 +144,10 @@ PortAudioLayer::stopStream()
this->handleError(err);
}
- isRunning_ = isStarted_ = false;
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ status_ = Status::Idle;
+ }
/* Flush the ring buffers */
flushUrgent();
diff --git a/src/media/audio/portaudio/portaudiolayer.h b/src/media/audio/portaudio/portaudiolayer.h
index a982c378ee..c54e713fa7 100644
--- a/src/media/audio/portaudio/portaudiolayer.h
+++ b/src/media/audio/portaudio/portaudiolayer.h
@@ -84,8 +84,6 @@ public:
std::shared_ptr<RingBuffer> mainRingBuffer_;
- bool isRunning_;
-
enum Direction {Input=0, Output=1, End=2};
PaStream* streams[(int)Direction::End];
diff --git a/src/media/audio/pulseaudio/pulselayer.cpp b/src/media/audio/pulseaudio/pulselayer.cpp
index 06c62ae63b..a90d0f7bf2 100644
--- a/src/media/audio/pulseaudio/pulselayer.cpp
+++ b/src/media/audio/pulseaudio/pulselayer.cpp
@@ -137,7 +137,8 @@ PulseLayer::PulseLayer(AudioPreference &pref)
pa_threaded_mainloop_wait(mainloop_);
}
- isStarted_ = true;
+ // No one had the opportunity to lock the mutex or listen on the CV yet.
+ status_ = Status::Started;
}
PulseLayer::~PulseLayer()
--
GitLab