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

Commit c8e09c61 authored by Andy Hung's avatar Andy Hung
Browse files

Improve AudioTrack offload timestamp startup glitch detector

New or existing glitch behavior for Nexus 5 offload audio:
we receive several 0 timestamps,
then we get a stale timestamp (very large),
then a few ms later we get a correct nonzero timestamp.

We attempt to hide the glitch because the retrograde timestamp
correction makes the glitch "sticky".

Bug: 21633313
Change-Id: I39153af718c151f9435e7d315651a811f72743da
parent 005e9d03
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -894,6 +894,7 @@ protected:
                                                    // only used for offloaded and direct tracks.

    bool                    mPreviousTimestampValid;// true if mPreviousTimestamp is valid
    bool                    mTimestampStartupGlitchReported; // reduce log spam
    bool                    mRetrogradeMotionReported; // reduce log spam
    AudioTimestamp          mPreviousTimestamp;     // used to detect retrograde motion

+26 −4
Original line number Diff line number Diff line
@@ -492,6 +492,8 @@ status_t AudioTrack::set(
    mObservedSequence = mSequence;
    mInUnderrun = false;
    mPreviousTimestampValid = false;
    mTimestampStartupGlitchReported = false;
    mRetrogradeMotionReported = false;

    return NO_ERROR;
}
@@ -519,6 +521,8 @@ status_t AudioTrack::start()
        // reset current position as seen by client to 0
        mPosition = 0;
        mPreviousTimestampValid = false;
        mTimestampStartupGlitchReported = false;
        mRetrogradeMotionReported = 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.
@@ -2218,7 +2222,12 @@ status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)
        }

        // Check whether a pending flush or stop has completed, as those commands may
        // be asynchronous or return near finish.
        // be asynchronous or return near finish or exhibit glitchy behavior.
        //
        // Originally this showed up as the first timestamp being a continuation of
        // the previous song under gapless playback.
        // However, we sometimes see zero timestamps, then a glitch of
        // the previous song's position, and then correct timestamps afterwards.
        if (mStartUs != 0 && mSampleRate != 0) {
            static const int kTimeJitterUs = 100000; // 100 ms
            static const int k1SecUs = 1000000;
@@ -2236,16 +2245,29 @@ status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)

                if (deltaPositionByUs > deltaTimeUs + kTimeJitterUs) {
                    // Verify that the counter can't count faster than the sample rate
                    // since the start time.  If greater, then that means we have failed
                    // since the start time.  If greater, then that means we may have failed
                    // to completely flush or stop the previous playing track.
                    ALOGW("incomplete flush or stop:"
                    ALOGW_IF(!mTimestampStartupGlitchReported,
                            "getTimestamp startup glitch detected"
                            " deltaTimeUs(%lld) deltaPositionUs(%lld) tsmPosition(%u)",
                            (long long)deltaTimeUs, (long long)deltaPositionByUs,
                            timestamp.mPosition);
                    mTimestampStartupGlitchReported = true;
                    if (previousTimestampValid
                            && mPreviousTimestamp.mPosition == 0 /* should be true if valid */) {
                        timestamp = mPreviousTimestamp;
                        mPreviousTimestampValid = true;
                        return NO_ERROR;
                    }
                    return WOULD_BLOCK;
                }
                if (deltaPositionByUs != 0) {
                    mStartUs = 0; // don't check again, we got valid nonzero position.
                }
            } else {
                mStartUs = 0; // don't check again, start time expired.
            }
            mStartUs = 0; // no need to check again, start timestamp has either expired or unneeded.
            mTimestampStartupGlitchReported = false;
        }
    } else {
        // Update the mapping between local consumed (mPosition) and server consumed (mServer)