From aedde2de396995e4354f3110cc37e86b48525892 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 27 Jun 2022 10:34:56 +0100 Subject: [PATCH] Clean up offload state tracking 1. The offloadSchedulingEnabled value doesn't need to be in PlaybackInfo because it's never updated in EPII. 2. The sleepingForOffload value in EPII wasn't updated explicitly (just via the return value of a method). It was also only meant to be enabled while the player is actively playing, but confusingly triggered from a path where the player may theoretically be buffering as well. 3. The offload sleeping (=not scheduling doSomeWork) was interwoven into the actual scheduling code making it slightly hard to follow. This can be improved slightly by keeping the offload sleeping decision and the scheduling separate. PiperOrigin-RevId: 457427293 --- .../android/exoplayer2/ExoPlayerImpl.java | 9 ++-- .../exoplayer2/ExoPlayerImplInternal.java | 40 ++++++---------- .../android/exoplayer2/PlaybackInfo.java | 46 ------------------- .../android/exoplayer2/ExoPlayerTest.java | 42 ++--------------- .../exoplayer2/MediaPeriodQueueTest.java | 1 - .../robolectric/TestPlayerRunHelper.java | 35 -------------- 6 files changed, 23 insertions(+), 150 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index a48a7e33ca9..8883eb9864c 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -422,6 +422,9 @@ public DeviceComponent getDeviceComponent() { public void experimentalSetOffloadSchedulingEnabled(boolean offloadSchedulingEnabled) { verifyApplicationThread(); internalPlayer.experimentalSetOffloadSchedulingEnabled(offloadSchedulingEnabled); + for (AudioOffloadListener listener : audioOffloadListeners) { + listener.onExperimentalOffloadSchedulingEnabledChanged(offloadSchedulingEnabled); + } } @Override @@ -1951,12 +1954,6 @@ private void updatePlaybackInfo( updateAvailableCommands(); listeners.flushEvents(); - if (previousPlaybackInfo.offloadSchedulingEnabled != newPlaybackInfo.offloadSchedulingEnabled) { - for (AudioOffloadListener listener : audioOffloadListeners) { - listener.onExperimentalOffloadSchedulingEnabledChanged( - newPlaybackInfo.offloadSchedulingEnabled); - } - } if (previousPlaybackInfo.sleepingForOffload != newPlaybackInfo.sleepingForOffload) { for (AudioOffloadListener listener : audioOffloadListeners) { listener.onExperimentalSleepingForOffloadChanged(newPlaybackInfo.sleepingForOffload); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index d107b14d597..7677740f5d3 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -809,10 +809,8 @@ private void setOffloadSchedulingEnabledInternal(boolean offloadSchedulingEnable return; } this.offloadSchedulingEnabled = offloadSchedulingEnabled; - @Player.State int state = playbackInfo.playbackState; - if (offloadSchedulingEnabled || state == Player.STATE_ENDED || state == Player.STATE_IDLE) { - playbackInfo = playbackInfo.copyWithOffloadSchedulingEnabled(offloadSchedulingEnabled); - } else { + if (!offloadSchedulingEnabled && playbackInfo.sleepingForOffload) { + // We need to wake the player up if offload scheduling is disabled and we are sleeping. handler.sendEmptyMessage(MSG_DO_SOME_WORK); } } @@ -1072,22 +1070,24 @@ && isLoadingPossible()) { throw new IllegalStateException("Playback stuck buffering and not loading"); } - if (offloadSchedulingEnabled != playbackInfo.offloadSchedulingEnabled) { - playbackInfo = playbackInfo.copyWithOffloadSchedulingEnabled(offloadSchedulingEnabled); - } - - boolean sleepingForOffload = false; - if ((shouldPlayWhenReady() && playbackInfo.playbackState == Player.STATE_READY) - || playbackInfo.playbackState == Player.STATE_BUFFERING) { - sleepingForOffload = !maybeScheduleWakeup(operationStartTimeMs, ACTIVE_INTERVAL_MS); - } else if (enabledRendererCount != 0 && playbackInfo.playbackState != Player.STATE_ENDED) { - scheduleNextWork(operationStartTimeMs, IDLE_INTERVAL_MS); - } + boolean isPlaying = shouldPlayWhenReady() && playbackInfo.playbackState == Player.STATE_READY; + boolean sleepingForOffload = offloadSchedulingEnabled && requestForRendererSleep && isPlaying; if (playbackInfo.sleepingForOffload != sleepingForOffload) { playbackInfo = playbackInfo.copyWithSleepingForOffload(sleepingForOffload); } requestForRendererSleep = false; // A sleep request is only valid for the current doSomeWork. + if (sleepingForOffload || playbackInfo.playbackState == Player.STATE_ENDED) { + // No need to schedule next work. + return; + } else if (isPlaying || playbackInfo.playbackState == Player.STATE_BUFFERING) { + // We are actively playing or waiting for data to be ready. Schedule next work quickly. + scheduleNextWork(operationStartTimeMs, ACTIVE_INTERVAL_MS); + } else if (playbackInfo.playbackState == Player.STATE_READY && enabledRendererCount != 0) { + // We are ready, but not playing. Schedule next work less often to handle non-urgent updates. + scheduleNextWork(operationStartTimeMs, IDLE_INTERVAL_MS); + } + TraceUtil.endSection(); } @@ -1120,15 +1120,6 @@ private void scheduleNextWork(long thisOperationStartTimeMs, long intervalMs) { handler.sendEmptyMessageAtTime(MSG_DO_SOME_WORK, thisOperationStartTimeMs + intervalMs); } - private boolean maybeScheduleWakeup(long operationStartTimeMs, long intervalMs) { - if (offloadSchedulingEnabled && requestForRendererSleep) { - return false; - } - - scheduleNextWork(operationStartTimeMs, intervalMs); - return true; - } - private void seekToInternal(SeekPosition seekPosition) throws ExoPlaybackException { playbackInfoUpdate.incrementPendingOperationAcks(/* operationAcks= */ 1); @@ -1459,7 +1450,6 @@ private void resetInternal( /* bufferedPositionUs= */ startPositionUs, /* totalBufferedDurationUs= */ 0, /* positionUs= */ startPositionUs, - offloadSchedulingEnabled, /* sleepingForOffload= */ false); if (releaseMediaSourceList) { mediaSourceList.release(); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java index d8295eb8ecc..5a985901d65 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java @@ -70,8 +70,6 @@ public final @PlaybackSuppressionReason int playbackSuppressionReason; /** The playback parameters. */ public final PlaybackParameters playbackParameters; - /** Whether offload scheduling is enabled for the main player loop. */ - public final boolean offloadSchedulingEnabled; /** Whether the main player loop is sleeping, while using offload scheduling. */ public final boolean sleepingForOffload; @@ -118,7 +116,6 @@ public static PlaybackInfo createDummy(TrackSelectorResult emptyTrackSelectorRes /* bufferedPositionUs= */ 0, /* totalBufferedDurationUs= */ 0, /* positionUs= */ 0, - /* offloadSchedulingEnabled= */ false, /* sleepingForOffload= */ false); } @@ -141,7 +138,6 @@ public static PlaybackInfo createDummy(TrackSelectorResult emptyTrackSelectorRes * @param bufferedPositionUs See {@link #bufferedPositionUs}. * @param totalBufferedDurationUs See {@link #totalBufferedDurationUs}. * @param positionUs See {@link #positionUs}. - * @param offloadSchedulingEnabled See {@link #offloadSchedulingEnabled}. * @param sleepingForOffload See {@link #sleepingForOffload}. */ public PlaybackInfo( @@ -162,7 +158,6 @@ public PlaybackInfo( long bufferedPositionUs, long totalBufferedDurationUs, long positionUs, - boolean offloadSchedulingEnabled, boolean sleepingForOffload) { this.timeline = timeline; this.periodId = periodId; @@ -181,7 +176,6 @@ public PlaybackInfo( this.bufferedPositionUs = bufferedPositionUs; this.totalBufferedDurationUs = totalBufferedDurationUs; this.positionUs = positionUs; - this.offloadSchedulingEnabled = offloadSchedulingEnabled; this.sleepingForOffload = sleepingForOffload; } @@ -233,7 +227,6 @@ public PlaybackInfo copyWithNewPosition( bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -263,7 +256,6 @@ public PlaybackInfo copyWithTimeline(Timeline timeline) { bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -293,7 +285,6 @@ public PlaybackInfo copyWithPlaybackState(int playbackState) { bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -323,7 +314,6 @@ public PlaybackInfo copyWithPlaybackError(@Nullable ExoPlaybackException playbac bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -353,7 +343,6 @@ public PlaybackInfo copyWithIsLoading(boolean isLoading) { bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -383,7 +372,6 @@ public PlaybackInfo copyWithLoadingMediaPeriodId(MediaPeriodId loadingMediaPerio bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -417,7 +405,6 @@ public PlaybackInfo copyWithPlayWhenReady( bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -447,38 +434,6 @@ public PlaybackInfo copyWithPlaybackParameters(PlaybackParameters playbackParame bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, - sleepingForOffload); - } - - /** - * Copies playback info with new offloadSchedulingEnabled. - * - * @param offloadSchedulingEnabled New offloadSchedulingEnabled state. See {@link - * #offloadSchedulingEnabled}. - * @return Copied playback info with new offload scheduling state. - */ - @CheckResult - public PlaybackInfo copyWithOffloadSchedulingEnabled(boolean offloadSchedulingEnabled) { - return new PlaybackInfo( - timeline, - periodId, - requestedContentPositionUs, - discontinuityStartPositionUs, - playbackState, - playbackError, - isLoading, - trackGroups, - trackSelectorResult, - staticMetadata, - loadingMediaPeriodId, - playWhenReady, - playbackSuppressionReason, - playbackParameters, - bufferedPositionUs, - totalBufferedDurationUs, - positionUs, - offloadSchedulingEnabled, sleepingForOffload); } @@ -508,7 +463,6 @@ public PlaybackInfo copyWithSleepingForOffload(boolean sleepingForOffload) { bufferedPositionUs, totalBufferedDurationUs, positionUs, - offloadSchedulingEnabled, sleepingForOffload); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index 39e9b646316..ca0ec0b6a0e 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -53,7 +53,6 @@ import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlaybackState; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPositionDiscontinuity; -import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilReceiveOffloadSchedulingEnabledNewState; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilSleepingForOffload; import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilTimelineChanged; import static com.google.android.exoplayer2.source.ads.ServerSideAdInsertionUtil.addAdGroupToAdPlaybackState; @@ -9631,47 +9630,16 @@ protected void onEnabled(boolean joining, boolean mayRenderStartOfStream) } @Test - public void enableOffloadSchedulingWhileIdle_isToggled_isReported() throws Exception { + public void enableOffloadScheduling_isReported() throws Exception { ExoPlayer player = new TestExoPlayerBuilder(context).build(); + ExoPlayer.AudioOffloadListener mockListener = mock(ExoPlayer.AudioOffloadListener.class); + player.addAudioOffloadListener(mockListener); player.experimentalSetOffloadSchedulingEnabled(true); - assertThat(runUntilReceiveOffloadSchedulingEnabledNewState(player)).isTrue(); + verify(mockListener).onExperimentalOffloadSchedulingEnabledChanged(true); player.experimentalSetOffloadSchedulingEnabled(false); - assertThat(runUntilReceiveOffloadSchedulingEnabledNewState(player)).isFalse(); - } - - @Test - public void enableOffloadSchedulingWhilePlaying_isToggled_isReported() throws Exception { - FakeSleepRenderer sleepRenderer = new FakeSleepRenderer(C.TRACK_TYPE_AUDIO).sleepOnNextRender(); - ExoPlayer player = new TestExoPlayerBuilder(context).setRenderers(sleepRenderer).build(); - Timeline timeline = new FakeTimeline(); - player.setMediaSource(new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT)); - player.prepare(); - player.play(); - - player.experimentalSetOffloadSchedulingEnabled(true); - assertThat(runUntilReceiveOffloadSchedulingEnabledNewState(player)).isTrue(); - - player.experimentalSetOffloadSchedulingEnabled(false); - assertThat(runUntilReceiveOffloadSchedulingEnabledNewState(player)).isFalse(); - } - - @Test - public void enableOffloadSchedulingWhileSleepingForOffload_isDisabled_isReported() - throws Exception { - FakeSleepRenderer sleepRenderer = new FakeSleepRenderer(C.TRACK_TYPE_AUDIO).sleepOnNextRender(); - ExoPlayer player = new TestExoPlayerBuilder(context).setRenderers(sleepRenderer).build(); - Timeline timeline = new FakeTimeline(); - player.setMediaSource(new FakeMediaSource(timeline, ExoPlayerTestRunner.AUDIO_FORMAT)); - player.experimentalSetOffloadSchedulingEnabled(true); - player.prepare(); - player.play(); - runUntilSleepingForOffload(player, /* expectedSleepForOffload= */ true); - - player.experimentalSetOffloadSchedulingEnabled(false); - - assertThat(runUntilReceiveOffloadSchedulingEnabledNewState(player)).isFalse(); + verify(mockListener).onExperimentalOffloadSchedulingEnabledChanged(false); } @Test diff --git a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java index ced931e5446..a09bc2f8850 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java @@ -1106,7 +1106,6 @@ private void setupTimeline(Timeline timeline) { /* bufferedPositionUs= */ 0, /* totalBufferedDurationUs= */ 0, /* positionUs= */ 0, - /* offloadSchedulingEnabled= */ false, /* sleepingForOffload= */ false); } diff --git a/robolectricutils/src/main/java/com/google/android/exoplayer2/robolectric/TestPlayerRunHelper.java b/robolectricutils/src/main/java/com/google/android/exoplayer2/robolectric/TestPlayerRunHelper.java index 58ed22f289d..85869c07506 100644 --- a/robolectricutils/src/main/java/com/google/android/exoplayer2/robolectric/TestPlayerRunHelper.java +++ b/robolectricutils/src/main/java/com/google/android/exoplayer2/robolectric/TestPlayerRunHelper.java @@ -182,41 +182,6 @@ public static ExoPlaybackException runUntilError(ExoPlayer player) throws Timeou return checkNotNull(player.getPlayerError()); } - /** - * Runs tasks of the main {@link Looper} until {@link - * ExoPlayer.AudioOffloadListener#onExperimentalOffloadSchedulingEnabledChanged} is called or a - * playback error occurs. - * - *

