From 14094b5094e931eef8917b77f80f58c23282d9e5 Mon Sep 17 00:00:00 2001 From: kimvde Date: Tue, 29 Oct 2024 09:02:00 -0700 Subject: [PATCH] Fix offset passed to VideoFrameReleaseControl In some cases, the streamOffsetUs was passed to VideoFrameReleaseControl.getFrameReleaseAction() but it should be the streamStartPositionUs. PiperOrigin-RevId: 691040172 --- .../exoplayer/video/DefaultVideoSink.java | 5 +-- .../video/MediaCodecVideoRenderer.java | 2 - .../video/PlaybackVideoGraphWrapper.java | 42 ++++++++----------- .../video/VideoFrameRenderControl.java | 40 +++++++++--------- .../media3/exoplayer/video/VideoSink.java | 7 +--- .../video/VideoFrameRenderControlTest.java | 18 ++++---- .../transformer/BufferingVideoSink.java | 10 +---- .../transformer/SequenceRenderersFactory.java | 1 - 8 files changed, 51 insertions(+), 74 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java index 1f8796586a..87006e7bf6 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java @@ -124,10 +124,7 @@ import java.util.concurrent.Executor; @Override public void setStreamTimestampInfo( - long streamStartPositionUs, - long streamOffsetUs, - long bufferTimestampAdjustmentUs, - long lastResetPositionUs) { + long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { throw new UnsupportedOperationException(); } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java index c9e9884358..d2859b2475 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java @@ -806,7 +806,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer videoSink.flush(/* resetPosition= */ true); videoSink.setStreamTimestampInfo( getOutputStreamStartPositionUs(), - getOutputStreamOffsetUs(), getBufferTimestampAdjustmentUs(), getLastResetPositionUs()); pendingVideoSinkInputStreamChange = true; @@ -1608,7 +1607,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer if (videoSink != null) { videoSink.setStreamTimestampInfo( getOutputStreamStartPositionUs(), - getOutputStreamOffsetUs(), getBufferTimestampAdjustmentUs(), getLastResetPositionUs()); } else { diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java index d5cde1d49e..28a805422e 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java @@ -468,10 +468,11 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video defaultVideoSink.setPlaybackSpeed(speed); } - private void onStreamOffsetChange( - long bufferTimestampAdjustmentUs, long bufferPresentationTimeUs, long streamOffsetUs) { + private void onStreamTimestampInfoChange( + long bufferTimestampAdjustmentUs, long bufferPresentationTimeUs, long streamStartPositionUs) { this.bufferTimestampAdjustmentUs = bufferTimestampAdjustmentUs; - videoFrameRenderControl.onStreamOffsetChange(bufferPresentationTimeUs, streamOffsetUs); + videoFrameRenderControl.onStreamStartPositionChange( + bufferPresentationTimeUs, streamStartPositionUs); } private static ColorInfo getAdjustedInputColorInfo(@Nullable ColorInfo inputColorInfo) { @@ -493,10 +494,9 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video @Nullable private Format inputFormat; private @InputType int inputType; private long inputStreamStartPositionUs; - private long inputStreamOffsetUs; private long inputBufferTimestampAdjustmentUs; private long lastResetPositionUs; - private boolean pendingInputStreamOffsetChange; + private boolean pendingInputStreamTimestampInfoChange; /** The buffer presentation time, in microseconds, of the final frame in the stream. */ private long finalBufferPresentationTimeUs; @@ -575,8 +575,8 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video lastBufferPresentationTimeUs = C.TIME_UNSET; PlaybackVideoGraphWrapper.this.flush(resetPosition); pendingInputStreamBufferPresentationTimeUs = C.TIME_UNSET; - // Don't change input stream offset or reset the pending input stream offset change so that - // it's announced with the next input frame. + // Don't change input stream start position or reset the pending input stream timestamp info + // change so that it's announced with the next input frame. // Don't reset isInputStreamChangePending because it's not guaranteed to receive a new input // stream after seeking. } @@ -658,16 +658,12 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video @Override public void setStreamTimestampInfo( - long streamStartPositionUs, - long streamOffsetUs, - long bufferTimestampAdjustmentUs, - long lastResetPositionUs) { - // Ors because this method could be called multiple times on a stream offset change. - pendingInputStreamOffsetChange |= - inputStreamOffsetUs != streamOffsetUs + long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { + // Ors because this method could be called multiple times on a timestamp info change. + pendingInputStreamTimestampInfoChange |= + inputStreamStartPositionUs != streamStartPositionUs || inputBufferTimestampAdjustmentUs != bufferTimestampAdjustmentUs; inputStreamStartPositionUs = streamStartPositionUs; - inputStreamOffsetUs = streamOffsetUs; inputBufferTimestampAdjustmentUs = bufferTimestampAdjustmentUs; this.lastResetPositionUs = lastResetPositionUs; } @@ -765,7 +761,7 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video return false; } - maybeSetStreamOffsetChange(bufferPresentationTimeUs); + maybeSetStreamTimestampInfo(bufferPresentationTimeUs); lastBufferPresentationTimeUs = bufferPresentationTimeUs; if (isLastFrame) { finalBufferPresentationTimeUs = bufferPresentationTimeUs; @@ -798,7 +794,7 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video long lastBufferPresentationTimeUs = copyTimestampIterator.getLastTimestampUs() - inputBufferTimestampAdjustmentUs; checkState(lastBufferPresentationTimeUs != C.TIME_UNSET); - maybeSetStreamOffsetChange(bufferPresentationTimeUs); + maybeSetStreamTimestampInfo(bufferPresentationTimeUs); this.lastBufferPresentationTimeUs = lastBufferPresentationTimeUs; finalBufferPresentationTimeUs = lastBufferPresentationTimeUs; return true; @@ -821,13 +817,11 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video // Other methods - private void maybeSetStreamOffsetChange(long bufferPresentationTimeUs) { - if (pendingInputStreamOffsetChange) { - PlaybackVideoGraphWrapper.this.onStreamOffsetChange( - inputBufferTimestampAdjustmentUs, - bufferPresentationTimeUs, - /* streamOffsetUs= */ inputStreamOffsetUs); - pendingInputStreamOffsetChange = false; + private void maybeSetStreamTimestampInfo(long bufferPresentationTimeUs) { + if (pendingInputStreamTimestampInfoChange) { + PlaybackVideoGraphWrapper.this.onStreamTimestampInfoChange( + inputBufferTimestampAdjustmentUs, bufferPresentationTimeUs, inputStreamStartPositionUs); + pendingInputStreamTimestampInfoChange = false; } } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameRenderControl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameRenderControl.java index e7749e8b7e..145699b178 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameRenderControl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameRenderControl.java @@ -65,7 +65,7 @@ import androidx.media3.exoplayer.ExoPlaybackException; private final VideoFrameReleaseControl videoFrameReleaseControl; private final VideoFrameReleaseControl.FrameReleaseInfo videoFrameReleaseInfo; private final TimedValueQueue videoSizeChanges; - private final TimedValueQueue streamOffsets; + private final TimedValueQueue streamStartPositionsUs; private final LongArrayQueue presentationTimestampsUs; /** @@ -76,7 +76,7 @@ import androidx.media3.exoplayer.ExoPlaybackException; @Nullable private VideoSize pendingOutputVideoSize; private VideoSize reportedVideoSize; - private long outputStreamOffsetUs; + private long outputStreamStartPositionUs; private long lastPresentationTimeUs; /** Creates an instance. */ @@ -86,7 +86,7 @@ import androidx.media3.exoplayer.ExoPlaybackException; this.videoFrameReleaseControl = videoFrameReleaseControl; videoFrameReleaseInfo = new VideoFrameReleaseControl.FrameReleaseInfo(); videoSizeChanges = new TimedValueQueue<>(); - streamOffsets = new TimedValueQueue<>(); + streamStartPositionsUs = new TimedValueQueue<>(); presentationTimestampsUs = new LongArrayQueue(); reportedVideoSize = VideoSize.UNKNOWN; lastPresentationTimeUs = C.TIME_UNSET; @@ -96,13 +96,13 @@ import androidx.media3.exoplayer.ExoPlaybackException; public void flush() { presentationTimestampsUs.clear(); lastPresentationTimeUs = C.TIME_UNSET; - if (streamOffsets.size() > 0) { - // There is a pending streaming offset change. If seeking within the same stream, keep the - // pending offset with timestamp zero ensures the offset is applied on the frames after - // flushing. Otherwise if seeking to another stream, a new offset will be set before a new - // frame arrives so we'll be able to apply the new offset. - long lastStreamOffset = getLastAndClear(streamOffsets); - streamOffsets.add(/* timestamp= */ 0, lastStreamOffset); + if (streamStartPositionsUs.size() > 0) { + // There is a pending streaming start position change. If seeking within the same stream, keep + // the pending start position with timestamp zero ensures the start position is applied on the + // frames after flushing. Otherwise if seeking to another stream, a new start position will be + // set before a new frame arrives so we'll be able to apply the new start position. + long lastStartPositionUs = getLastAndClear(streamStartPositionsUs); + streamStartPositionsUs.add(/* timestamp= */ 0, lastStartPositionUs); } if (pendingOutputVideoSize == null) { if (videoSizeChanges.size() > 0) { @@ -139,8 +139,8 @@ import androidx.media3.exoplayer.ExoPlaybackException; public void render(long positionUs, long elapsedRealtimeUs) throws ExoPlaybackException { while (!presentationTimestampsUs.isEmpty()) { long presentationTimeUs = presentationTimestampsUs.element(); - // Check whether this buffer comes with a new stream offset. - if (maybeUpdateOutputStreamOffset(presentationTimeUs)) { + // Check whether this buffer comes with a new stream start position. + if (maybeUpdateOutputStreamStartPosition(presentationTimeUs)) { videoFrameReleaseControl.onProcessedStreamChange(); } @VideoFrameReleaseControl.FrameReleaseAction @@ -149,7 +149,7 @@ import androidx.media3.exoplayer.ExoPlaybackException; presentationTimeUs, positionUs, elapsedRealtimeUs, - outputStreamOffsetUs, + outputStreamStartPositionUs, /* isLastFrame= */ false, videoFrameReleaseInfo); switch (frameReleaseAction) { @@ -195,8 +195,8 @@ import androidx.media3.exoplayer.ExoPlaybackException; // TODO b/257464707 - Support extensively modified media. } - public void onStreamOffsetChange(long presentationTimeUs, long streamOffsetUs) { - streamOffsets.add(presentationTimeUs, streamOffsetUs); + public void onStreamStartPositionChange(long presentationTimeUs, long streamStartPositionUs) { + streamStartPositionsUs.add(presentationTimeUs, streamStartPositionUs); } private void dropFrame() { @@ -219,10 +219,12 @@ import androidx.media3.exoplayer.ExoPlaybackException; renderTimeNs, presentationTimeUs, videoFrameReleaseControl.onFrameReleasedIsFirstFrame()); } - private boolean maybeUpdateOutputStreamOffset(long presentationTimeUs) { - @Nullable Long newOutputStreamOffsetUs = streamOffsets.pollFloor(presentationTimeUs); - if (newOutputStreamOffsetUs != null && newOutputStreamOffsetUs != outputStreamOffsetUs) { - outputStreamOffsetUs = newOutputStreamOffsetUs; + private boolean maybeUpdateOutputStreamStartPosition(long presentationTimeUs) { + @Nullable + Long newOutputStreamStartPositionUs = streamStartPositionsUs.pollFloor(presentationTimeUs); + if (newOutputStreamStartPositionUs != null + && newOutputStreamStartPositionUs != outputStreamStartPositionUs) { + outputStreamStartPositionUs = newOutputStreamStartPositionUs; return true; } return false; diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java index a2202783a4..54e237d9b8 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java @@ -207,17 +207,12 @@ public interface VideoSink { * * @param streamStartPositionUs The start position of the buffer presentation timestamps of the * current stream, in microseconds. - * @param streamOffsetUs The offset that is added to the buffer presentation timestamps by the - * player, in microseconds. * @param bufferTimestampAdjustmentUs The timestamp adjustment to add to the buffer presentation * timestamps to convert them to frame presentation timestamps, in microseconds. * @param lastResetPositionUs The renderer last reset position, in microseconds. */ void setStreamTimestampInfo( - long streamStartPositionUs, - long streamOffsetUs, - long bufferTimestampAdjustmentUs, - long lastResetPositionUs); + long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs); /** Sets the output surface info. */ void setOutputSurfaceInfo(Surface outputSurface, Size outputResolution); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/VideoFrameRenderControlTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/VideoFrameRenderControlTest.java index 6109b68d8d..7210e1d48c 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/VideoFrameRenderControlTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/VideoFrameRenderControlTest.java @@ -21,7 +21,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import androidx.media3.common.VideoSize; -import androidx.media3.exoplayer.ExoPlaybackException; import androidx.media3.test.utils.FakeClock; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -109,7 +108,7 @@ public class VideoFrameRenderControlTest { } @Test - public void renderFrames_withStreamOffsetSetChange_firstFrameAgain() throws Exception { + public void renderFrames_withStreamStartPositionChange_firstFrameAgain() throws Exception { VideoFrameRenderControl.FrameRenderer frameRenderer = mock(VideoFrameRenderControl.FrameRenderer.class); FakeClock clock = new FakeClock(/* isAutoAdvancing= */ false); @@ -122,8 +121,8 @@ public class VideoFrameRenderControlTest { videoFrameReleaseControl.onStarted(); videoFrameRenderControl.onOutputSizeChanged( /* width= */ VIDEO_WIDTH, /* height= */ VIDEO_HEIGHT); - videoFrameRenderControl.onStreamOffsetChange( - /* presentationTimeUs= */ 0, /* streamOffsetUs= */ 10_000); + videoFrameRenderControl.onStreamStartPositionChange( + /* presentationTimeUs= */ 0, /* streamStartPositionUs= */ 10_000); videoFrameRenderControl.onOutputFrameAvailableForRendering(/* presentationTimeUs= */ 0); videoFrameRenderControl.render(/* positionUs= */ 0, /* elapsedRealtimeUs= */ 0); @@ -131,18 +130,18 @@ public class VideoFrameRenderControlTest { inOrder .verify(frameRenderer) .onVideoSizeChanged(new VideoSize(/* width= */ VIDEO_WIDTH, /* height= */ VIDEO_HEIGHT)); - // First frame has the first stream offset. + // First frame has the first stream start position. inOrder.verify(frameRenderer).renderFrame(anyLong(), eq(0L), eq(true)); inOrder.verifyNoMoreInteractions(); // 10 milliseconds pass clock.advanceTime(/* timeDiffMs= */ 10); - videoFrameRenderControl.onStreamOffsetChange( - /* presentationTimeUs= */ 10_000, /* streamOffsetUs= */ 20_000); + videoFrameRenderControl.onStreamStartPositionChange( + /* presentationTimeUs= */ 10_000, /* streamStartPositionUs= */ 20_000); videoFrameRenderControl.onOutputFrameAvailableForRendering(/* presentationTimeUs= */ 10_000); videoFrameRenderControl.render(/* positionUs= */ 10_000, /* elapsedRealtimeUs= */ 0); - // Second frame has the second stream offset and it is also a first frame. + // Second frame has the second stream start position and it is also a first frame. inOrder .verify(frameRenderer) .renderFrame( @@ -344,8 +343,7 @@ public class VideoFrameRenderControlTest { long positionUs, long elapsedRealtimeUs, boolean isLastFrame, - boolean treatDroppedBuffersAsSkipped) - throws ExoPlaybackException { + boolean treatDroppedBuffersAsSkipped) { return shouldIgnoreFrames; } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java b/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java index dfdff1b028..678317816c 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java @@ -172,17 +172,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void setStreamTimestampInfo( - long streamStartPositionUs, - long streamOffsetUs, - long bufferTimestampAdjustmentUs, - long lastResetPositionUs) { + long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { executeOrDelay( videoSink -> videoSink.setStreamTimestampInfo( - streamStartPositionUs, - streamOffsetUs, - bufferTimestampAdjustmentUs, - lastResetPositionUs)); + streamStartPositionUs, bufferTimestampAdjustmentUs, lastResetPositionUs)); } @Override diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java index 5efb3bab38..a3ccd8d3d0 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java @@ -460,7 +460,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; videoSink.setPendingVideoEffects(videoEffects); videoSink.setStreamTimestampInfo( streamStartPositionUs, - getStreamOffsetUs(), /* bufferTimestampAdjustmentUs= */ offsetToCompositionTimeUs, getLastResetPositionUs()); videoSink.onInputStreamChanged(