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

Commit 6d7b119a authored by Andy Hung's avatar Andy Hung
Browse files

Improve timestamp jitter fix

Use last valid kernel timestamp on server side, not client side.

Bug: 28465713
Change-Id: I33590d6922980f288355f947bc56279245058429
parent 0b667e2f
Loading
Loading
Loading
Loading
+10 −3
Original line number Diff line number Diff line
@@ -37,9 +37,16 @@ public:
struct ExtendedTimestamp {
    enum Location {
        LOCATION_INVALID = -1,
        LOCATION_CLIENT,   // timestamp of last read frame from client-server track buffer
        LOCATION_SERVER,   // timestamp of newest frame from client-server track buffer
        // Locations in the audio playback / record pipeline.
        LOCATION_CLIENT,   // timestamp of last read frame from client-server track buffer.
        LOCATION_SERVER,   // timestamp of newest frame from client-server track buffer.
        LOCATION_KERNEL,   // timestamp of newest frame in the kernel (alsa) buffer.

        // Historical data: info when the kernel timestamp was OK (prior to the newest frame).
        // This may be useful when the newest frame kernel timestamp is unavailable.
        // Available for playback timestamps.
        LOCATION_SERVER_LASTKERNELOK, // timestamp of server the prior time kernel timestamp OK.
        LOCATION_KERNEL_LASTKERNELOK, // timestamp of kernel the prior time kernel timestamp OK.
        LOCATION_MAX       // for sizing arrays only
    };

@@ -101,7 +108,7 @@ struct ExtendedTimestamp {
        // look for the closest-to-hw stage in the pipeline with a valid timestamp.
        // We omit LOCATION_CLIENT as we prefer at least LOCATION_SERVER based accuracy
        // when getting the best timestamp.
        for (int i = LOCATION_MAX - 1; i >= LOCATION_SERVER; --i) {
        for (int i = LOCATION_KERNEL; i >= LOCATION_SERVER; --i) {
            if (mTimeNs[i] > 0) {
                *position = mPosition[i];
                *time = mTimeNs[i] + mTimebaseOffset[timebase];
+0 −1
Original line number Diff line number Diff line
@@ -1047,7 +1047,6 @@ protected:
    bool                    mRetrogradeMotionReported; // reduce log spam
    AudioTimestamp          mPreviousTimestamp;     // used to detect retrograde motion
    ExtendedTimestamp::Location mPreviousLocation;  // location used for previous timestamp
    double                  mComputedLatencyMs;     // latency between server and kernel

    uint32_t                mUnderrunCountOffset;   // updated when restoring tracks

+10 −15
Original line number Diff line number Diff line
@@ -536,7 +536,6 @@ status_t AudioTrack::set(
    mTimestampStartupGlitchReported = false;
    mRetrogradeMotionReported = false;
    mPreviousLocation = ExtendedTimestamp::LOCATION_INVALID;
    mComputedLatencyMs = 0.;
    mUnderrunCountOffset = 0;
    mFramesWritten = 0;
    mFramesWrittenServerOffset = 0;
@@ -570,7 +569,6 @@ status_t AudioTrack::start()
        mTimestampStartupGlitchReported = false;
        mRetrogradeMotionReported = false;
        mPreviousLocation = ExtendedTimestamp::LOCATION_INVALID;
        mComputedLatencyMs = 0.;

        // read last server side position change via timestamp.
        ExtendedTimestamp ets;
@@ -2375,25 +2373,22 @@ status_t AudioTrack::getTimestamp(AudioTimestamp& timestamp)
                if (location == ExtendedTimestamp::LOCATION_SERVER) {
                    ALOGW_IF(mPreviousLocation == ExtendedTimestamp::LOCATION_KERNEL,
                            "getTimestamp() location moved from kernel to server");
                    const double latencyMs = mComputedLatencyMs > 0.
                            ? mComputedLatencyMs : mAfLatency;
                    const int64_t frames =
                            int64_t(latencyMs * mSampleRate * mPlaybackRate.mSpeed / 1000);
                    ALOGV("mComputedLatencyMs:%lf  mAfLatency:%u  frame adjustment:%lld",
                            mComputedLatencyMs, mAfLatency, (long long)frames);
                            (ets.mTimeNs[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] < 0 ||
                            ets.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] < 0)
                            ?
                            int64_t((double)mAfLatency * mSampleRate * mPlaybackRate.mSpeed
                                    / 1000)
                            :
                            (ets.mPosition[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK]
                            - ets.mPosition[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK]);
                    ALOGV("frame adjustment:%lld  timestamp:%s",
                            (long long)frames, ets.toString().c_str());
                    if (frames >= ets.mPosition[location]) {
                        timestamp.mPosition = 0;
                    } else {
                        timestamp.mPosition = (uint32_t)(ets.mPosition[location] - frames);
                    }
                } else if (location == ExtendedTimestamp::LOCATION_KERNEL) {
                    const double bufferDiffMs =
                            (double)(ets.mPosition[ExtendedTimestamp::LOCATION_SERVER]
                                   - ets.mPosition[ExtendedTimestamp::LOCATION_KERNEL])
                                   * 1000 / ((double)mSampleRate * mPlaybackRate.mSpeed);
                    mComputedLatencyMs = bufferDiffMs > 0. ? bufferDiffMs : 0.;
                    ALOGV("mComputedLatencyMs:%lf  mAfLatency:%d",
                            mComputedLatencyMs, mAfLatency);
                }
                mPreviousLocation = location;
            } else {
+18 −0
Original line number Diff line number Diff line
@@ -2899,6 +2899,24 @@ bool AudioFlinger::PlaybackThread::threadLoop()
                // sink will block whie writing.
                ExtendedTimestamp timestamp; // use private copy to fetch
                (void) mNormalSink->getTimestamp(timestamp);

                // We keep track of the last valid kernel position in case we are in underrun
                // and the normal mixer period is the same as the fast mixer period, or there
                // is some error from the HAL.
                if (mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL] >= 0) {
                    mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] =
                            mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL];
                    mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL_LASTKERNELOK] =
                            mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_KERNEL];

                    mTimestamp.mPosition[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] =
                            mTimestamp.mPosition[ExtendedTimestamp::LOCATION_SERVER];
                    mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_SERVER_LASTKERNELOK] =
                            mTimestamp.mTimeNs[ExtendedTimestamp::LOCATION_SERVER];
                } else {
                    ALOGV("getTimestamp error - no valid kernel position");
                }

                // copy over kernel info
                mTimestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL] =
                        timestamp.mPosition[ExtendedTimestamp::LOCATION_KERNEL];
+1 −1
Original line number Diff line number Diff line
@@ -1103,7 +1103,7 @@ void AudioFlinger::PlaybackThread::Track::updateTrackFrameInfo(
        if (local.mTimeNs[i] > 0) {
            local.mPosition[i] = mFrameMap.findX(local.mPosition[i]);
            // check drain state from the latest stage in the pipeline.
            if (!checked) {
            if (!checked && i <= ExtendedTimestamp::LOCATION_KERNEL) {
                mAudioTrackServerProxy->setDrained(
                        local.mPosition[i] >= mAudioTrackServerProxy->framesReleased());
                checked = true;