Improve DefaultMediaClock behaviour.

DefaultMediaClock has currently two non-ideal behaviours:
1. One part of checking if it should use the renderer clock is checking whether
   the associated renderer finished reading its stream. This only makes sense
   if the renderer isn't already reading ahead into the next period. This can
   be solved by forwarding if we are reading ahead to the sync command.
2. When switching from stand-alone to renderer clock we assume they are
   exactly at the same position. This is true in theory, but in practise there
   may be small differences due to the different natures of these clocks. To
   prevent jumping backwards in time, we can temporarily stop the stand-alone
   clock and only switch once the renderer clock reached the same position.

PiperOrigin-RevId: 260690468
This commit is contained in:
tonihei 2019-07-30 12:47:31 +01:00 committed by Oliver Woodman
parent 39d5867c97
commit ce2e2797cb
3 changed files with 111 additions and 60 deletions

View file

@ -40,11 +40,13 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
void onPlaybackParametersChanged(PlaybackParameters newPlaybackParameters);
}
private final StandaloneMediaClock standaloneMediaClock;
private final StandaloneMediaClock standaloneClock;
private final PlaybackParameterListener listener;
@Nullable private Renderer rendererClockSource;
@Nullable private MediaClock rendererClock;
private boolean isUsingStandaloneClock;
private boolean standaloneClockIsStarted;
/**
* Creates a new instance with listener for playback parameter changes and a {@link Clock} to use
@ -56,21 +58,24 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
*/
public DefaultMediaClock(PlaybackParameterListener listener, Clock clock) {
this.listener = listener;
this.standaloneMediaClock = new StandaloneMediaClock(clock);
this.standaloneClock = new StandaloneMediaClock(clock);
isUsingStandaloneClock = true;
}
/**
* Starts the standalone fallback clock.
*/
public void start() {
standaloneMediaClock.start();
standaloneClockIsStarted = true;
standaloneClock.start();
}
/**
* Stops the standalone fallback clock.
*/
public void stop() {
standaloneMediaClock.stop();
standaloneClockIsStarted = false;
standaloneClock.stop();
}
/**
@ -79,7 +84,7 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
* @param positionUs The position to set in microseconds.
*/
public void resetPosition(long positionUs) {
standaloneMediaClock.resetPosition(positionUs);
standaloneClock.resetPosition(positionUs);
}
/**
@ -99,8 +104,7 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
}
this.rendererClock = rendererMediaClock;
this.rendererClockSource = renderer;
rendererClock.setPlaybackParameters(standaloneMediaClock.getPlaybackParameters());
ensureSynced();
rendererClock.setPlaybackParameters(standaloneClock.getPlaybackParameters());
}
}
@ -114,30 +118,25 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
if (renderer == rendererClockSource) {
this.rendererClock = null;
this.rendererClockSource = null;
isUsingStandaloneClock = true;
}
}
/**
* Syncs internal clock if needed and returns current clock position in microseconds.
*
* @param isReadingAhead Whether the renderers are reading ahead.
*/
public long syncAndGetPositionUs() {
if (isUsingRendererClock()) {
ensureSynced();
return rendererClock.getPositionUs();
} else {
return standaloneMediaClock.getPositionUs();
}
public long syncAndGetPositionUs(boolean isReadingAhead) {
syncClocks(isReadingAhead);
return getPositionUs();
}
// MediaClock implementation.
@Override
public long getPositionUs() {
if (isUsingRendererClock()) {
return rendererClock.getPositionUs();
} else {
return standaloneMediaClock.getPositionUs();
}
return isUsingStandaloneClock ? standaloneClock.getPositionUs() : rendererClock.getPositionUs();
}
@Override
@ -146,32 +145,53 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
rendererClock.setPlaybackParameters(playbackParameters);
playbackParameters = rendererClock.getPlaybackParameters();
}
standaloneMediaClock.setPlaybackParameters(playbackParameters);
standaloneClock.setPlaybackParameters(playbackParameters);
}
@Override
public PlaybackParameters getPlaybackParameters() {
return rendererClock != null ? rendererClock.getPlaybackParameters()
: standaloneMediaClock.getPlaybackParameters();
return rendererClock != null
? rendererClock.getPlaybackParameters()
: standaloneClock.getPlaybackParameters();
}
private void ensureSynced() {
private void syncClocks(boolean isReadingAhead) {
if (shouldUseStandaloneClock(isReadingAhead)) {
isUsingStandaloneClock = true;
if (standaloneClockIsStarted) {
standaloneClock.start();
}
return;
}
long rendererClockPositionUs = rendererClock.getPositionUs();
standaloneMediaClock.resetPosition(rendererClockPositionUs);
if (isUsingStandaloneClock) {
// Ensure enabling the renderer clock doesn't jump backwards in time.
if (rendererClockPositionUs < standaloneClock.getPositionUs()) {
standaloneClock.stop();
return;
}
isUsingStandaloneClock = false;
if (standaloneClockIsStarted) {
standaloneClock.start();
}
}
// Continuously sync stand-alone clock to renderer clock so that it can take over if needed.
standaloneClock.resetPosition(rendererClockPositionUs);
PlaybackParameters playbackParameters = rendererClock.getPlaybackParameters();
if (!playbackParameters.equals(standaloneMediaClock.getPlaybackParameters())) {
standaloneMediaClock.setPlaybackParameters(playbackParameters);
if (!playbackParameters.equals(standaloneClock.getPlaybackParameters())) {
standaloneClock.setPlaybackParameters(playbackParameters);
listener.onPlaybackParametersChanged(playbackParameters);
}
}
private boolean isUsingRendererClock() {
// Use the renderer clock if the providing renderer has not ended or needs the next sample
// stream to reenter the ready state. The latter case uses the standalone clock to avoid getting
// stuck if tracks in the current period have uneven durations.
// See: https://github.com/google/ExoPlayer/issues/1874.
return rendererClockSource != null && !rendererClockSource.isEnded()
&& (rendererClockSource.isReady() || !rendererClockSource.hasReadStreamToEnd());
private boolean shouldUseStandaloneClock(boolean isReadingAhead) {
// Use the standalone clock if the clock providing renderer is not set or has ended. Also use
// the standalone clock if the renderer is not ready and we have finished reading the stream or
// are reading ahead to avoid getting stuck if tracks in the current period have uneven
// durations. See: https://github.com/google/ExoPlayer/issues/1874.
return rendererClockSource == null
|| rendererClockSource.isEnded()
|| (!rendererClockSource.isReady()
&& (isReadingAhead || rendererClockSource.hasReadStreamToEnd()));
}
}

View file

@ -535,7 +535,9 @@ import java.util.concurrent.atomic.AtomicBoolean;
playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL);
}
} else {
rendererPositionUs = mediaClock.syncAndGetPositionUs();
rendererPositionUs =
mediaClock.syncAndGetPositionUs(
/* isReadingAhead= */ playingPeriodHolder != queue.getReadingPeriod());
periodPositionUs = playingPeriodHolder.toPeriodTime(rendererPositionUs);
maybeTriggerPendingMessages(playbackInfo.positionUs, periodPositionUs);
playbackInfo.positionUs = periodPositionUs;

View file

@ -53,13 +53,14 @@ public class DefaultMediaClockTest {
@Test
public void standaloneResetPosition_getPositionShouldReturnSameValue() throws Exception {
mediaClock.resetPosition(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
}
@Test
public void standaloneGetAndResetPosition_shouldNotTriggerCallback() throws Exception {
mediaClock.resetPosition(TEST_POSITION_US);
mediaClock.syncAndGetPositionUs();
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
verifyNoMoreInteractions(listener);
}
@ -77,7 +78,7 @@ public class DefaultMediaClockTest {
@Test
public void standaloneStart_shouldStartClock() throws Exception {
mediaClock.start();
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
@ -98,7 +99,7 @@ public class DefaultMediaClockTest {
mediaClock.start();
mediaClock.stop();
mediaClock.start();
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
@ -130,7 +131,7 @@ public class DefaultMediaClockTest {
mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
mediaClock.start();
// Asserts that clock is running with speed declared in getPlaybackParameters().
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
@ -165,6 +166,7 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer =
new MediaClockRenderer(TEST_PLAYBACK_PARAMETERS, /* playbackParametersAreMutable= */ false);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
verify(listener).onPlaybackParametersChanged(TEST_PLAYBACK_PARAMETERS);
}
@ -174,6 +176,7 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
/* playbackParametersAreMutable= */ false);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
verifyNoMoreInteractions(listener);
}
@ -183,7 +186,9 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer =
new MediaClockRenderer(TEST_PLAYBACK_PARAMETERS, /* playbackParametersAreMutable= */ false);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
mediaClock.onRendererDisabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
assertThat(mediaClock.getPlaybackParameters()).isEqualTo(TEST_PLAYBACK_PARAMETERS);
}
@ -193,6 +198,7 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
/* playbackParametersAreMutable= */ true);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
assertThat(mediaClock.getPlaybackParameters()).isEqualTo(TEST_PLAYBACK_PARAMETERS);
}
@ -203,6 +209,7 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
/* playbackParametersAreMutable= */ true);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
verifyNoMoreInteractions(listener);
}
@ -213,6 +220,7 @@ public class DefaultMediaClockTest {
FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
/* playbackParametersAreMutable= */ false);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
assertThat(mediaClock.getPlaybackParameters()).isEqualTo(PlaybackParameters.DEFAULT);
}
@ -223,7 +231,8 @@ public class DefaultMediaClockTest {
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClockRenderer.positionUs = TEST_POSITION_US;
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
// We're not advancing the renderer media clock. Thus, the clock should appear to be stopped.
assertClockIsStopped();
}
@ -235,9 +244,11 @@ public class DefaultMediaClockTest {
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClockRenderer.positionUs = TEST_POSITION_US;
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
mediaClock.resetPosition(0);
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
}
@Test
@ -246,23 +257,24 @@ public class DefaultMediaClockTest {
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClockRenderer.positionUs = TEST_POSITION_US;
mediaClock.syncAndGetPositionUs();
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
mediaClock.onRendererDisabled(mediaClockRenderer);
fakeClock.advanceTime(SLEEP_TIME_MS);
assertThat(mediaClock.syncAndGetPositionUs())
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US + C.msToUs(SLEEP_TIME_MS));
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
public void getPositionWithPlaybackParameterChange_shouldTriggerCallback()
throws ExoPlaybackException {
MediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
/* playbackParametersAreMutable= */ true);
MediaClockRenderer mediaClockRenderer =
new MediaClockRenderer(
PlaybackParameters.DEFAULT, /* playbackParametersAreMutable= */ true);
mediaClock.onRendererEnabled(mediaClockRenderer);
// Silently change playback parameters of renderer clock.
mediaClockRenderer.playbackParameters = TEST_PLAYBACK_PARAMETERS;
mediaClock.syncAndGetPositionUs();
mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
verify(listener).onPlaybackParametersChanged(TEST_PLAYBACK_PARAMETERS);
}
@ -283,7 +295,18 @@ public class DefaultMediaClockTest {
/* isEnded= */ false, /* hasReadStreamToEnd= */ true);
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
public void rendererNotReadyAndReadingAhead_shouldFallbackToStandaloneClock()
throws ExoPlaybackException {
MediaClockRenderer mediaClockRenderer =
new MediaClockRenderer(
/* isReady= */ false, /* isEnded= */ false, /* hasReadStreamToEnd= */ false);
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
assertClockIsRunning(/* isReadingAhead= */ true);
}
@Test
@ -293,7 +316,7 @@ public class DefaultMediaClockTest {
/* isEnded= */ true, /* hasReadStreamToEnd= */ true);
mediaClock.start();
mediaClock.onRendererEnabled(mediaClockRenderer);
assertClockIsRunning();
assertClockIsRunning(/* isReadingAhead= */ false);
}
@Test
@ -302,7 +325,8 @@ public class DefaultMediaClockTest {
MediaClockRenderer mediaClockRenderer = new MediaClockRenderer();
mediaClockRenderer.positionUs = TEST_POSITION_US;
mediaClock.onRendererDisabled(mediaClockRenderer);
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(C.msToUs(fakeClock.elapsedRealtime()));
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(C.msToUs(fakeClock.elapsedRealtime()));
}
@Test
@ -312,7 +336,8 @@ public class DefaultMediaClockTest {
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClock.onRendererEnabled(mediaClockRenderer);
mediaClockRenderer.positionUs = TEST_POSITION_US;
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
}
@Test
@ -328,20 +353,24 @@ public class DefaultMediaClockTest {
} catch (ExoPlaybackException e) {
// Expected.
}
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(TEST_POSITION_US);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(TEST_POSITION_US);
}
private void assertClockIsRunning() {
long clockStartUs = mediaClock.syncAndGetPositionUs();
private void assertClockIsRunning(boolean isReadingAhead) {
long clockStartUs = mediaClock.syncAndGetPositionUs(isReadingAhead);
fakeClock.advanceTime(SLEEP_TIME_MS);
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(clockStartUs
+ mediaClock.getPlaybackParameters().getMediaTimeUsForPlayoutTimeMs(SLEEP_TIME_MS));
assertThat(mediaClock.syncAndGetPositionUs(isReadingAhead))
.isEqualTo(
clockStartUs
+ mediaClock.getPlaybackParameters().getMediaTimeUsForPlayoutTimeMs(SLEEP_TIME_MS));
}
private void assertClockIsStopped() {
long positionAtStartUs = mediaClock.syncAndGetPositionUs();
long positionAtStartUs = mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false);
fakeClock.advanceTime(SLEEP_TIME_MS);
assertThat(mediaClock.syncAndGetPositionUs()).isEqualTo(positionAtStartUs);
assertThat(mediaClock.syncAndGetPositionUs(/* isReadingAhead= */ false))
.isEqualTo(positionAtStartUs);
}
@SuppressWarnings("HidingField")