Fix vorbis-specific workaround to only compare audio timestamps
This workaround was introduced in 2014 [1]. At the time it was correctly
only comparing audio timestamps because mAnchorTimeMediaUs only held
audio timestamps then. In 2017 [2], mAnchorTimeMediaUs started to hold
video timestamps too - at which point this comparison became invalid and
a bug was introduced that can lead to control-flow exiting too early
when handling an audio buffer with the same timestamp as the preceding
video buffer. If this happens at the start of playback, then the
**second** audio buffer timestamp is passed to
MediaClock::setStartingMediaTime, which can lead to incorrect values
being returned from NuPlayerRenderer::getCurrentPosition (when paused
at the start of playback).
[1] https://cs.android.com/android/_/android/platform/frameworks/av/+/eecb7805bbbb712925d4372c505f8c7f5c4fb5ed
[2] https://cs.android.com/android/_/android/platform/frameworks/av/+/17648f365e5914fa772bbfebaf9f5c6a37ddfb99
Test: atest MediaPlayerTest
Bug: 283882552
Bug: 282472894
Bug: 263063118
Change-Id: I7fbe6bdaff27c93f15d9d85902306a3f6e00b978
diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
index 57c4791..b37a131 100644
--- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
+++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
@@ -132,6 +132,7 @@
mMediaClock(mediaClock),
mPlaybackSettings(AUDIO_PLAYBACK_RATE_DEFAULT),
mAudioFirstAnchorTimeMediaUs(-1),
+ mAudioAnchorTimeMediaUs(-1),
mAnchorTimeMediaUs(-1),
mAnchorNumFramesWritten(-1),
mVideoLateByUs(0LL),
@@ -433,6 +434,7 @@
// Called on renderer looper.
void NuPlayer::Renderer::clearAnchorTime() {
mMediaClock->clearAnchor();
+ mAudioAnchorTimeMediaUs = -1;
mAnchorTimeMediaUs = -1;
mAnchorNumFramesWritten = -1;
}
@@ -1261,7 +1263,7 @@
Mutex::Autolock autoLock(mLock);
// TRICKY: vorbis decoder generates multiple frames with the same
// timestamp, so only update on the first frame with a given timestamp
- if (mediaTimeUs == mAnchorTimeMediaUs) {
+ if (mediaTimeUs == mAudioAnchorTimeMediaUs) {
return;
}
setAudioFirstAnchorTimeIfNeeded_l(mediaTimeUs);
@@ -1299,6 +1301,7 @@
}
}
mAnchorNumFramesWritten = mNumFramesWritten;
+ mAudioAnchorTimeMediaUs = mediaTimeUs;
mAnchorTimeMediaUs = mediaTimeUs;
}