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

Commit f450ced7 authored by Mikhail Naganov's avatar Mikhail Naganov Committed by Cherrypicker Worker
Browse files

Clean up HAL render position reporting 

Since AIDL HAL reports 64-bit position values, move the logic
for proper expansion of 32-bit positions from AudioStreamOut
into StreamOutHidl. Add synchronization which did not exist
in AudioStreamOut. This is because the calling code
does not always use same mutexes when calling into relevant
methods, thus data races are possible.

Also, clean up StreamOutHalInterface and AudioStreamOut
interface by removing obsolete methods.

Bug: 338557486
Test: atest CtsMediaAudioTestCases
(cherry picked from https://android-review.googlesource.com/q/commit:0ea58fe84db5c97a905dfbd0be718eef325b049e)
Merged-In: I30701af589f3721b3d8bf7886acc251ceefadc46
Change-Id: I30701af589f3721b3d8bf7886acc251ceefadc46
parent bcf7dcb8

media/libaudiohal/impl/StreamHalAidl.cpp

+7 −7
Original line number Diff line number Diff line
@@ -654,21 +654,16 @@ status_t StreamOutHalAidl::write(const void *buffer, size_t bytes, size_t *writt 
    return transfer(const_cast<void*>(buffer), bytes, written);

}



status_t StreamOutHalAidl::getRenderPosition(uint32_t *dspFrames) {

status_t StreamOutHalAidl::getRenderPosition(uint64_t *dspFrames) {

    if (dspFrames == nullptr) {

        return BAD_VALUE;

    }

    int64_t aidlFrames = 0, aidlTimestamp = 0;

    RETURN_STATUS_IF_ERROR(getObservablePosition(&aidlFrames, &aidlTimestamp));

    *dspFrames = static_cast<uint32_t>(aidlFrames);

    *dspFrames = aidlFrames;

    return OK;

}



status_t StreamOutHalAidl::getNextWriteTimestamp(int64_t *timestamp __unused) {

    // Obsolete, use getPresentationPosition.

    return INVALID_OPERATION;

}



status_t StreamOutHalAidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {

    ALOGD("%p %s", this, __func__);

    TIME_CHECK();
@@ -729,6 +724,11 @@ status_t StreamOutHalAidl::getPresentationPosition(uint64_t *frames, struct time 
    return OK;

}



status_t StreamOutHalAidl::presentationComplete() {

    ALOGD("%p %s::%s", this, getClassName().c_str(), __func__);

    return OK;

}



status_t StreamOutHalAidl::updateSourceMetadata(

        const StreamOutHalInterface::SourceMetadata& sourceMetadata) {

    TIME_CHECK();

media/libaudiohal/impl/StreamHalAidl.h

+4 −4
Original line number Diff line number Diff line
@@ -308,10 +308,7 @@ class StreamOutHalAidl : public virtual StreamOutHalInterface, 


    // Return the number of audio frames written by the audio dsp to DAC since

    // the output has exited standby.

    status_t getRenderPosition(uint32_t *dspFrames) override;



    // Get the local time at which the next write to the audio driver will be presented.

    status_t getNextWriteTimestamp(int64_t *timestamp) override;

    status_t getRenderPosition(uint64_t *dspFrames) override;



    // Set the callback for notifying completion of non-blocking write and drain.

    status_t setCallback(wp<StreamOutHalInterfaceCallback> callback) override;
@@ -337,6 +334,9 @@ class StreamOutHalAidl : public virtual StreamOutHalInterface, 
    // Return a recent count of the number of audio frames presented to an external observer.

    status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp) override;



    // Notifies the HAL layer that the framework considers the current playback as completed.

    status_t presentationComplete() override;



    // Called when the metadata of the stream's source has been changed.

    status_t updateSourceMetadata(const SourceMetadata& sourceMetadata) override;

media/libaudiohal/impl/StreamHalHidl.cpp

+50 −17
Original line number Diff line number Diff line
@@ -17,6 +17,8 @@ 
#define LOG_TAG "StreamHalHidl"

//#define LOG_NDEBUG 0



#include <cinttypes>



#include <android/hidl/manager/1.0/IServiceManager.h>

#include <hwbinder/IPCThreadState.h>

#include <media/AudioParameter.h>
@@ -589,32 +591,39 @@ status_t StreamOutHalHidl::prepareForWriting(size_t bufferSize) { 
    return OK;

}



status_t StreamOutHalHidl::getRenderPosition(uint32_t *dspFrames) {

status_t StreamOutHalHidl::getRenderPosition(uint64_t *dspFrames) {

    // TIME_CHECK();  // TODO(b/243839867) reenable only when optimized.

    if (mStream == 0) return NO_INIT;

    Result retval;

    uint32_t halPosition = 0;

    Return<void> ret = mStream->getRenderPosition(

            [&](Result r, uint32_t d) {

                retval = r;

                if (retval == Result::OK) {

                    *dspFrames = d;

                    halPosition = d;

                }

            });

    return processReturn("getRenderPosition", ret, retval);

}



status_t StreamOutHalHidl::getNextWriteTimestamp(int64_t *timestamp) {

    TIME_CHECK();

    if (mStream == 0) return NO_INIT;

    Result retval;

    Return<void> ret = mStream->getNextWriteTimestamp(

            [&](Result r, int64_t t) {

                retval = r;

                if (retval == Result::OK) {

                    *timestamp = t;

    status_t status = processReturn("getRenderPosition", ret, retval);

    if (status != OK) {

        return status;

    }

            });

    return processReturn("getRenderPosition", ret, retval);

    // Maintain a 64-bit render position using the 32-bit result from the HAL.

    // This delta calculation relies on the arithmetic overflow behavior

    // of integers. For example (100 - 0xFFFFFFF0) = 116.

    std::lock_guard l(mPositionMutex);

    const auto truncatedPosition = (uint32_t)mRenderPosition;

    int32_t deltaHalPosition; // initialization not needed, overwitten by __builtin_sub_overflow()

    (void) __builtin_sub_overflow(halPosition, truncatedPosition, &deltaHalPosition);



    if (deltaHalPosition >= 0) {

        mRenderPosition += deltaHalPosition;

    } else if (mExpectRetrograde) {

        mExpectRetrograde = false;

        mRenderPosition -= static_cast<uint64_t>(-deltaHalPosition);

        ALOGW("Retrograde motion of %" PRId32 " frames", -deltaHalPosition);

    }

    *dspFrames = mRenderPosition;

    return OK;

}



status_t StreamOutHalHidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {
@@ -667,9 +676,23 @@ status_t StreamOutHalHidl::drain(bool earlyNotify) { 
status_t StreamOutHalHidl::flush() {

    TIME_CHECK();

    if (mStream == 0) return NO_INIT;

    {

        std::lock_guard l(mPositionMutex);

        mRenderPosition = 0;

        mExpectRetrograde = false;

    }

    return processReturn("pause", mStream->flush());

}



status_t StreamOutHalHidl::standby() {

    {

        std::lock_guard l(mPositionMutex);

        mRenderPosition = 0;

        mExpectRetrograde = false;

    }

    return StreamHalHidl::standby();

}



status_t StreamOutHalHidl::getPresentationPosition(uint64_t *frames, struct timespec *timestamp) {

    // TIME_CHECK();  // TODO(b/243839867) reenable only when optimized.

    if (mStream == 0) return NO_INIT;
@@ -696,6 +719,16 @@ status_t StreamOutHalHidl::getPresentationPosition(uint64_t *frames, struct time 
    }

}



status_t StreamOutHalHidl::presentationComplete() {

    // Avoid suppressing retrograde motion in mRenderPosition for gapless offload/direct when

    // transitioning between tracks.

    // The HAL resets the frame position without flush/stop being called, but calls back prior to

    // this event. So, on the next occurrence of retrograde motion, we permit backwards movement of

    // mRenderPosition.

    mExpectRetrograde = true;

    return OK;

}



#if MAJOR_VERSION == 2

status_t StreamOutHalHidl::updateSourceMetadata(

        const StreamOutHalInterface::SourceMetadata& /* sourceMetadata */) {

media/libaudiohal/impl/StreamHalHidl.h

+13 −4
Original line number Diff line number Diff line
@@ -18,10 +18,12 @@ 
#define ANDROID_HARDWARE_STREAM_HAL_HIDL_H



#include <atomic>

#include <mutex>



#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStream.h)

#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStreamIn.h)

#include PATH(android/hardware/audio/FILE_VERSION/IStreamOut.h)

#include <android-base/thread_annotations.h>

#include <fmq/EventFlag.h>

#include <fmq/MessageQueue.h>

#include <media/audiohal/EffectHalInterface.h>
@@ -119,6 +121,9 @@ class StreamHalHidl : public virtual StreamHalInterface, public CoreConversionHe 


class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl {

  public:

    // Put the audio hardware input/output into standby mode (from StreamHalInterface).

    status_t standby() override;



    // Return the frame size (number of bytes per sample) of a stream.

    virtual status_t getFrameSize(size_t *size);
@@ -136,10 +141,7 @@ class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl { 


    // Return the number of audio frames written by the audio dsp to DAC since

    // the output has exited standby.

    virtual status_t getRenderPosition(uint32_t *dspFrames);



    // Get the local time at which the next write to the audio driver will be presented.

    virtual status_t getNextWriteTimestamp(int64_t *timestamp);

    virtual status_t getRenderPosition(uint64_t *dspFrames);



    // Set the callback for notifying completion of non-blocking write and drain.

    virtual status_t setCallback(wp<StreamOutHalInterfaceCallback> callback);
@@ -165,6 +167,9 @@ class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl { 
    // Return a recent count of the number of audio frames presented to an external observer.

    virtual status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp);



    // Notifies the HAL layer that the framework considers the current playback as completed.

    status_t presentationComplete() override;



    // Called when the metadata of the stream's source has been changed.

    status_t updateSourceMetadata(const SourceMetadata& sourceMetadata) override;
@@ -221,6 +226,10 @@ class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl { 
    std::unique_ptr<StatusMQ> mStatusMQ;

    std::atomic<pid_t> mWriterClient;

    EventFlag* mEfGroup;

    std::mutex mPositionMutex;

    // Used to expand correctly the 32-bit position from the HAL.

    uint64_t mRenderPosition GUARDED_BY(mPositionMutex) = 0;

    bool mExpectRetrograde GUARDED_BY(mPositionMutex) = false; // See 'presentationComplete'.



    // Can not be constructed directly by clients.

    StreamOutHalHidl(const sp<::android::hardware::audio::CPP_VERSION::IStreamOut>& stream);

media/libaudiohal/include/media/audiohal/StreamHalInterface.h

+4 −4
Original line number Diff line number Diff line
@@ -151,10 +151,7 @@ class StreamOutHalInterface : public virtual StreamHalInterface { 


    // Return the number of audio frames written by the audio dsp to DAC since

    // the output has exited standby.

    virtual status_t getRenderPosition(uint32_t *dspFrames) = 0;



    // Get the local time at which the next write to the audio driver will be presented.

    virtual status_t getNextWriteTimestamp(int64_t *timestamp) = 0;

    virtual status_t getRenderPosition(uint64_t *dspFrames) = 0;



    // Set the callback for notifying completion of non-blocking write and drain.

    // The callback must be owned by someone else. The output stream does not own it
@@ -182,6 +179,9 @@ class StreamOutHalInterface : public virtual StreamHalInterface { 
    // Return a recent count of the number of audio frames presented to an external observer.

    virtual status_t getPresentationPosition(uint64_t *frames, struct timespec *timestamp) = 0;



    // Notifies the HAL layer that the framework considers the current playback as completed.

    virtual status_t presentationComplete() = 0;



    struct SourceMetadata {

        std::vector<playback_track_metadata_v7_t> tracks;

    };