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

Commit 1b42097f authored by Phil Burk's avatar Phil Burk
Browse files

AudioTrack: fix spurious retrograde messages 



The retrograde motion was confused by some positions coming from
the DSP on offloaded tracks. So the retrograde check was moved up
into AudioTrack.cpp. This allows us to take advantage of the checks
for invalid positions based on timing.

Bug: 2047891
Change-Id: Ifcad2349201443a7f1711347c203297100449536
Signed-off-by: default avatarPhil Burk <philburk@google.com>
parent 5f88cdae

include/media/AudioTrack.h

+3 −0
Original line number Diff line number Diff line
@@ -832,6 +832,9 @@ protected: 
    int64_t                 mStartUs;               // the start time after flush or stop.

                                                    // only used for offloaded and direct tracks.



    bool                    mPreviousTimestampValid;// true if mPreviousTimestamp is valid

    AudioTimestamp          mPreviousTimestamp;     // used to detect retrograde motion



    audio_output_flags_t    mFlags;

        // const after set(), except for bits AUDIO_OUTPUT_FLAG_FAST and AUDIO_OUTPUT_FLAG_OFFLOAD.

        // mLock must be held to read or write those bits reliably.

media/libmedia/AudioTrack.cpp

+44 −0
Original line number Diff line number Diff line
@@ -470,6 +470,7 @@ status_t AudioTrack::set( 
    mSequence = 1;

    mObservedSequence = mSequence;

    mInUnderrun = false;

    mPreviousTimestampValid = false;



    return NO_ERROR;

}
@@ -496,6 +497,8 @@ status_t AudioTrack::start() 
    if (previousState == STATE_STOPPED || previousState == STATE_FLUSHED) {

        // reset current position as seen by client to 0

        mPosition = 0;

        mPreviousTimestampValid = false;



        // For offloaded tracks, we don't know if the hardware counters are really zero here,

        // since the flush is asynchronous and stop may not fully drain.

        // We save the time when the track is started to later verify whether
@@ -995,6 +998,7 @@ status_t AudioTrack::reload() 
    mNewPosition = mUpdatePeriod;

    (void) updateAndGetPosition_l();

    mPosition = 0;

    mPreviousTimestampValid = false;

#if 0

    // The documentation is not clear on the behavior of reload() and the restoration

    // of loop count. Historically we have not restored loop count, start, end,
@@ -2089,6 +2093,11 @@ status_t AudioTrack::setParameters(const String8& keyValuePairs) 
status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)

{

    AutoMutex lock(mLock);



    bool previousTimestampValid = mPreviousTimestampValid;

    // Set false here to cover all the error return cases.

    mPreviousTimestampValid = false;



    // FIXME not implemented for fast tracks; should use proxy and SSQ

    if (mFlags & AUDIO_OUTPUT_FLAG_FAST) {

        return INVALID_OPERATION;
@@ -2187,6 +2196,41 @@ status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp) 
        // IAudioTrack.  And timestamp.mPosition is initially in server's

        // point of view, so we need to apply the same fudge factor to it.

    }



    // Prevent retrograde motion in timestamp.

    // This is sometimes caused by erratic reports of the available space in the ALSA drivers.

    if (status == NO_ERROR) {

        if (previousTimestampValid) {

#define TIME_TO_NANOS(time) ((uint64_t)time.tv_sec * 1000000000 + time.tv_nsec)

            const uint64_t previousTimeNanos = TIME_TO_NANOS(mPreviousTimestamp.mTime);

            const uint64_t currentTimeNanos = TIME_TO_NANOS(timestamp.mTime);

#undef TIME_TO_NANOS

            if (currentTimeNanos < previousTimeNanos) {

                ALOGW("retrograde timestamp time");

                // FIXME Consider blocking this from propagating upwards.

            }



            // Looking at signed delta will work even when the timestamps

            // are wrapping around.

            int32_t deltaPosition = static_cast<int32_t>(timestamp.mPosition

                    - mPreviousTimestamp.mPosition);

            // position can bobble slightly as an artifact; this hides the bobble

            static const int32_t MINIMUM_POSITION_DELTA = 8;

            ALOGW_IF(deltaPosition < 0,

                    "retrograde timestamp position corrected, %d = %u - %u, (at %llu, %llu nanos)",

                    deltaPosition,

                    timestamp.mPosition,

                    mPreviousTimestamp.mPosition,

                    currentTimeNanos,

                    previousTimeNanos);

            if (deltaPosition < MINIMUM_POSITION_DELTA) {

                timestamp = mPreviousTimestamp;  // Use last valid timestamp.

            }

        }

        mPreviousTimestamp = timestamp;

        mPreviousTimestampValid = true;

    }



    return status;

}

