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

Commit e2084703 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

libaudiohal@aidl: Fix position and latency reporting 

For latency, use the last returned value if the current
command did not provide latency (not every command does).
This avoids "Suspicious latency value reported by HAL..."
warnings.

Make sure that position and timestamp values are non-negative
(HAL may left them initialized to the default value `-1`).

Replace `if (status = ...; status != OK) return status` with
`RETURN_STATUS_IF_ERROR`. This also fixes `getRenderPosition`
which was returning `OK`.

Bug: 302132812
Bug: 302519087
Test: atest CtsMediaAudioTestCases
Change-Id: I5780eafd558bca85e966aa8374573df6a4133b9a
parent dcd31c41

media/libaudiohal/impl/StreamHalAidl.cpp

+29 −56
Original line number Diff line number Diff line
@@ -193,7 +193,7 @@ status_t StreamHalAidl::standby() { 
    StreamDescriptor::Reply reply;

    switch (state) {

        case StreamDescriptor::State::ACTIVE:

            if (status_t status = pause(&reply); status != OK) return status;

            RETURN_STATUS_IF_ERROR(pause(&reply));

            if (reply.state != StreamDescriptor::State::PAUSED) {

                ALOGE("%s: unexpected stream state: %s (expected PAUSED)",

                        __func__, toString(reply.state).c_str());
@@ -203,7 +203,7 @@ status_t StreamHalAidl::standby() { 
        case StreamDescriptor::State::PAUSED:

        case StreamDescriptor::State::DRAIN_PAUSED:

            if (mIsInput) return flush();

            if (status_t status = flush(&reply); status != OK) return status;

            RETURN_STATUS_IF_ERROR(flush(&reply));

            if (reply.state != StreamDescriptor::State::IDLE) {

                ALOGE("%s: unexpected stream state: %s (expected IDLE)",

                        __func__, toString(reply.state).c_str());
@@ -211,10 +211,8 @@ status_t StreamHalAidl::standby() { 
            }

            FALLTHROUGH_INTENDED;

        case StreamDescriptor::State::IDLE:

            if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::standby>(),

                            &reply, true /*safeFromNonWorkerThread*/); status != OK) {

                return status;

            }

            RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::standby>(),

                            &reply, true /*safeFromNonWorkerThread*/));

            if (reply.state != StreamDescriptor::State::STANDBY) {

                ALOGE("%s: unexpected stream state: %s (expected STANDBY)",

                        __func__, toString(reply.state).c_str());
@@ -244,10 +242,7 @@ status_t StreamHalAidl::start() { 
    const auto state = getState();

    StreamDescriptor::Reply reply;

    if (state == StreamDescriptor::State::STANDBY) {

        if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply, true);

                status != OK) {

            return status;

        }

        RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply, true));

        return sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), &reply, true);

    }