If a playback error occurs it will be thrown wrapped in an {@link IllegalStateException}. - * - * @param player The {@link Player}. - * @return The new offloadSchedulingEnabled state. - * @throws TimeoutException If the {@link RobolectricUtil#DEFAULT_TIMEOUT_MS default timeout} is - * exceeded. - */ - public static boolean runUntilReceiveOffloadSchedulingEnabledNewState(ExoPlayer player) - throws TimeoutException { - verifyMainTestThread(player); - AtomicReference<@NullableType Boolean> offloadSchedulingEnabledReceiver = - new AtomicReference<>(); - ExoPlayer.AudioOffloadListener listener = - new ExoPlayer.AudioOffloadListener() { - @Override - public void onExperimentalOffloadSchedulingEnabledChanged( - boolean offloadSchedulingEnabled) { - offloadSchedulingEnabledReceiver.set(offloadSchedulingEnabled); - } - }; - player.addAudioOffloadListener(listener); - runMainLooperUntil( - () -> offloadSchedulingEnabledReceiver.get() != null || player.getPlayerError() != null); - player.removeAudioOffloadListener(listener); - if (player.getPlayerError() != null) { - throw new IllegalStateException(player.getPlayerError()); - } - return checkNotNull(offloadSchedulingEnabledReceiver.get()); - } - /** * Runs tasks of the main {@link Looper} until {@link * ExoPlayer.AudioOffloadListener#onExperimentalSleepingForOffloadChanged(boolean)} is called or a