diff options
author | Glenn Kasten <gkasten@google.com> | 2012-01-09 09:40:36 -0800 |
---|---|---|
committer | Glenn Kasten <gkasten@google.com> | 2012-02-08 17:01:49 -0800 |
commit | 6637baae4244aec731c4014da72418d330636ae1 (patch) | |
tree | 5244135379ecfdd8ce7394ccfed0b363ccabcf56 /services/audioflinger | |
parent | 98ec94c5854daccc3474758524e7f4adfe535ce0 (diff) | |
download | frameworks_av-6637baae4244aec731c4014da72418d330636ae1.zip frameworks_av-6637baae4244aec731c4014da72418d330636ae1.tar.gz frameworks_av-6637baae4244aec731c4014da72418d330636ae1.tar.bz2 |
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
Diffstat (limited to 'services/audioflinger')
-rw-r--r-- | services/audioflinger/AudioFlinger.cpp | 41 | ||||
-rw-r--r-- | 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 93c91fb..4cf570c 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<AudioFlinger>& 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>& 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 97103c4..674b047 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<Track> createTrack_l( const sp<AudioFlinger::Client>& client, @@ -776,6 +772,7 @@ private: int mBytesWritten; private: bool mMasterMute; + void setMasterMute_l(bool muted) { mMasterMute = muted; } protected: SortedVector< wp<Track> > 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); |