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

Commit 01542f27 authored by Glenn Kasten's avatar Glenn Kasten
Browse files

Only write to mDevice once 

This fixes a bug where readers might see intermediate values.
Also add comments about how mStandby and mDevice are used.

Change-Id: Idc84e56c21381a45137a2ca5ff9c57d437201869
parent c1dae24a

services/audioflinger/AudioFlinger.cpp

+8 −6
Original line number Diff line number Diff line
@@ -1127,8 +1127,7 @@ AudioFlinger::ThreadBase::ThreadBase(const sp<AudioFlinger>& audioFlinger, audio 
        mChannelCount(0),

        mFrameSize(1), mFormat(AUDIO_FORMAT_INVALID),

        mParamStatus(NO_ERROR),

        mStandby(false), mId(id),

        mDevice(device),

        mStandby(false), mDevice(device), mId(id),

        mDeathRecipient(new PMDeathRecipient(this))

{

}
@@ -3448,7 +3447,7 @@ bool AudioFlinger::MixerThread::checkForNewParameters_l() 


            // forward device change to effects that have requested to be

            // aware of attached audio device.

            mDevice = (uint32_t)value;

            mDevice = value;

            for (size_t i = 0; i < mEffectChains.size(); i++) {

                mEffectChains[i]->setDevice_l(mDevice);

            }
@@ -4036,6 +4035,7 @@ bool AudioFlinger::DuplicatingThread::outputsReady(const SortedVector< sp<Output 
            return false;

        }

        PlaybackThread *playbackThread = (PlaybackThread *)thread.get();

        // see note at standby() declaration

        if (playbackThread->standby() && !playbackThread->isSuspended()) {

            ALOGV("DuplicatingThread output track %p on thread %p Not Ready", outputTracks[i].get(), thread.get());

            return false;
@@ -6474,11 +6474,12 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l() 
            // store input device and output device but do not forward output device to audio HAL.

            // Note that status is ignored by the caller for output device

            // (see AudioFlinger::setParameters()

            audio_devices_t newDevice = mDevice;

            if (value & AUDIO_DEVICE_OUT_ALL) {

                mDevice &= (uint32_t)~(value & AUDIO_DEVICE_OUT_ALL);

                newDevice &= (uint32_t)~(value & AUDIO_DEVICE_OUT_ALL);

                status = BAD_VALUE;

            } else {

                mDevice &= (uint32_t)~(value & AUDIO_DEVICE_IN_ALL);

                newDevice &= (uint32_t)~(value & AUDIO_DEVICE_IN_ALL);

                // disable AEC and NS if the device is a BT SCO headset supporting those pre processings

                if (mTrack != NULL) {

                    bool suspend = audio_is_bluetooth_sco_device(
@@ -6487,7 +6488,8 @@ bool AudioFlinger::RecordThread::checkForNewParameters_l() 
                    setEffectSuspended_l(FX_IID_NS, suspend, mTrack->sessionId());

                }

            }

            mDevice |= (uint32_t)value;

            newDevice |= value;

            mDevice = newDevice;    // since mDevice is read by other threads, only write to it once

        }

        if (status == NO_ERROR) {

            status = mInput->stream->common.set_parameters(&mInput->stream->common, keyValuePair.string());

services/audioflinger/AudioFlinger.h

+21 −5
Original line number Diff line number Diff line
@@ -484,13 +484,19 @@ private: 
        };



        virtual     status_t    initCheck() const = 0;



                    // static externally-visible

                    type_t      type() const { return mType; }

                    audio_io_handle_t id() const { return mId;}



                    // dynamic externally-visible

                    uint32_t    sampleRate() const { return mSampleRate; }

                    int         channelCount() const { return mChannelCount; }

                    audio_format_t format() const { return mFormat; }

                    // Called by AudioFlinger::frameCount(audio_io_handle_t output) and effects,

                    // and returns the normal mix buffer's frame count.  No API for HAL frame count.

                    size_t      frameCount() const { return mNormalFrameCount; }



                    void        wakeUp()    { mWaitWorkCV.broadcast(); }

        // Should be "virtual status_t requestExitAndWait()" and override same

        // method in Thread, but Thread::requestExitAndWait() is not yet virtual.
@@ -502,9 +508,11 @@ private: 
                    void        sendConfigEvent(int event, int param = 0);

                    void        sendConfigEvent_l(int event, int param = 0);

                    void        processConfigEvents();

                    audio_io_handle_t id() const { return mId;}



                    // see note at declaration of mStandby and mDevice

                    bool        standby() const { return mStandby; }

                    uint32_t    device() const { return mDevice; }

                    audio_devices_t device() const { return mDevice; }



        virtual     audio_stream_t* stream() const = 0;



                    sp<EffectHandle> createEffect_l(
@@ -647,11 +655,19 @@ private: 
                    status_t                mParamStatus;



                    Vector<ConfigEvent>     mConfigEvents;

                    bool                    mStandby;



                    // These fields are written and read by thread itself without lock or barrier,

                    // and read by other threads without lock or barrier via standby() and device().

                    // Because of the absence of a lock or barrier, any other thread that reads

                    // these fields must use the information in isolation, or be prepared to deal

                    // with possibility that it might be inconsistent with other information.

                    bool                    mStandby;   // Whether thread is currently in standby.

                    audio_devices_t         mDevice;    // output device for PlaybackThread

                                                        // input + output devices for RecordThread



                    const audio_io_handle_t mId;

                    Vector< sp<EffectChain> > mEffectChains;

                    uint32_t                mDevice;    // output device for PlaybackThread

                                                        // input + output devices for RecordThread



                    static const int        kNameLength = 16;   // prctl(PR_SET_NAME) limit

                    char                    mName[kNameLength];

                    sp<IPowerManager>       mPowerManager;