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

Commit 58f5ce14 authored by Phil Burk's avatar Phil Burk
Browse files

aaudio: prevent onAudioDeviceUpdate past close 

To prevent late device update callbacks, the AudioStream
was made into a RefBase AudioDeviceCallback.
So now we can use a smart pointer to delay deletion of the
stream if any callbacks are in flight while the stream is closed.

So an AudioStream is now a RefBased object and can be used with
android::sp. That required some changes to the way it is referenced
in several places.

MyPlayerBase was modified to use a weak pointer to the parent stream.

Bug: 161914201
Bug: 163165126
Bug: 164411271
Test: see bug for repro of the crash
Test: atest CtsNativeMediaAAudioTestCases
Change-Id: Ic24aca3eaf5d1610504bb89c06001a37d0d4a1c3
parent 320910fc

media/libaaudio/src/client/AudioStreamInternalCapture.cpp

+8 −2
Original line number Diff line number Diff line
@@ -14,8 +14,6 @@ 
 * limitations under the License.

 */



#define LOG_TAG (mInService ? "AudioStreamInternalCapture_Service" \

                          : "AudioStreamInternalCapture_Client")

//#define LOG_NDEBUG 0

#include <utils/Log.h>
@@ -29,6 +27,14 @@ 
#define ATRACE_TAG ATRACE_TAG_AUDIO

#include <utils/Trace.h>



// We do this after the #includes because if a header uses ALOG.

// it would fail on the reference to mInService.

#undef LOG_TAG

// This file is used in both client and server processes.

// This is needed to make sense of the logs more easily.

#define LOG_TAG (mInService ? "AudioStreamInternalCapture_Service" \

                          : "AudioStreamInternalCapture_Client")



using android::WrappingBuffer;



using namespace aaudio;

media/libaaudio/src/client/AudioStreamInternalPlay.cpp

+8 −2
Original line number Diff line number Diff line
@@ -14,8 +14,6 @@ 
 * limitations under the License.

 */



#define LOG_TAG (mInService ? "AudioStreamInternalPlay_Service" \

                          : "AudioStreamInternalPlay_Client")

//#define LOG_NDEBUG 0

#include <utils/Log.h>
@@ -26,6 +24,14 @@ 
#include "client/AudioStreamInternalPlay.h"

#include "utility/AudioClock.h"



// We do this after the #includes because if a header uses ALOG.

// it would fail on the reference to mInService.

#undef LOG_TAG

// This file is used in both client and server processes.

// This is needed to make sense of the logs more easily.

#define LOG_TAG (mInService ? "AudioStreamInternalPlay_Service" \

                            : "AudioStreamInternalPlay_Client")



using android::WrappingBuffer;



using namespace aaudio;

media/libaaudio/src/core/AAudioAudio.cpp

+2 −1
Original line number Diff line number Diff line
@@ -263,7 +263,8 @@ AAUDIO_API aaudio_result_t AAudioStream_close(AAudioStream* stream) { 
                  __func__, id);

        } else {

            audioStream->unregisterPlayerBase();

            delete audioStream;

            // Allow the stream to be deleted.

            AudioStreamBuilder::stopUsingStream(audioStream);

        }

        ALOGD("%s(s#%u) returned %d ---------", __func__, id, result);

    }

media/libaaudio/src/core/AudioStream.cpp

+35 −16
Original line number Diff line number Diff line
@@ -39,7 +39,7 @@ static aaudio_stream_id_t AAudio_getNextStreamId() { 
}



AudioStream::AudioStream()

        : mPlayerBase(new MyPlayerBase(this))

        : mPlayerBase(new MyPlayerBase())

        , mStreamId(AAudio_getNextStreamId())

        {

    // mThread is a pthread_t of unknown size so we need memset.
@@ -48,6 +48,10 @@ AudioStream::AudioStream() 
}



