From b9d12837b40e86222b32e21c86ffd4bf0ecd2882 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 9 Jan 2025 07:07:32 -0800 Subject: [PATCH] Use live providers in CompositionPlayer to remove workaround When CompositionPlayer added repeat modes in https://github.com/androidx/media/commit/d0afb96c4073b902b6d78205c96328c7c270492e, it changed the logic in SimpleBasePlayer to workaround the specific scenario in CompositionPlayer that didn't seem to work correctly. The workaround is not really generically applicable though. The source of the original problem was that the position values of all state objects were connected to the "live" source of the current player so that the repetition case couldn't be detected (the position before and after the seek looked the same). The solution is to use the newly added LivePositionSuppliers and disconnect them before seeking. PiperOrigin-RevId: 713659638 --- .../media3/common/SimpleBasePlayer.java | 12 +++---- .../media3/transformer/CompositionPlayer.java | 31 ++++++++++++++----- .../transformer/CompositionPlayerTest.java | 1 + 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java index 13ec2e4e9f..d27a8a7f1e 100644 --- a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java @@ -4064,14 +4064,10 @@ public abstract class SimpleBasePlayer extends BasePlayer { } // Only mark changes within the current item as a transition if we are repeating automatically // or via a seek to next/previous. - if (positionDiscontinuityReason == DISCONTINUITY_REASON_AUTO_TRANSITION) { - if ((getContentPositionMsInternal(previousState, window) - > getContentPositionMsInternal(newState, window)) - || (newState.hasPositionDiscontinuity - && newState.discontinuityPositionMs == C.TIME_UNSET - && isRepeatingCurrentItem)) { - return MEDIA_ITEM_TRANSITION_REASON_REPEAT; - } + if (positionDiscontinuityReason == DISCONTINUITY_REASON_AUTO_TRANSITION + && getContentPositionMsInternal(previousState, window) + > getContentPositionMsInternal(newState, window)) { + return MEDIA_ITEM_TRANSITION_REASON_REPEAT; } if (positionDiscontinuityReason == DISCONTINUITY_REASON_SEEK && isRepeatingCurrentItem) { return MEDIA_ITEM_TRANSITION_REASON_SEEK; diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java index f3b3c4ca01..fddcf45493 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java @@ -258,6 +258,7 @@ public final class CompositionPlayer extends SimpleBasePlayer COMMAND_STOP, COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, + COMMAND_SEEK_TO_DEFAULT_POSITION, COMMAND_SEEK_BACK, COMMAND_SEEK_FORWARD, COMMAND_GET_CURRENT_MEDIA_ITEM, @@ -307,7 +308,12 @@ public final class CompositionPlayer extends SimpleBasePlayer @Nullable private SurfaceHolder surfaceHolder; @Nullable private Surface displaySurface; private boolean repeatingCompositionSeekInProgress; + private LivePositionSupplier positionSupplier; + private LivePositionSupplier bufferedPositionSupplier; + private LivePositionSupplier totalBufferedDurationSupplier; + // "this" reference for position suppliers. + @SuppressWarnings("initialization:methodref.receiver.bound.invalid") private CompositionPlayer(Builder builder) { super(checkNotNull(builder.looper), builder.clock); context = builder.context; @@ -322,6 +328,9 @@ public final class CompositionPlayer extends SimpleBasePlayer compositionDurationUs = C.TIME_UNSET; playbackState = STATE_IDLE; volume = 1.0f; + positionSupplier = new LivePositionSupplier(this::getContentPositionMs); + bufferedPositionSupplier = new LivePositionSupplier(this::getBufferedPositionMs); + totalBufferedDurationSupplier = new LivePositionSupplier(this::getTotalBufferedDurationMs); } /** @@ -446,9 +455,9 @@ public final class CompositionPlayer extends SimpleBasePlayer .setPlayWhenReady(playWhenReady, playWhenReadyChangeReason) .setRepeatMode(repeatMode) .setVolume(volume) - .setContentPositionMs(this::getContentPositionMs) - .setContentBufferedPositionMs(this::getBufferedPositionMs) - .setTotalBufferedDurationMs(this::getTotalBufferedDurationMs) + .setContentPositionMs(positionSupplier) + .setContentBufferedPositionMs(bufferedPositionSupplier) + .setTotalBufferedDurationMs(totalBufferedDurationSupplier) .setNewlyRenderedFirstFrame(getRenderedFirstFrameAndReset()); if (repeatingCompositionSeekInProgress) { state.setPositionDiscontinuity(DISCONTINUITY_REASON_AUTO_TRANSITION, C.TIME_UNSET); @@ -565,6 +574,7 @@ public final class CompositionPlayer extends SimpleBasePlayer @Override protected ListenableFuture handleSeek( int mediaItemIndex, long positionMs, @Player.Command int seekCommand) { + resetLivePositionSuppliers(); CompositionPlayerInternal compositionPlayerInternal = checkStateNotNull(this.compositionPlayerInternal); compositionPlayerInternal.startSeek(positionMs); @@ -964,11 +974,7 @@ public final class CompositionPlayer extends SimpleBasePlayer private void repeatCompositionPlayback() { repeatingCompositionSeekInProgress = true; - seekTo( - getCurrentMediaItemIndex(), - /* positionMs= */ C.TIME_UNSET, - Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, - /* isRepeatingCurrentItem= */ true); + seekToDefaultPosition(); } private ImmutableList createPlaylist() { @@ -980,6 +986,15 @@ public final class CompositionPlayer extends SimpleBasePlayer .build()); } + private void resetLivePositionSuppliers() { + positionSupplier.disconnect(getContentPositionMs()); + bufferedPositionSupplier.disconnect(getBufferedPositionMs()); + totalBufferedDurationSupplier.disconnect(getTotalBufferedDurationMs()); + positionSupplier = new LivePositionSupplier(this::getContentPositionMs); + bufferedPositionSupplier = new LivePositionSupplier(this::getBufferedPositionMs); + totalBufferedDurationSupplier = new LivePositionSupplier(this::getTotalBufferedDurationMs); + } + private static long getCompositionDurationUs(Composition composition) { checkState(!composition.sequences.isEmpty()); return getSequenceDurationUs(composition.sequences.get(0)); diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/CompositionPlayerTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/CompositionPlayerTest.java index 08c4c68f37..0ff224b934 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/CompositionPlayerTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/CompositionPlayerTest.java @@ -237,6 +237,7 @@ public class CompositionPlayerTest { Player.COMMAND_STOP, Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, + Player.COMMAND_SEEK_TO_DEFAULT_POSITION, Player.COMMAND_SEEK_BACK, Player.COMMAND_SEEK_FORWARD, Player.COMMAND_GET_CURRENT_MEDIA_ITEM,