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

Commit d14f1ad8 authored by Mikhail Naganov's avatar Mikhail Naganov Committed by Gerrit Code Review
Browse files

Merge "Fix thread safety in libaudioclient tests" into main

parents 442ee225 1196e406

media/libaudioclient/tests/Android.bp

+3 −0
Original line number Diff line number Diff line
@@ -133,6 +133,9 @@ cc_defaults { 
        "libaudiomanager",

        "libaudiopolicy",

    ],

    cflags: [

        "-Wthread-safety",

    ],

    data: ["bbb*.raw"],

    srcs: [

        "audio_test_utils.cpp",

media/libaudioclient/tests/audio_test_utils.cpp

+48 −19
Original line number Diff line number Diff line
@@ -28,25 +28,35 @@ 


void OnAudioDeviceUpdateNotifier::onAudioDeviceUpdate(audio_io_handle_t audioIo,

                                                      audio_port_handle_t deviceId) {

    std::unique_lock<std::mutex> lock{mMutex};

    ALOGI("%s: audioIo=%d deviceId=%d", __func__, audioIo, deviceId);

    {

        std::lock_guard lock(mMutex);

        mAudioIo = audioIo;

        mDeviceId = deviceId;

    }

    mCondition.notify_all();

}



status_t OnAudioDeviceUpdateNotifier::waitForAudioDeviceCb(audio_port_handle_t expDeviceId) {

    std::unique_lock<std::mutex> lock{mMutex};

    std::unique_lock lock(mMutex);

    android::base::ScopedLockAssertion lock_assertion(mMutex);

    if (mAudioIo == AUDIO_IO_HANDLE_NONE ||

        (expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId)) {

        mCondition.wait_for(lock, std::chrono::milliseconds(500));

        if (mAudioIo == AUDIO_IO_HANDLE_NONE ||

            (expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId))

            (expDeviceId != AUDIO_PORT_HANDLE_NONE && expDeviceId != mDeviceId)) {

            return TIMED_OUT;

        }

    }

    return OK;

}



std::pair<audio_io_handle_t, audio_port_handle_t>

OnAudioDeviceUpdateNotifier::getLastPortAndDevice() const {

    std::lock_guard lock(mMutex);

    return {mAudioIo, mDeviceId};

}



