From 7996766b225a7df02c2b92ec695d0dfb7bcffc2b Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 26 Jun 2023 09:41:47 +0000 Subject: [PATCH] Rollback of https://github.com/androidx/media/commit/b69b33206e1a6f5f8b192d59f9f8e883cd19faf4 *** Original commit *** Mark output sample as decode-only based on start time We currently do the same check on the input timestamps and expect the output timestamps to match. Some codecs produce samples with modified timestamps and the logic is a lot safer when the comparison with the start time is done on the output side of the codec. Issue: google/ExoPlayer#11000 *** PiperOrigin-RevId: 543379665 --- RELEASENOTES.md | 3 - .../media3/exoplayer/BaseRenderer.java | 2 +- .../mediacodec/MediaCodecRenderer.java | 22 +++- .../mediacodec/MediaCodecRendererTest.java | 106 ------------------ .../video/MediaCodecVideoRendererTest.java | 2 +- 5 files changed, 23 insertions(+), 112 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index c1dee68696..45b2881da8 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -25,9 +25,6 @@ playback when the playback suppression due to no suitable output has been enabled via `ExoPlayer.Builder.setSuppressPlaybackOnUnsuitableOutput`. - * Fix seeking issues in AC4 streams caused by not identifying decode-only - samples correctly - ([#11000](https://github.com/google/ExoPlayer/issues/11000)). * Fix issue in `PlaybackStatsListener` where spurious `PlaybackStats` are created after the playlist is cleared. * Transformer: diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/BaseRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/BaseRenderer.java index 33b9c44e5c..b11e946e90 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/BaseRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/BaseRenderer.java @@ -112,7 +112,7 @@ public abstract class BaseRenderer implements Renderer, RendererCapabilities { state = STATE_ENABLED; onEnabled(joining, mayRenderStartOfStream); replaceStream(formats, stream, startPositionUs, offsetUs); - resetPosition(startPositionUs, joining); + resetPosition(positionUs, joining); } @Override diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java index 0933cb3f51..387ac41d92 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java @@ -84,6 +84,7 @@ import java.lang.annotation.Target; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.List; /** An abstract renderer that uses {@link MediaCodec} to decode samples for rendering. */ @@ -299,6 +300,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer { private final DecoderInputBuffer buffer; private final DecoderInputBuffer bypassSampleBuffer; private final BatchBuffer bypassBatchBuffer; + private final ArrayList decodeOnlyPresentationTimestamps; private final MediaCodec.BufferInfo outputBufferInfo; private final ArrayDeque pendingOutputStreamChanges; private final OggOpusAudioPacketizer oggOpusAudioPacketizer; @@ -399,6 +401,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer { buffer = new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_DISABLED); bypassSampleBuffer = new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_DIRECT); bypassBatchBuffer = new BatchBuffer(); + decodeOnlyPresentationTimestamps = new ArrayList<>(); outputBufferInfo = new MediaCodec.BufferInfo(); currentPlaybackSpeed = 1f; targetPlaybackSpeed = 1f; @@ -918,6 +921,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer { shouldSkipAdaptationWorkaroundOutputBuffer = false; isDecodeOnlyOutputBuffer = false; isLastOutputBuffer = false; + decodeOnlyPresentationTimestamps.clear(); largestQueuedPresentationTimeUs = C.TIME_UNSET; lastBufferInStreamPresentationTimeUs = C.TIME_UNSET; lastProcessedOutputBufferTimeUs = C.TIME_UNSET; @@ -1374,6 +1378,9 @@ public abstract class MediaCodecRenderer extends BaseRenderer { c2Mp3TimestampTracker.getLastOutputBufferPresentationTimeUs(inputFormat)); } + if (buffer.isDecodeOnly()) { + decodeOnlyPresentationTimestamps.add(presentationTimeUs); + } if (waitingForFirstSampleInFormat) { if (!pendingOutputStreamChanges.isEmpty()) { pendingOutputStreamChanges.peekLast().formatQueue.add(presentationTimeUs, inputFormat); @@ -1903,7 +1910,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer { && largestQueuedPresentationTimeUs != C.TIME_UNSET) { outputBufferInfo.presentationTimeUs = largestQueuedPresentationTimeUs; } - isDecodeOnlyOutputBuffer = outputBufferInfo.presentationTimeUs < getLastResetPositionUs(); + isDecodeOnlyOutputBuffer = isDecodeOnlyBuffer(outputBufferInfo.presentationTimeUs); isLastOutputBuffer = lastBufferInStreamPresentationTimeUs == outputBufferInfo.presentationTimeUs; updateOutputFormatForTime(outputBufferInfo.presentationTimeUs); @@ -2194,6 +2201,19 @@ public abstract class MediaCodecRenderer extends BaseRenderer { maybeInitCodecOrBypass(); } + private boolean isDecodeOnlyBuffer(long presentationTimeUs) { + // We avoid using decodeOnlyPresentationTimestamps.remove(presentationTimeUs) because it would + // box presentationTimeUs, creating a Long object that would need to be garbage collected. + int size = decodeOnlyPresentationTimestamps.size(); + for (int i = 0; i < size; i++) { + if (decodeOnlyPresentationTimestamps.get(i) == presentationTimeUs) { + decodeOnlyPresentationTimestamps.remove(i); + return true; + } + } + return false; + } + @RequiresApi(23) private void updateDrmSessionV23() throws ExoPlaybackException { CryptoConfig cryptoConfig = sourceDrmSession.getCryptoConfig(); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/mediacodec/MediaCodecRendererTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/mediacodec/MediaCodecRendererTest.java index 241c0a2ffd..b27a3d3106 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/mediacodec/MediaCodecRendererTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/mediacodec/MediaCodecRendererTest.java @@ -19,9 +19,6 @@ import static androidx.media3.exoplayer.DecoderReuseEvaluation.REUSE_RESULT_YES_ import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.END_OF_STREAM_ITEM; import static androidx.media3.test.utils.FakeSampleStream.FakeSampleStreamItem.oneByteSample; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.spy; @@ -326,90 +323,6 @@ public class MediaCodecRendererTest { inOrder.verify(renderer).onProcessedOutputBuffer(400); } - @Test - public void render_afterEnableWithStartPositionUs_skipsSamplesBeforeStartPositionUs() - throws Exception { - Format format = - new Format.Builder().setSampleMimeType(MimeTypes.AUDIO_AAC).setAverageBitrate(1000).build(); - FakeSampleStream fakeSampleStream = - createFakeSampleStream(format, /* sampleTimesUs...= */ 0, 100, 200, 300, 400, 500); - MediaCodecRenderer renderer = spy(new TestRenderer()); - renderer.init(/* index= */ 0, PlayerId.UNSET); - - renderer.enable( - RendererConfiguration.DEFAULT, - new Format[] {format}, - fakeSampleStream, - /* positionUs= */ 0, - /* joining= */ false, - /* mayRenderStartOfStream= */ true, - /* startPositionUs= */ 300, - /* offsetUs= */ 0); - renderer.start(); - renderer.setCurrentStreamFinal(); - long positionUs = 0; - while (!renderer.isEnded()) { - renderer.render(positionUs, SystemClock.elapsedRealtime()); - positionUs += 100; - } - - InOrder inOrder = inOrder(renderer); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 0, /* isDecodeOnly= */ true); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 100, /* isDecodeOnly= */ true); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 200, /* isDecodeOnly= */ true); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 300, /* isDecodeOnly= */ false); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 400, /* isDecodeOnly= */ false); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 500, /* isDecodeOnly= */ false); - } - - @Test - public void render_afterPositionReset_skipsSamplesBeforeStartPositionUs() throws Exception { - Format format = - new Format.Builder().setSampleMimeType(MimeTypes.AUDIO_AAC).setAverageBitrate(1000).build(); - FakeSampleStream fakeSampleStream = - createFakeSampleStream(format, /* sampleTimesUs...= */ 0, 100, 200, 300, 400, 500); - MediaCodecRenderer renderer = spy(new TestRenderer()); - renderer.init(/* index= */ 0, PlayerId.UNSET); - renderer.enable( - RendererConfiguration.DEFAULT, - new Format[] {format}, - fakeSampleStream, - /* positionUs= */ 0, - /* joining= */ false, - /* mayRenderStartOfStream= */ true, - /* startPositionUs= */ 400, - /* offsetUs= */ 0); - renderer.start(); - - renderer.resetPosition(/* positionUs= */ 200); - renderer.setCurrentStreamFinal(); - long positionUs = 0; - while (!renderer.isEnded()) { - renderer.render(positionUs, SystemClock.elapsedRealtime()); - positionUs += 100; - } - - InOrder inOrder = inOrder(renderer); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 0, /* isDecodeOnly= */ true); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 100, /* isDecodeOnly= */ true); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 200, /* isDecodeOnly= */ false); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 300, /* isDecodeOnly= */ false); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 400, /* isDecodeOnly= */ false); - verifyProcessOutputBufferDecodeOnly( - inOrder, renderer, /* presentationTimeUs= */ 500, /* isDecodeOnly= */ false); - } - private FakeSampleStream createFakeSampleStream(Format format, long... sampleTimesUs) { ImmutableList.Builder sampleListBuilder = ImmutableList.builder(); @@ -516,23 +429,4 @@ public class MediaCodecRendererTest { /* discardReasons= */ 0); } } - - private static void verifyProcessOutputBufferDecodeOnly( - InOrder inOrder, MediaCodecRenderer renderer, long presentationTimeUs, boolean isDecodeOnly) - throws Exception { - inOrder - .verify(renderer) - .processOutputBuffer( - anyLong(), - anyLong(), - any(), - any(), - anyInt(), - anyInt(), - anyInt(), - eq(presentationTimeUs), - eq(isDecodeOnly), - anyBoolean(), - any()); - } } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java index 634d467d6c..5d6dac43d8 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java @@ -255,7 +255,7 @@ public class MediaCodecVideoRendererTest { /* positionUs= */ 0, /* joining= */ false, /* mayRenderStartOfStream= */ true, - /* startPositionUs= */ 30_000, + /* startPositionUs= */ 0, /* offsetUs= */ 0); mediaCodecVideoRenderer.start();