From 9608ae4e3df558eaaf3050fbdd3a24513b403e8f Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 9 Jan 2025 07:02:13 -0800 Subject: [PATCH] Add LivePositionSupplier util to SimpleBasePlayer In some cases the position is supplied from a "live" source that keeps changing its value at repeated calls. This typically happens when forwarding to another backend with unpredictable position values. The problem is that as soon as the backend system has a position discontinuity, any previously created PositionSupplier now returns a position that no longer makes sense in the context of the State object it belongs to. ForwardingSimpleBasePlayer already works around this issue by having a util class to disconnect these live sources. This change moves the same util to SimpleBasePlayer itself so it can be reused by other implementations. PiperOrigin-RevId: 713658046 --- .../common/ForwardingSimpleBasePlayer.java | 80 +++++++------------ .../media3/common/SimpleBasePlayer.java | 58 +++++++++++++- .../media3/common/SimpleBasePlayerTest.java | 30 +++++++ 3 files changed, 116 insertions(+), 52 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/ForwardingSimpleBasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/ForwardingSimpleBasePlayer.java index 821f8687de..5924a0c96d 100644 --- a/libraries/common/src/main/java/androidx/media3/common/ForwardingSimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/ForwardingSimpleBasePlayer.java @@ -60,7 +60,7 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { private final Player player; - private ForwardingPositionSupplier currentPositionSupplier; + private LivePositionSuppliers livePositionSuppliers; private Metadata lastTimedMetadata; private @Player.PlayWhenReadyChangeReason int playWhenReadyChangeReason; private @Player.DiscontinuityReason int pendingDiscontinuityReason; @@ -78,7 +78,7 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { this.lastTimedMetadata = new Metadata(/* presentationTimeUs= */ C.TIME_UNSET); this.playWhenReadyChangeReason = Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST; this.pendingDiscontinuityReason = Player.DISCONTINUITY_REASON_INTERNAL; - this.currentPositionSupplier = new ForwardingPositionSupplier(player); + this.livePositionSuppliers = new LivePositionSuppliers(player); player.addListener( new Listener() { @Override @@ -99,15 +99,8 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { @Player.DiscontinuityReason int reason) { pendingDiscontinuityReason = reason; pendingPositionDiscontinuityNewPositionMs = newPosition.positionMs; - // Any previously created State will directly call through to player.getCurrentPosition - // via the existing position supplier. From this point onwards, this is wrong as the - // player had a discontinuity and will now return a new position unrelated to the old - // State. We can disconnect these old State objects from the underlying Player by fixing - // the position to the one before the discontinuity and using a new (live) position - // supplier for future State objects. - currentPositionSupplier.setConstant( - oldPosition.positionMs, oldPosition.contentPositionMs); - currentPositionSupplier = new ForwardingPositionSupplier(player); + livePositionSuppliers.disconnect(oldPosition.positionMs, oldPosition.contentPositionMs); + livePositionSuppliers = new LivePositionSuppliers(player); } @Override @@ -132,18 +125,18 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { protected State getState() { // Ordered alphabetically by State.Builder setters. State.Builder state = new State.Builder(); - ForwardingPositionSupplier positionSupplier = currentPositionSupplier; + LivePositionSuppliers positionSuppliers = livePositionSuppliers; if (player.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)) { - state.setAdBufferedPositionMs(positionSupplier::getBufferedPositionMs); - state.setAdPositionMs(positionSupplier::getCurrentPositionMs); + state.setAdBufferedPositionMs(positionSuppliers.bufferedPositionSupplier); + state.setAdPositionMs(positionSuppliers.currentPositionSupplier); } if (player.isCommandAvailable(Player.COMMAND_GET_AUDIO_ATTRIBUTES)) { state.setAudioAttributes(player.getAudioAttributes()); } state.setAvailableCommands(player.getAvailableCommands()); if (player.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)) { - state.setContentBufferedPositionMs(positionSupplier::getContentBufferedPositionMs); - state.setContentPositionMs(positionSupplier::getContentPositionMs); + state.setContentBufferedPositionMs(positionSuppliers.contentBufferedPositionSupplier); + state.setContentPositionMs(positionSuppliers.contentPositionSupplier); if (player.isCommandAvailable(Player.COMMAND_GET_TIMELINE)) { state.setCurrentAd(player.getCurrentAdGroupIndex(), player.getCurrentAdIndexInAdGroup()); } @@ -194,7 +187,7 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { state.setSurfaceSize(player.getSurfaceSize()); state.setTimedMetadata(lastTimedMetadata); if (player.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)) { - state.setTotalBufferedDurationMs(positionSupplier::getTotalBufferedDurationMs); + state.setTotalBufferedDurationMs(positionSuppliers.totalBufferedPositionSupplier); } state.setTrackSelectionParameters(player.getTrackSelectionParameters()); state.setVideoSize(player.getVideoSize()); @@ -456,44 +449,29 @@ public class ForwardingSimpleBasePlayer extends SimpleBasePlayer { * Forwards to the changing position values of the wrapped player until the forwarding is * deactivated with constant values. */ - private static final class ForwardingPositionSupplier { + private static final class LivePositionSuppliers { - private final Player player; + public final LivePositionSupplier currentPositionSupplier; + public final LivePositionSupplier bufferedPositionSupplier; + public final LivePositionSupplier contentPositionSupplier; + public final LivePositionSupplier contentBufferedPositionSupplier; + public final LivePositionSupplier totalBufferedPositionSupplier; - private long positionsMs; - private long contentPositionMs; - - public ForwardingPositionSupplier(Player player) { - this.player = player; - this.positionsMs = C.TIME_UNSET; - this.contentPositionMs = C.TIME_UNSET; + public LivePositionSuppliers(Player player) { + currentPositionSupplier = new LivePositionSupplier(player::getCurrentPosition); + bufferedPositionSupplier = new LivePositionSupplier(player::getBufferedPosition); + contentPositionSupplier = new LivePositionSupplier(player::getContentPosition); + contentBufferedPositionSupplier = + new LivePositionSupplier(player::getContentBufferedPosition); + totalBufferedPositionSupplier = new LivePositionSupplier(player::getTotalBufferedDuration); } - public void setConstant(long positionMs, long contentPositionMs) { - this.positionsMs = positionMs; - this.contentPositionMs = contentPositionMs; - } - - public long getCurrentPositionMs() { - return positionsMs == C.TIME_UNSET ? player.getCurrentPosition() : positionsMs; - } - - public long getBufferedPositionMs() { - return positionsMs == C.TIME_UNSET ? player.getBufferedPosition() : positionsMs; - } - - public long getContentPositionMs() { - return contentPositionMs == C.TIME_UNSET ? player.getContentPosition() : contentPositionMs; - } - - public long getContentBufferedPositionMs() { - return contentPositionMs == C.TIME_UNSET - ? player.getContentBufferedPosition() - : contentPositionMs; - } - - public long getTotalBufferedDurationMs() { - return positionsMs == C.TIME_UNSET ? player.getTotalBufferedDuration() : 0; + public void disconnect(long positionMs, long contentPositionMs) { + currentPositionSupplier.disconnect(positionMs); + bufferedPositionSupplier.disconnect(positionMs); + contentPositionSupplier.disconnect(contentPositionMs); + contentBufferedPositionSupplier.disconnect(contentPositionMs); + totalBufferedPositionSupplier.disconnect(/* finalValue= */ 0); } } } 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 18ff39859e..13ec2e4e9f 100644 --- a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java @@ -2069,7 +2069,21 @@ public abstract class SimpleBasePlayer extends BasePlayer { } } - /** A supplier for a position. */ + /** + * A supplier for a position. + * + *

Convenience methods and classes for creating position suppliers: + * + *

+ */ protected interface PositionSupplier { /** An instance returning a constant position of zero. */ @@ -2102,6 +2116,48 @@ public abstract class SimpleBasePlayer extends BasePlayer { long get(); } + /** + * A {@link PositionSupplier} connected to a live provider that returns a new value on each + * invocation until it is {@linkplain #disconnect disconnected} from the live source. + * + *

The recommended usage of this class is to create a new instance connected to the live source + * and keep returning this instance as long as the position source is still valid. As soon as the + * position source becomes invalid, for example when handling a position discontinuity, call + * {@link #disconnect} with the final position that will be returned for all future invocations. + */ + protected static final class LivePositionSupplier implements PositionSupplier { + + private final PositionSupplier livePosition; + + private long finalValue; + + /** + * Creates the live position supplier. + * + * @param livePosition The function returning the live position. + */ + public LivePositionSupplier(PositionSupplier livePosition) { + this.livePosition = livePosition; + this.finalValue = C.TIME_UNSET; + } + + /** + * Disconnects the position supplier from the live source. + * + *

All future invocations of {@link #get()} will return the provided final position. + * + * @param finalValue The final position value. + */ + public void disconnect(long finalValue) { + this.finalValue = finalValue; + } + + @Override + public long get() { + return finalValue != C.TIME_UNSET ? finalValue : livePosition.get(); + } + } + /** * Position difference threshold below which we do not automatically report a position * discontinuity, in milliseconds. diff --git a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java index a7e2a79b0a..aa37200c7d 100644 --- a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java @@ -8698,6 +8698,36 @@ public class SimpleBasePlayerTest { assertThat(callForwarded.get()).isFalse(); } + @Test + public void livePositionProvider_returnsChangingLivePosition() { + AtomicInteger livePositionMs = new AtomicInteger(/* initialValue= */ 100); + SimpleBasePlayer.LivePositionSupplier livePositionSupplier = + new SimpleBasePlayer.LivePositionSupplier(livePositionMs::get); + + long position1Ms = livePositionSupplier.get(); + livePositionMs.set(200); + long position2Ms = livePositionSupplier.get(); + livePositionMs.set(300); + long position3Ms = livePositionSupplier.get(); + + assertThat(position1Ms).isEqualTo(100); + assertThat(position2Ms).isEqualTo(200); + assertThat(position3Ms).isEqualTo(300); + } + + @Test + public void livePositionProvider_disconnect_returnsFinalPosition() { + AtomicInteger livePositionMs = new AtomicInteger(/* initialValue= */ 100); + SimpleBasePlayer.LivePositionSupplier livePositionSupplier = + new SimpleBasePlayer.LivePositionSupplier(livePositionMs::get); + + livePositionSupplier.disconnect(/* finalValue= */ 150); + livePositionMs.set(200); + long positionMs = livePositionSupplier.get(); + + assertThat(positionMs).isEqualTo(150); + } + private static Object[] getAnyArguments(Method method) { Object[] arguments = new Object[method.getParameterCount()]; Class[] argumentTypes = method.getParameterTypes();