From 8b02b994348431dca6979fc0f03bf9787164a2a0 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Mon, 9 Jan 2012 09:40:36 -0800 Subject: Fix races related to volume and mute Fix race conditions when setting master volume, master mute, stream volume, stream mute for a playback thread, and when reading stream volume of a playback thread. Lock order is AudioFlinger, then thread. Rename streamVolumeInternal to streamVolume_l, comment, and use it to implement streamVolume(). Code size reduction: - Remove dead code: AudioFlinger::PlaybackThread::masterVolume, masterMute, streamMute. - Change return type of non-binder methods that always succeed from status_t to void. - Remove virtual from volume and mute methods that don't need it. This change saves 228 bytes but decreases performance of binder operations due to the added locks. Change-Id: Iac75abc1f54784873a667d1981b2e08f8f31e5c9 --- services/audioflinger/AudioFlinger.cpp | 41 +++++++++++++++++----------------- services/audioflinger/AudioFlinger.h | 21 ++++++++--------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index 93c91fb60a31..4cf570cf06aa 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -729,7 +729,7 @@ float AudioFlinger::streamVolume(audio_stream_type_t stream, audio_io_handle_t o } volume = thread->streamVolume(stream); } else { - volume = mStreamTypes[stream].volume; + volume = streamVolume_l(stream); } return volume; @@ -741,7 +741,8 @@ bool AudioFlinger::streamMute(audio_stream_type_t stream) const return true; } - return mStreamTypes[stream].mute; + AutoMutex lock(mLock); + return streamMute_l(stream); } status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& keyValuePairs) @@ -1383,11 +1384,13 @@ AudioFlinger::PlaybackThread::PlaybackThread(const sp& audioFlinge // There is no AUDIO_STREAM_MIN, and ++ operator does not compile for (audio_stream_type_t stream = (audio_stream_type_t) 0; stream < AUDIO_STREAM_CNT; stream = (audio_stream_type_t) (stream + 1)) { - mStreamTypes[stream].volume = mAudioFlinger->streamVolumeInternal(stream); - mStreamTypes[stream].mute = mAudioFlinger->streamMute(stream); + mStreamTypes[stream].volume = mAudioFlinger->streamVolume_l(stream); + mStreamTypes[stream].mute = mAudioFlinger->streamMute_l(stream); // initialized by stream_type_t default constructor // mStreamTypes[stream].valid = true; } + // mStreamTypes[AUDIO_STREAM_CNT] exists but isn't explicitly initialized here, + // because mAudioFlinger doesn't have one to copy from } AudioFlinger::PlaybackThread::~PlaybackThread() @@ -1582,40 +1585,36 @@ uint32_t AudioFlinger::PlaybackThread::latency() const } } -status_t AudioFlinger::PlaybackThread::setMasterVolume(float value) +void AudioFlinger::PlaybackThread::setMasterVolume(float value) { + Mutex::Autolock _l(mLock); mMasterVolume = value; - return NO_ERROR; } -status_t AudioFlinger::PlaybackThread::setMasterMute(bool muted) +void AudioFlinger::PlaybackThread::setMasterMute(bool muted) { - mMasterMute = muted; - return NO_ERROR; + Mutex::Autolock _l(mLock); + setMasterMute_l(muted); } -status_t AudioFlinger::PlaybackThread::setStreamVolume(audio_stream_type_t stream, float value) +void AudioFlinger::PlaybackThread::setStreamVolume(audio_stream_type_t stream, float value) { + Mutex::Autolock _l(mLock); mStreamTypes[stream].volume = value; - return NO_ERROR; } -status_t AudioFlinger::PlaybackThread::setStreamMute(audio_stream_type_t stream, bool muted) +void AudioFlinger::PlaybackThread::setStreamMute(audio_stream_type_t stream, bool muted) { + Mutex::Autolock _l(mLock); mStreamTypes[stream].mute = muted; - return NO_ERROR; } float AudioFlinger::PlaybackThread::streamVolume(audio_stream_type_t stream) const { + Mutex::Autolock _l(mLock); return mStreamTypes[stream].volume; } -bool AudioFlinger::PlaybackThread::streamMute(audio_stream_type_t stream) const -{ - return mStreamTypes[stream].mute; -} - // addTrack_l() must be called with ThreadBase::mLock held status_t AudioFlinger::PlaybackThread::addTrack_l(const sp& track) { @@ -1945,7 +1944,7 @@ bool AudioFlinger::MixerThread::threadLoop() property_get("ro.audio.silent", value, "0"); if (atoi(value)) { ALOGD("Silence is golden"); - setMasterMute(true); + setMasterMute_l(true); } } @@ -2661,7 +2660,7 @@ bool AudioFlinger::DirectOutputThread::threadLoop() property_get("ro.audio.silent", value, "0"); if (atoi(value)) { ALOGD("Silence is golden"); - setMasterMute(true); + setMasterMute_l(true); } } @@ -3057,7 +3056,7 @@ bool AudioFlinger::DuplicatingThread::threadLoop() property_get("ro.audio.silent", value, "0"); if (atoi(value)) { ALOGD("Silence is golden"); - setMasterMute(true); + setMasterMute_l(true); } } diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 97103c491def..674b0475d763 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -710,17 +710,13 @@ private: virtual uint32_t latency() const; - virtual status_t setMasterVolume(float value); - virtual status_t setMasterMute(bool muted); + void setMasterVolume(float value); + void setMasterMute(bool muted); - virtual float masterVolume() const { return mMasterVolume; } - virtual bool masterMute() const { return mMasterMute; } + void setStreamVolume(audio_stream_type_t stream, float value); + void setStreamMute(audio_stream_type_t stream, bool muted); - virtual status_t setStreamVolume(audio_stream_type_t stream, float value); - virtual status_t setStreamMute(audio_stream_type_t stream, bool muted); - - virtual float streamVolume(audio_stream_type_t stream) const; - virtual bool streamMute(audio_stream_type_t stream) const; + float streamVolume(audio_stream_type_t stream) const; sp createTrack_l( const sp& client, @@ -776,6 +772,7 @@ private: int mBytesWritten; private: bool mMasterMute; + void setMasterMute_l(bool muted) { mMasterMute = muted; } protected: SortedVector< wp > mActiveTracks; @@ -899,7 +896,11 @@ private: PlaybackThread *checkPlaybackThread_l(audio_io_handle_t output) const; MixerThread *checkMixerThread_l(audio_io_handle_t output) const; RecordThread *checkRecordThread_l(audio_io_handle_t input) const; - float streamVolumeInternal(audio_stream_type_t stream) const + // no range check, AudioFlinger::mLock held + bool streamMute_l(audio_stream_type_t stream) const + { return mStreamTypes[stream].mute; } + // no range check, doesn't check per-thread stream volume, AudioFlinger::mLock held + float streamVolume_l(audio_stream_type_t stream) const { return mStreamTypes[stream].volume; } void audioConfigChanged_l(int event, audio_io_handle_t ioHandle, void *param2); -- cgit v1.2.3-59-g8ed1b