Skip to content

Commit

Permalink
Clean up offload state tracking
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@tonihei @icbaker
tonihei authored and icbaker committed Jun 27, 2022
1 parent aaa0152 commit aedde2d
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 150 deletions.
9 changes: 3 additions & 6 deletions library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ public DeviceComponent getDeviceComponent() {
public void experimentalSetOffloadSchedulingEnabled(boolean offloadSchedulingEnabled) {
verifyApplicationThread();
internalPlayer.experimentalSetOffloadSchedulingEnabled(offloadSchedulingEnabled);
for (AudioOffloadListener listener : audioOffloadListeners) {
listener.onExperimentalOffloadSchedulingEnabledChanged(offloadSchedulingEnabled);
}
}

@Override
Expand Down Expand Up @@ -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);
Expand Down
40 changes: 15 additions & 25 deletions library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -1459,7 +1450,6 @@ private void resetInternal(
/* bufferedPositionUs= */ startPositionUs,
/* totalBufferedDurationUs= */ 0,
/* positionUs= */ startPositionUs,
offloadSchedulingEnabled,
/* sleepingForOffload= */ false);
if (releaseMediaSourceList) {
mediaSourceList.release();
Expand Down
46 changes: 0 additions & 46 deletions library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -118,7 +116,6 @@ public static PlaybackInfo createDummy(TrackSelectorResult emptyTrackSelectorRes
/* bufferedPositionUs= */ 0,
/* totalBufferedDurationUs= */ 0,
/* positionUs= */ 0,
/* offloadSchedulingEnabled= */ false,
/* sleepingForOffload= */ false);
}

Expand All @@ -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(
Expand All @@ -162,7 +158,6 @@ public PlaybackInfo(
long bufferedPositionUs,
long totalBufferedDurationUs,
long positionUs,
boolean offloadSchedulingEnabled,
boolean sleepingForOffload) {
this.timeline = timeline;
this.periodId = periodId;
Expand All @@ -181,7 +176,6 @@ public PlaybackInfo(
this.bufferedPositionUs = bufferedPositionUs;
this.totalBufferedDurationUs = totalBufferedDurationUs;
this.positionUs = positionUs;
this.offloadSchedulingEnabled = offloadSchedulingEnabled;
this.sleepingForOffload = sleepingForOffload;
}

Expand Down Expand Up @@ -233,7 +227,6 @@ public PlaybackInfo copyWithNewPosition(
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -263,7 +256,6 @@ public PlaybackInfo copyWithTimeline(Timeline timeline) {
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -293,7 +285,6 @@ public PlaybackInfo copyWithPlaybackState(int playbackState) {
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -323,7 +314,6 @@ public PlaybackInfo copyWithPlaybackError(@Nullable ExoPlaybackException playbac
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -353,7 +343,6 @@ public PlaybackInfo copyWithIsLoading(boolean isLoading) {
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -383,7 +372,6 @@ public PlaybackInfo copyWithLoadingMediaPeriodId(MediaPeriodId loadingMediaPerio
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -417,7 +405,6 @@ public PlaybackInfo copyWithPlayWhenReady(
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -508,7 +463,6 @@ public PlaybackInfo copyWithSleepingForOffload(boolean sleepingForOffload) {
bufferedPositionUs,
totalBufferedDurationUs,
positionUs,
offloadSchedulingEnabled,
sleepingForOffload);
}
}
42 changes: 5 additions & 37 deletions library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,6 @@ private void setupTimeline(Timeline timeline) {
/* bufferedPositionUs= */ 0,
/* totalBufferedDurationUs= */ 0,
/* positionUs= */ 0,
/* offloadSchedulingEnabled= */ false,
/* sleepingForOffload= */ false);
}

Expand Down
35 changes: 0 additions & 35 deletions ...ricutils/src/main/java/com/google/android/exoplayer2/robolectric/TestPlayerRunHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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
Expand Down

0 comments on commit aedde2d

Please sign in to comment.