@@ -264,10 +259,7 @@ status_t StreamHalAidl::getLatency(uint32_t *latency) { 
    ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);

    if (!mStream) return NO_INIT;

    StreamDescriptor::Reply reply;

    if (status_t status = updateCountersIfNeeded(&reply); status != OK) {

        return status;

    }



    RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));

    *latency = std::clamp(std::max<int32_t>(0, reply.latencyMs), 1, 3000);

    ALOGW_IF(reply.latencyMs != static_cast<int32_t>(*latency),

             "Suspicious latency value reported by HAL: %d, clamped to %u", reply.latencyMs,
@@ -279,11 +271,9 @@ status_t StreamHalAidl::getObservablePosition(int64_t *frames, int64_t *timestam 
    ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);

    if (!mStream) return NO_INIT;

    StreamDescriptor::Reply reply;

    if (status_t status = updateCountersIfNeeded(&reply); status != OK) {

        return status;

    }

    *frames = reply.observable.frames;

    *timestamp = reply.observable.timeNs;

    RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));

    *frames = std::max<int64_t>(0, reply.observable.frames);

    *timestamp = std::max<int64_t>(0, reply.observable.timeNs);

    return OK;

}
@@ -292,12 +282,9 @@ status_t StreamHalAidl::getHardwarePosition(int64_t *frames, int64_t *timestamp) 
    if (!mStream) return NO_INIT;

    StreamDescriptor::Reply reply;

    // TODO: switch to updateCountersIfNeeded once we sort out mWorkerTid initialization

    if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), &reply, true);

            status != OK) {

        return status;

    }

    *frames = reply.hardware.frames;

    *timestamp = reply.hardware.timeNs;

    RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::getStatus>(), &reply, true));

    *frames = std::max<int64_t>(0, reply.hardware.frames);

    *timestamp = std::max<int64_t>(0, reply.hardware.timeNs);

    return OK;

}
@@ -305,10 +292,8 @@ status_t StreamHalAidl::getXruns(int32_t *frames) { 
    ALOGV("%p %s::%s", this, getClassName().c_str(), __func__);

    if (!mStream) return NO_INIT;

    StreamDescriptor::Reply reply;

    if (status_t status = updateCountersIfNeeded(&reply); status != OK) {

        return status;

    }

    *frames = reply.xrunFrames;

    RETURN_STATUS_IF_ERROR(updateCountersIfNeeded(&reply));

    *frames = std::max<int32_t>(0, reply.xrunFrames);

    return OK;

}
@@ -323,10 +308,7 @@ status_t StreamHalAidl::transfer(void *buffer, size_t bytes, size_t *transferred 
    // stream state), however this scenario wasn't supported by the HIDL HAL.

    if (getState() == StreamDescriptor::State::STANDBY) {

        StreamDescriptor::Reply reply;

        if (status_t status = sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply);

                status != OK) {

            return status;

        }

        RETURN_STATUS_IF_ERROR(sendCommand(makeHalCommand<HalCommand::Tag::start>(), &reply));

        if (reply.state != StreamDescriptor::State::IDLE) {

            ALOGE("%s: failed to get the stream out of standby, actual state: %s",

                    __func__, toString(reply.state).c_str());
@@ -345,9 +327,7 @@ status_t StreamHalAidl::transfer(void *buffer, size_t bytes, size_t *transferred 
        }

    }

    StreamDescriptor::Reply reply;

    if (status_t status = sendCommand(burst, &reply); status != OK) {

        return status;

    }

    RETURN_STATUS_IF_ERROR(sendCommand(burst, &reply));

    *transferred = reply.fmqByteCount;

    if (mIsInput) {

        LOG_ALWAYS_FATAL_IF(*transferred > bytes,
@@ -385,11 +365,8 @@ status_t StreamHalAidl::resume(StreamDescriptor::Reply* reply) { 
            if (state == StreamDescriptor::State::IDLE) {

                StreamDescriptor::Reply localReply{};

                StreamDescriptor::Reply* innerReply = reply ?: &localReply;

                if (status_t status =

                        sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), innerReply);

                        status != OK) {

                    return status;

                }

                RETURN_STATUS_IF_ERROR(

                        sendCommand(makeHalCommand<HalCommand::Tag::burst>(0), innerReply));

                if (innerReply->state != StreamDescriptor::State::ACTIVE) {

                    ALOGE("%s: unexpected stream state: %s (expected ACTIVE)",

                            __func__, toString(innerReply->state).c_str());
@@ -452,10 +429,7 @@ status_t StreamHalAidl::getMmapPosition(struct audio_mmap_position *position) { 
        return BAD_VALUE;

    }

    int64_t aidlPosition = 0, aidlTimestamp = 0;

    if (status_t status = getHardwarePosition(&aidlPosition, &aidlTimestamp); status != OK) {

        return status;

    }



    RETURN_STATUS_IF_ERROR(getHardwarePosition(&aidlPosition, &aidlTimestamp));

    position->time_nanoseconds = aidlTimestamp;

    position->position_frames = static_cast<int32_t>(aidlPosition);

    return OK;
@@ -503,6 +477,11 @@ status_t StreamHalAidl::sendCommand( 
    }

    {

        std::lock_guard l(mLock);

        // Not every command replies with 'latencyMs' field filled out, substitute the last

        // returned value in that case.

        if (reply->latencyMs <= 0) {

            reply->latencyMs = mLastReply.latencyMs;

        }

        mLastReply = *reply;

    }

    switch (reply->status) {
@@ -608,10 +587,8 @@ status_t StreamOutHalAidl::getRenderPosition(uint32_t *dspFrames) { 
        return BAD_VALUE;

    }

    int64_t aidlFrames = 0, aidlTimestamp = 0;

    if (status_t status = getObservablePosition(&aidlFrames, &aidlTimestamp); status != OK) {

        return OK;

    }

    *dspFrames = std::clamp<int64_t>(aidlFrames, 0, UINT32_MAX);

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

    *dspFrames = static_cast<uint32_t>(aidlFrames);

    return OK;

}
@@ -680,10 +657,8 @@ status_t StreamOutHalAidl::getPresentationPosition(uint64_t *frames, struct time 
        return BAD_VALUE;

    }

    int64_t aidlFrames = 0, aidlTimestamp = 0;

    if (status_t status = getObservablePosition(&aidlFrames, &aidlTimestamp); status != OK) {

        return status;

    }

    *frames = std::max<int64_t>(0, aidlFrames);

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

    *frames = aidlFrames;

    timestamp->tv_sec = aidlTimestamp / NANOS_PER_SECOND;

    timestamp->tv_nsec = aidlTimestamp - timestamp->tv_sec * NANOS_PER_SECOND;

    return OK;
@@ -901,9 +876,7 @@ status_t StreamInHalAidl::getInputFramesLost(uint32_t *framesLost) { 
        return BAD_VALUE;

    }

    int32_t aidlXruns = 0;

    if (status_t status = getXruns(&aidlXruns); status != OK) {

        return status;

    }

    RETURN_STATUS_IF_ERROR(getXruns(&aidlXruns));

    *framesLost = std::max<int32_t>(0, aidlXruns);

    return OK;

}

media/libaudiohal/impl/StreamHalAidl.h

+3 −0
Original line number Diff line number Diff line
@@ -207,10 +207,13 @@ class StreamHalAidl : public virtual StreamHalInterface, public ConversionHelper 


    status_t getLatency(uint32_t *latency);



    // Always returns non-negative values.

    status_t getObservablePosition(int64_t *frames, int64_t *timestamp);



    // Always returns non-negative values.

    status_t getHardwarePosition(int64_t *frames, int64_t *timestamp);



    // Always returns non-negative values.

    status_t getXruns(int32_t *frames);



    status_t transfer(void *buffer, size_t bytes, size_t *transferred);