services/audioflinger/PlaybackTracks.h

+0 −5
Original line number Diff line number Diff line
@@ -156,11 +156,6 @@ private: 
    bool                mResumeToStopping; // track was paused in stopping state.

    bool                mFlushHwPending; // track requests for thread flush



    // for last call to getTimestamp

    bool                mPreviousTimestampValid;

    // This is either the first timestamp or one that has passed

    // the check to prevent retrograde motion.

    AudioTimestamp      mPreviousTimestamp;

};  // end of Track



class TimedTrack : public Track {

services/audioflinger/Tracks.cpp

+2 −45
Original line number Diff line number Diff line
@@ -404,8 +404,7 @@ AudioFlinger::PlaybackThread::Track::Track( 
    mIsInvalid(false),

    mAudioTrackServerProxy(NULL),

    mResumeToStopping(false),

    mFlushHwPending(false),

    mPreviousTimestampValid(false)

    mFlushHwPending(false)

{

    // client == 0 implies sharedBuffer == 0

    ALOG_ASSERT(!(client == 0 && sharedBuffer != 0));
@@ -863,7 +862,6 @@ void AudioFlinger::PlaybackThread::Track::reset() 
        if (mState == FLUSHED) {

            mState = IDLE;

        }

        mPreviousTimestampValid = false;

    }

}
@@ -885,12 +883,10 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times 
{

    // Client should implement this using SSQ; the unpresented frame count in latch is irrelevant

    if (isFastTrack()) {

        // FIXME no lock held to set mPreviousTimestampValid = false

        return INVALID_OPERATION;

    }

    sp<ThreadBase> thread = mThread.promote();

    if (thread == 0) {

        // FIXME no lock held to set mPreviousTimestampValid = false

        return INVALID_OPERATION;

    }
@@ -900,7 +896,6 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times 
    status_t result = INVALID_OPERATION;

    if (!isOffloaded() && !isDirect()) {

        if (!playbackThread->mLatchQValid) {

            mPreviousTimestampValid = false;

            return INVALID_OPERATION;

        }

        // FIXME Not accurate under dynamic changes of sample rate and speed.
@@ -919,10 +914,7 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times 
        uint32_t framesWritten = i >= 0 ?

                playbackThread->mLatchQ.mFramesReleased[i] :

                mAudioTrackServerProxy->framesReleased();

        if (framesWritten < unpresentedFrames) {

            mPreviousTimestampValid = false;

            // return invalid result

        } else {

        if (framesWritten >= unpresentedFrames) {

            timestamp.mPosition = framesWritten - unpresentedFrames;

            timestamp.mTime = playbackThread->mLatchQ.mTimestamp.mTime;

            result = NO_ERROR;
@@ -931,41 +923,6 @@ status_t AudioFlinger::PlaybackThread::Track::getTimestamp(AudioTimestamp& times 
        result = playbackThread->getTimestamp_l(timestamp);

    }



    // Prevent retrograde motion in timestamp.

    if (result == NO_ERROR) {

        if (mPreviousTimestampValid) {

            if (timestamp.mTime.tv_sec < mPreviousTimestamp.mTime.tv_sec ||

                    (timestamp.mTime.tv_sec == mPreviousTimestamp.mTime.tv_sec &&

                    timestamp.mTime.tv_nsec < mPreviousTimestamp.mTime.tv_nsec)) {

                ALOGW("WARNING - retrograde timestamp time");

                // FIXME Consider blocking this from propagating upwards.

            }



            // Looking at signed delta will work even when the timestamps

            // are wrapping around.

            int32_t deltaPosition = static_cast<int32_t>(timestamp.mPosition

                    - mPreviousTimestamp.mPosition);

            // position can bobble slightly as an artifact; this hides the bobble

            static const int32_t MINIMUM_POSITION_DELTA = 8;

            if (deltaPosition < 0) {

#define TIME_TO_NANOS(time) ((uint64_t)time.tv_sec * 1000000000 + time.tv_nsec)

                ALOGW("WARNING - retrograde timestamp position corrected,"

                        " %d = %u - %u, (at %llu, %llu nanos)",

                        deltaPosition,

                        timestamp.mPosition,

                        mPreviousTimestamp.mPosition,

                        TIME_TO_NANOS(timestamp.mTime),

                        TIME_TO_NANOS(mPreviousTimestamp.mTime));

#undef TIME_TO_NANOS

            }

            if (deltaPosition < MINIMUM_POSITION_DELTA) {

                // Current timestamp is bad. Use last valid timestamp.

                timestamp = mPreviousTimestamp;

            }

        }

        mPreviousTimestamp = timestamp;

        mPreviousTimestampValid = true;

    }

    return result;

}