Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8b02b994 authored by Glenn Kasten's avatar Glenn Kasten
Browse files

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
parent 52835f5d

services/audioflinger/AudioFlinger.cpp

+20 −21
Original line number Diff line number Diff line
@@ -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);

                        }

                    }

services/audioflinger/AudioFlinger.h

+11 −10
Original line number Diff line number Diff line
@@ -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);