AudioStream::~AudioStream() {

    // Please preserve this log because there have been several bugs related to

    // AudioStream deletion and late callbacks.

    ALOGD("%s(s#%u) mPlayerBase strongCount = %d",

            __func__, getId(), mPlayerBase->getStrongCount());

    // If the stream is deleted when OPEN or in use then audio resources will leak.

    // This would indicate an internal error. So we want to find this ASAP.

    LOG_ALWAYS_FATAL_IF(!(getState() == AAUDIO_STREAM_STATE_CLOSED
@@ -55,8 +59,6 @@ AudioStream::~AudioStream() { 
                          || getState() == AAUDIO_STREAM_STATE_DISCONNECTED),

                        "~AudioStream() - still in use, state = %s",

                        AudioGlobal_convertStreamStateToText(getState()));



    mPlayerBase->clearParentReference(); // remove reference to this AudioStream

}



aaudio_result_t AudioStream::open(const AudioStreamBuilder& builder)
@@ -534,11 +536,18 @@ bool AudioStream::collidesWithCallback() const { 
}



#if AAUDIO_USE_VOLUME_SHAPER

android::media::VolumeShaper::Status AudioStream::applyVolumeShaper(

        const android::media::VolumeShaper::Configuration& configuration __unused,

        const android::media::VolumeShaper::Operation& operation __unused) {

    ALOGW("applyVolumeShaper() is not supported");

    return android::media::VolumeShaper::Status::ok();

::android::binder::Status AudioStream::MyPlayerBase::applyVolumeShaper(

        const ::android::media::VolumeShaper::Configuration& configuration,

        const ::android::media::VolumeShaper::Operation& operation) {

    android::sp<AudioStream> audioStream;

    {

        std::lock_guard<std::mutex> lock(mParentLock);

        audioStream = mParent.promote();

    }

    if (audioStream) {

        return audioStream->applyVolumeShaper(configuration, operation);

    }

    return android::NO_ERROR;

}

#endif
@@ -548,26 +557,36 @@ void AudioStream::setDuckAndMuteVolume(float duckAndMuteVolume) { 
    doSetVolume(); // apply this change

}



AudioStream::MyPlayerBase::MyPlayerBase(AudioStream *parent) : mParent(parent) {

}



AudioStream::MyPlayerBase::~MyPlayerBase() {

}



void AudioStream::MyPlayerBase::registerWithAudioManager() {

void AudioStream::MyPlayerBase::registerWithAudioManager(const android::sp<AudioStream>& parent) {

    std::lock_guard<std::mutex> lock(mParentLock);

    mParent = parent;

    if (!mRegistered) {

        init(android::PLAYER_TYPE_AAUDIO, AAudioConvert_usageToInternal(mParent->getUsage()));

        init(android::PLAYER_TYPE_AAUDIO, AAudioConvert_usageToInternal(parent->getUsage()));

        mRegistered = true;

    }

}



void AudioStream::MyPlayerBase::unregisterWithAudioManager() {

    std::lock_guard<std::mutex> lock(mParentLock);

    if (mRegistered) {

        baseDestroy();

        mRegistered = false;

    }

}



android::status_t AudioStream::MyPlayerBase::playerSetVolume() {

    android::sp<AudioStream> audioStream;

    {

        std::lock_guard<std::mutex> lock(mParentLock);

        audioStream = mParent.promote();

    }

    if (audioStream) {

        // No pan and only left volume is taken into account from IPLayer interface

        audioStream->setDuckAndMuteVolume(mVolumeMultiplierL  /* * mPanMultiplierL */);

    }

    return android::NO_ERROR;

}



void AudioStream::MyPlayerBase::destroy() {

    unregisterWithAudioManager();

}

media/libaaudio/src/core/AudioStream.h

+22 −22
Original line number Diff line number Diff line
@@ -25,8 +25,10 @@ 
#include <binder/Status.h>

#include <utils/StrongPointer.h>



#include "media/VolumeShaper.h"

#include "media/PlayerBase.h"

#include <media/AudioSystem.h>

#include <media/PlayerBase.h>

#include <media/VolumeShaper.h>



#include "utility/AAudioUtilities.h"

#include "utility/MonotonicCounter.h"
@@ -45,7 +47,8 @@ constexpr pid_t CALLBACK_THREAD_NONE = 0; 
/**

 * AAudio audio stream.

 */

class AudioStream {

// By extending AudioDeviceCallback, we also inherit from RefBase.

class AudioStream : public android::AudioSystem::AudioDeviceCallback {

public:



    AudioStream();
@@ -344,6 +347,10 @@ public: 
     */

    bool collidesWithCallback() const;



    // Implement AudioDeviceCallback

    void onAudioDeviceUpdate(audio_io_handle_t audioIo,

            audio_port_handle_t deviceId) override {};



    // ============== I/O ===========================

    // A Stream will only implement read() or write() depending on its direction.

    virtual aaudio_result_t write(const void *buffer __unused,
@@ -382,7 +389,7 @@ public: 
     */

    void registerPlayerBase() {

        if (getDirection() == AAUDIO_DIRECTION_OUTPUT) {

            mPlayerBase->registerWithAudioManager();

            mPlayerBase->registerWithAudioManager(this);

        }

    }
@@ -430,14 +437,14 @@ protected: 
    // PlayerBase allows the system to control the stream volume.

    class MyPlayerBase : public android::PlayerBase {

    public:

        explicit MyPlayerBase(AudioStream *parent);

        MyPlayerBase() {};



        virtual ~MyPlayerBase();

        virtual ~MyPlayerBase() = default;



        /**

         * Register for volume changes and remote control.

         */

        void registerWithAudioManager();

        void registerWithAudioManager(const android::sp<AudioStream>& parent);



        /**

         * UnRegister.
@@ -449,8 +456,6 @@ protected: 
         */

        void destroy() override;



        void clearParentReference() { mParent = nullptr; }



        // Just a stub. The ability to start audio through PlayerBase is being deprecated.

        android::status_t playerStart() override {

            return android::NO_ERROR;
@@ -466,18 +471,10 @@ protected: 
            return android::NO_ERROR;

        }



        android::status_t playerSetVolume() override {

            // No pan and only left volume is taken into account from IPLayer interface

            mParent->setDuckAndMuteVolume(mVolumeMultiplierL  /* * mPanMultiplierL */);

            return android::NO_ERROR;

        }

        android::status_t playerSetVolume() override;



#if AAUDIO_USE_VOLUME_SHAPER

        ::android::binder::Status applyVolumeShaper(

                const ::android::media::VolumeShaper::Configuration& configuration,

                const ::android::media::VolumeShaper::Operation& operation) {

            return mParent->applyVolumeShaper(configuration, operation);

        }

        ::android::binder::Status applyVolumeShaper();

#endif



        aaudio_result_t getResult() {
@@ -485,7 +482,10 @@ protected: 
        }



    private:

        AudioStream          *mParent;

        // Use a weak pointer so the AudioStream can be deleted.



        std::mutex               mParentLock;

        android::wp<AudioStream> mParent;

        aaudio_result_t          mResult = AAUDIO_OK;

        bool                     mRegistered = false;

    };