AudioPlayback::AudioPlayback(uint32_t sampleRate, audio_format_t format,

                             audio_channel_mask_t channelMask, audio_output_flags_t flags,

                             audio_session_t sessionId, AudioTrack::transfer_type transferType,
@@ -147,9 +157,8 @@ status_t AudioPlayback::start() { 
}



void AudioPlayback::onBufferEnd() {

    std::unique_lock<std::mutex> lock{mMutex};

    std::lock_guard lock(mMutex);

    mStopPlaying = true;

    mCondition.notify_all();

}



status_t AudioPlayback::fillBuffer() {
@@ -187,7 +196,12 @@ status_t AudioPlayback::waitForConsumption(bool testSeek) { 
    const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;

    int counter = 0;

    size_t totalFrameCount = mMemCapacity / mTrack->frameSize();

    while (!mStopPlaying && counter < maxTries) {

    bool stopPlaying;

    {

        std::lock_guard lock(mMutex);

        stopPlaying = mStopPlaying;

    }

    while (!stopPlaying && counter < maxTries) {

        uint32_t currPosition;

        mTrack->getPosition(&currPosition);

        if (currPosition >= totalFrameCount) counter++;
@@ -213,7 +227,10 @@ status_t AudioPlayback::waitForConsumption(bool testSeek) { 
            mTrack->start();

        }

        std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_PERIOD_MS));

        std::lock_guard lock(mMutex);

        stopPlaying = mStopPlaying;

    }

    std::lock_guard lock(mMutex);

    if (!mStopPlaying && counter == maxTries) return TIMED_OUT;

    return OK;

}
@@ -228,8 +245,10 @@ status_t AudioPlayback::onProcess(bool testSeek) { 
}



void AudioPlayback::stop() {

    std::unique_lock<std::mutex> lock{mMutex};

    {

        std::lock_guard lock(mMutex);

        mStopPlaying = true;

    }

    if (mState != PLAY_STOPPED && mState != PLAY_NO_INIT) {

        int32_t msec = 0;

        (void)mTrack->pendingDuration(&msec);
@@ -257,11 +276,14 @@ size_t AudioCapture::onMoreData(const AudioRecord::Buffer& buffer) { 
        return 0;

    }



    {

        std::lock_guard l(mMutex);

        // no more frames to read

        if (mNumFramesReceived >= mNumFramesToRecord || mStopRecording) {

            mStopRecording = true;

            return 0;

        }

    }



    int64_t timeUs = 0, position = 0, timeNs = 0;

    ExtendedTimestamp ts;
@@ -324,9 +346,12 @@ size_t AudioCapture::onMoreData(const AudioRecord::Buffer& buffer) { 
    }



    if (tmpQueue.size() > 0) {

        std::unique_lock<std::mutex> lock{mMutex};

        for (auto it = tmpQueue.begin(); it != tmpQueue.end(); it++)

            mBuffersReceived.push_back(std::move(*it));

        {

            std::lock_guard lock(mMutex);

            mBuffersReceived.insert(mBuffersReceived.end(),

                                    std::make_move_iterator(tmpQueue.begin()),

                                    std::make_move_iterator(tmpQueue.end()));

        }

        mCondition.notify_all();

    }

    return buffer.size();
@@ -484,7 +509,10 @@ status_t AudioCapture::start(AudioSystem::sync_event_t event, audio_session_t tr 


status_t AudioCapture::stop() {

    status_t status = OK;

    {

        std::lock_guard l(mMutex);

        mStopRecording = true;

    }

    if (mState != REC_STOPPED && mState != REC_NO_INIT) {

        if (mInputSource != AUDIO_SOURCE_DEFAULT) {

            bool state = false;
@@ -530,7 +558,8 @@ status_t AudioCapture::obtainBufferCb(RawBuffer& buffer) { 
    if (REC_STARTED != mState) return INVALID_OPERATION;

    const int maxTries = MAX_WAIT_TIME_MS / WAIT_PERIOD_MS;

    int counter = 0;

    std::unique_lock<std::mutex> lock{mMutex};

    std::unique_lock lock(mMutex);

    android::base::ScopedLockAssertion lock_assertion(mMutex);

    while (mBuffersReceived.empty() && !mStopRecording && counter < maxTries) {

        mCondition.wait_for(lock, std::chrono::milliseconds(WAIT_PERIOD_MS));

        counter++;

media/libaudioclient/tests/audio_test_utils.h

+21 −21
Original line number Diff line number Diff line
@@ -19,14 +19,13 @@ 


#include <sys/stat.h>

#include <unistd.h>

#include <atomic>

#include <chrono>

#include <cinttypes>

#include <deque>

#include <memory>

#include <mutex>

#include <thread>

#include <utility>



#include <android-base/thread_annotations.h>

#include <binder/MemoryDealer.h>

#include <media/AidlConversion.h>

#include <media/AudioRecord.h>
@@ -63,13 +62,15 @@ std::string dumpPatch(const audio_patch& patch); 


class OnAudioDeviceUpdateNotifier : public AudioSystem::AudioDeviceCallback {

  public:

    audio_io_handle_t mAudioIo = AUDIO_IO_HANDLE_NONE;

    audio_port_handle_t mDeviceId = AUDIO_PORT_HANDLE_NONE;

    std::mutex mMutex;

    std::condition_variable mCondition;



    void onAudioDeviceUpdate(audio_io_handle_t audioIo, audio_port_handle_t deviceId);

    void onAudioDeviceUpdate(audio_io_handle_t audioIo, audio_port_handle_t deviceId) override;

    status_t waitForAudioDeviceCb(audio_port_handle_t expDeviceId = AUDIO_PORT_HANDLE_NONE);

    std::pair<audio_io_handle_t, audio_port_handle_t> getLastPortAndDevice() const;



  private:

    audio_io_handle_t mAudioIo GUARDED_BY(mMutex) = AUDIO_IO_HANDLE_NONE;

    audio_port_handle_t mDeviceId GUARDED_BY(mMutex) = AUDIO_PORT_HANDLE_NONE;

    mutable std::mutex mMutex;

    std::condition_variable mCondition;

};



// Simple AudioPlayback class.
@@ -86,15 +87,14 @@ class AudioPlayback : public AudioTrack::IAudioTrackCallback { 
    status_t create();

    sp<AudioTrack> getAudioTrackHandle();

    status_t start();

    status_t waitForConsumption(bool testSeek = false);

    status_t waitForConsumption(bool testSeek = false) EXCLUDES(mMutex);

    status_t fillBuffer();

    status_t onProcess(bool testSeek = false);

    virtual void onBufferEnd() override;

    void stop();

    void onBufferEnd() override EXCLUDES(mMutex);

    void stop() EXCLUDES(mMutex);



    bool mStopPlaying;

    std::mutex mMutex;

    std::condition_variable mCondition;

    bool mStopPlaying GUARDED_BY(mMutex);

    mutable std::mutex mMutex;



    enum State {

        PLAY_NO_INIT,
@@ -144,7 +144,7 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback { 
                 AudioRecord::transfer_type transferType = AudioRecord::TRANSFER_CALLBACK,

                 const audio_attributes_t* attributes = nullptr);

    ~AudioCapture();

    size_t onMoreData(const AudioRecord::Buffer& buffer) override;

    size_t onMoreData(const AudioRecord::Buffer& buffer) override EXCLUDES(mMutex);

    void onOverrun() override;

    void onMarker(uint32_t markerPosition) override;

    void onNewPos(uint32_t newPos) override;
@@ -156,10 +156,10 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback { 
    sp<AudioRecord> getAudioRecordHandle();

    status_t start(AudioSystem::sync_event_t event = AudioSystem::SYNC_EVENT_NONE,

                   audio_session_t triggerSession = AUDIO_SESSION_NONE);

    status_t obtainBufferCb(RawBuffer& buffer);

    status_t obtainBufferCb(RawBuffer& buffer) EXCLUDES(mMutex);

    status_t obtainBuffer(RawBuffer& buffer);

    status_t audioProcess();

    status_t stop();

    status_t stop() EXCLUDES(mMutex);



    uint32_t mFrameCount;

    uint32_t mNotificationFrames;
@@ -192,13 +192,13 @@ class AudioCapture : public AudioRecord::IAudioRecordCallback { 
    size_t mMaxBytesPerCallback = 2048;

    sp<AudioRecord> mRecord;

    State mState;

    bool mStopRecording;

    bool mStopRecording GUARDED_BY(mMutex);

    std::string mFileName;

    int mOutFileFd = -1;



    std::mutex mMutex;

    mutable std::mutex mMutex;

    std::condition_variable mCondition;

    std::deque<RawBuffer> mBuffersReceived;

    std::deque<RawBuffer> mBuffersReceived GUARDED_BY(mMutex);

};



#endif  // AUDIO_TEST_UTILS_H_

media/libaudioclient/tests/audiorecord_tests.cpp

+8 −4
Original line number Diff line number Diff line
@@ -101,10 +101,12 @@ TEST_F(AudioRecordTest, TestAudioCbNotifier) { 
    EXPECT_EQ(OK, mAC->getAudioRecordHandle()->addAudioDeviceCallback(cb));

    EXPECT_EQ(OK, mAC->start()) << "record creation failed";

    EXPECT_EQ(OK, cb->waitForAudioDeviceCb());

    EXPECT_EQ(AUDIO_IO_HANDLE_NONE, cbOld->mAudioIo);

    EXPECT_EQ(AUDIO_PORT_HANDLE_NONE, cbOld->mDeviceId);

    EXPECT_NE(AUDIO_IO_HANDLE_NONE, cb->mAudioIo);

    EXPECT_NE(AUDIO_PORT_HANDLE_NONE, cb->mDeviceId);

    const auto [oldAudioIo, oldDeviceId] = cbOld->getLastPortAndDevice();

    EXPECT_EQ(AUDIO_IO_HANDLE_NONE, oldAudioIo);

    EXPECT_EQ(AUDIO_PORT_HANDLE_NONE, oldDeviceId);

    const auto [audioIo, deviceId] = cb->getLastPortAndDevice();

    EXPECT_NE(AUDIO_IO_HANDLE_NONE, audioIo);

    EXPECT_NE(AUDIO_PORT_HANDLE_NONE, deviceId);

    EXPECT_EQ(BAD_VALUE, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(nullptr));

    EXPECT_EQ(INVALID_OPERATION, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cbOld));

    EXPECT_EQ(OK, mAC->getAudioRecordHandle()->removeAudioDeviceCallback(cb));
@@ -155,6 +157,7 @@ TEST_F(AudioRecordTest, TestGetSetMarker) { 
            << "getMarkerPosition() failed";

    EXPECT_EQ(OK, mAC->start()) << "start recording failed";

    EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";

    // TODO(b/348658586): Properly synchronize callback updates with the test thread.

    EXPECT_EQ(marker, mAC->mMarkerPosition)

            << "configured marker and received marker are different";

    EXPECT_EQ(mAC->mReceivedCbMarkerAtPosition, mAC->mMarkerPosition)
@@ -170,6 +173,7 @@ TEST_F(AudioRecordTest, TestGetSetMarkerPeriodical) { 
            << "getPositionUpdatePeriod() failed";

    EXPECT_EQ(OK, mAC->start()) << "start recording failed";

    EXPECT_EQ(OK, mAC->audioProcess()) << "audioProcess failed";

    // TODO(b/348658586): Properly synchronize callback updates with the test thread.

    EXPECT_EQ(marker, mAC->mMarkerPeriod) << "configured marker and received marker are different";

    EXPECT_EQ(mAC->mReceivedCbMarkerCount, mAC->mNumFramesToRecord / mAC->mMarkerPeriod)

            << "configured marker and received cb marker are different";

media/libaudioclient/tests/audiorouting_tests.cpp

+4 −3
Original line number Diff line number Diff line
@@ -64,16 +64,17 @@ TEST(AudioTrackTest, TestPerformanceMode) { 
        EXPECT_EQ(OK, ap->start()) << "audio track start failed";

        EXPECT_EQ(OK, ap->onProcess());

        EXPECT_EQ(OK, cb->waitForAudioDeviceCb());

        EXPECT_TRUE(checkPatchPlayback(cb->mAudioIo, cb->mDeviceId));

        const auto [audioIo, deviceId] = cb->getLastPortAndDevice();

        EXPECT_TRUE(checkPatchPlayback(audioIo, deviceId));

        EXPECT_NE(0, ap->getAudioTrackHandle()->getFlags() & output_flags[i]);

        audio_patch patch;

        EXPECT_EQ(OK, getPatchForOutputMix(cb->mAudioIo, patch));

        EXPECT_EQ(OK, getPatchForOutputMix(audioIo, patch));

        if (output_flags[i] != AUDIO_OUTPUT_FLAG_FAST) {

            // A "normal" output can still have a FastMixer, depending on the buffer size.

            // Thus, a fast track can be created on a mix port which does not have the FAST flag.

            for (auto j = 0; j < patch.num_sources; j++) {

                if (patch.sources[j].type == AUDIO_PORT_TYPE_MIX &&

                    patch.sources[j].ext.mix.handle == cb->mAudioIo) {

                    patch.sources[j].ext.mix.handle == audioIo) {

                    SCOPED_TRACE(dumpPortConfig(patch.sources[j]));

                    EXPECT_NE(0, patch.sources[j].flags.output & output_flags[i])

                            << "expected output flag "