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

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

audiohal: Fix incompatibility with legacy hal for writes 

The combo call to stream_out->write + get_presentation_position
wasn't delivering the results of these calls entirely
correctly. Since the 'WriteResult' struct was lacking the field
for returning the status of the call to
'get_presentation_position', the client could erroneously update
the presentation position to bogus values if the call to the
legacy HAL had failed.

Updated IStreamOut.WriteStatus to include the missing field,
and updated the code to fill it out.

Also fixed logspam resulting from calling a stubbed
stream_in->get_capture_position.

Bug: 30222631
Test: Loopback RTT, media CTS
Change-Id: I38ac3b01beb095e176b54608e11e71ae5d5eafb6
parent df1d3b32

audio/2.0/IStreamOut.hal

+9 −2
Original line number Diff line number Diff line
@@ -47,14 +47,21 @@ interface IStreamOut extends IStream { 
     * Data structure passed back to the client via status message queue

     * of 'write' operation.

     *

     * Possible values of 'retval' field:

     * Possible values of 'writeRetval' field:

     *  - OK, write operation was successful;

     *  - INVALID_ARGUMENTS, stream was not configured properly;

     *  - INVALID_STATE, stream is in a state that doesn't allow writes.

     *

     * Possible values of 'presentationPositionRetval' field (must only

     * be considered if 'writeRetval' field is set to 'OK'):

     *  - OK, presentation position retrieved successfully;

     *  - INVALID_ARGUMENTS, indicates that the position can't be retrieved;

     *  - INVALID_OPERATION, retrieving presentation position isn't supported;

     */

    struct WriteStatus {

        Result retval;

        Result writeRetval;

        uint64_t written;

        Result presentationPositionRetval;

        uint64_t frames;    // presentation position

        TimeSpec timeStamp; // presentation position

    };

audio/2.0/default/StreamIn.cpp

+4 −1
Original line number Diff line number Diff line
@@ -330,7 +330,10 @@ Return<void> StreamIn::getCapturePosition(getCapturePosition_cb _hidl_cb) { 
        int64_t halFrames, halTime;

        retval = Stream::analyzeStatus(

                "get_capture_position",

                mStream->get_capture_position(mStream, &halFrames, &halTime));

                mStream->get_capture_position(mStream, &halFrames, &halTime),

                // HAL may have a stub function, always returning ENOSYS, don't

                // spam the log in this case.

                ENOSYS);

        if (retval == Result::OK) {

            frames = halFrames;

            time = halTime;

audio/2.0/default/StreamOut.cpp

+28 −26
Original line number Diff line number Diff line
@@ -88,24 +88,20 @@ bool WriteThread::threadLoop() { 
        }



        const size_t availToRead = mDataMQ->availableToRead();

        Result retval = Result::OK;

        uint64_t written = 0;

        IStreamOut::WriteStatus status;

        status.writeRetval = Result::OK;

        status.written = 0;

        if (mDataMQ->read(&mBuffer[0], availToRead)) {

            ssize_t writeResult = mStream->write(mStream, &mBuffer[0], availToRead);

            if (writeResult >= 0) {

                written = writeResult;

                status.written = writeResult;

            } else {

                retval = Stream::analyzeStatus("write", writeResult);

                status.writeRetval = Stream::analyzeStatus("write", writeResult);

            }

        }

        uint64_t frames = 0;

        struct timespec halTimeStamp = { 0, 0 };

        if (retval == Result::OK && mStream->get_presentation_position != NULL) {

            mStream->get_presentation_position(mStream, &frames, &halTimeStamp);

        }

        IStreamOut::WriteStatus status = { retval, written, frames,

                                           { static_cast<uint64_t>(halTimeStamp.tv_sec),

                                             static_cast<uint64_t>(halTimeStamp.tv_nsec) } };

        status.presentationPositionRetval = status.writeRetval == Result::OK ?

                StreamOut::getPresentationPositionImpl(mStream, &status.frames, &status.timeStamp) :

                Result::OK;

        if (!mStatusMQ->write(&status)) {

            ALOGW("status message queue write failed");

        }
@@ -399,23 +395,29 @@ Return<Result> StreamOut::flush() { 
            Result::NOT_SUPPORTED;

}



Return<void> StreamOut::getPresentationPosition(getPresentationPosition_cb _hidl_cb)  {

// static

Result StreamOut::getPresentationPositionImpl(

        audio_stream_out_t *stream, uint64_t *frames, TimeSpec *timeStamp) {

    Result retval(Result::NOT_SUPPORTED);

    uint64_t frames = 0;

    TimeSpec timeStamp = { 0, 0 };

    if (mStream->get_presentation_position != NULL) {

    if (stream->get_presentation_position == NULL) return retval;

    struct timespec halTimeStamp;

    retval = Stream::analyzeStatus(

            "get_presentation_position",

                mStream->get_presentation_position(mStream, &frames, &halTimeStamp),

            stream->get_presentation_position(stream, frames, &halTimeStamp),

            // Don't logspam on EINVAL--it's normal for get_presentation_position

            // to return it sometimes.

            EINVAL);

    if (retval == Result::OK) {

            timeStamp.tvSec = halTimeStamp.tv_sec;

            timeStamp.tvNSec = halTimeStamp.tv_nsec;

        timeStamp->tvSec = halTimeStamp.tv_sec;

        timeStamp->tvNSec = halTimeStamp.tv_nsec;

    }

    return retval;

}



Return<void> StreamOut::getPresentationPosition(getPresentationPosition_cb _hidl_cb)  {

    uint64_t frames = 0;

    TimeSpec timeStamp = { 0, 0 };

    Result retval = getPresentationPositionImpl(mStream, &frames, &timeStamp);

    _hidl_cb(retval, frames, timeStamp);

    return Void();

}

audio/2.0/default/StreamOut.h

+3 −0
Original line number Diff line number Diff line
@@ -108,6 +108,9 @@ struct StreamOut : public IStreamOut { 
    Return<void> createMmapBuffer(int32_t minSizeFrames, createMmapBuffer_cb _hidl_cb) override;

    Return<void> getMmapPosition(getMmapPosition_cb _hidl_cb) override;



    static Result getPresentationPositionImpl(

            audio_stream_out_t *stream, uint64_t *frames, TimeSpec *timeStamp);



  private:

    bool mIsClosed;

    audio_hw_device_t *mDevice;