From f238db8208af4575d7a151f94bf7b0281059f362 Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 5 Jun 2024 09:44:51 -0700 Subject: [PATCH] Exit early if buffer becomes invalid When the frame release control invalidates a buffer and returns that the buffer must be ignored, we need to exit early before performing additional checks that may result in method calls using the invalid buffer. PiperOrigin-RevId: 640555688 --- RELEASENOTES.md | 2 + .../video/MediaCodecVideoRenderer.java | 7 ++- .../video/MediaCodecVideoRendererTest.java | 54 ++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 1b10bdc3bf..5bc0f4effb 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -147,6 +147,8 @@ This release includes the following changes since the * Fix decoder fallback logic for Dolby Vision to use a compatible AV1 decoder if needed ([#1389](https://github.com/androidx/media/pull/1389)). + * Fix codec exception that may be caused by enabling a video renderer + mid-playback. * Text: * Fix issue where subtitles starting before a seek position are skipped. This issue was only introduced in Media3 1.4.0-alpha01. 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 2dfc9fe7ed..6241139781 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 @@ -1319,6 +1319,11 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer isLastBuffer, videoFrameReleaseInfo); + if (frameReleaseAction == VideoFrameReleaseControl.FRAME_RELEASE_IGNORE) { + // The buffer is no longer valid and needs to be ignored. + return false; + } + // Skip decode-only buffers, e.g. after seeking, immediately. This check must be performed after // getting the release action from the video frame release control although not necessary. // That's because the release control estimates the content frame rate from frame timestamps @@ -1370,8 +1375,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer dropOutputBuffer(codec, bufferIndex, presentationTimeUs); updateVideoFrameProcessingOffsetCounters(videoFrameReleaseInfo.getEarlyUs()); return true; - case VideoFrameReleaseControl.FRAME_RELEASE_IGNORE: - // Falls with next case. case VideoFrameReleaseControl.FRAME_RELEASE_TRY_AGAIN_LATER: return false; case VideoFrameReleaseControl.FRAME_RELEASE_SCHEDULED: 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 20a322b1d1..9ed7aa24c1 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 @@ -199,7 +199,7 @@ public class MediaCodecVideoRendererTest { } @Test - public void render_dropsLateBuffer() throws Exception { + public void render_withLateBuffer_dropsBuffer() throws Exception { FakeSampleStream fakeSampleStream = new FakeSampleStream( new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024), @@ -238,6 +238,58 @@ public class MediaCodecVideoRendererTest { verify(eventListener).onDroppedFrames(eq(1), anyLong()); } + @Test + public void render_withVeryLateBuffer_dropsBuffersUpstream() throws Exception { + FakeSampleStream fakeSampleStream = + new FakeSampleStream( + new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024), + /* mediaSourceEventDispatcher= */ null, + DrmSessionManager.DRM_UNSUPPORTED, + new DrmSessionEventListener.EventDispatcher(), + /* initialFormat= */ VIDEO_H264, + ImmutableList.of( + oneByteSample(/* timeUs= */ 0, C.BUFFER_FLAG_KEY_FRAME), // First buffer. + oneByteSample(/* timeUs= */ 20_000))); // Very late buffer. + fakeSampleStream.writeData(/* startPositionUs= */ 0); + mediaCodecVideoRenderer.enable( + RendererConfiguration.DEFAULT, + new Format[] {VIDEO_H264}, + fakeSampleStream, + /* positionUs= */ 0, + /* joining= */ false, + /* mayRenderStartOfStream= */ true, + /* startPositionUs= */ 0, + /* offsetUs= */ 0, + /* mediaPeriodId= */ new MediaSource.MediaPeriodId(new Object())); + + mediaCodecVideoRenderer.start(); + mediaCodecVideoRenderer.render(0, SystemClock.elapsedRealtime() * 1000); + mediaCodecVideoRenderer.render(10_000, SystemClock.elapsedRealtime() * 1000); + // Ensure existing buffer will be 1 second late and new (not yet read) buffers are available + // to be skipped and to skip to in the input stream. + int posUs = 1_020_000; + fakeSampleStream.append( + ImmutableList.of( + oneByteSample(/* timeUs= */ 30_000), + oneByteSample(/* timeUs= */ 1_020_000, C.BUFFER_FLAG_KEY_FRAME), + oneByteSample(/* timeUs= */ 1_200_000), + END_OF_STREAM_ITEM)); + fakeSampleStream.writeData(/* startPositionUs= */ 0); + mediaCodecVideoRenderer.setCurrentStreamFinal(); + while (!mediaCodecVideoRenderer.isEnded()) { + mediaCodecVideoRenderer.render(posUs, SystemClock.elapsedRealtime() * 1000); + posUs += 10_000; + } + shadowOf(testMainLooper).idle(); + + ArgumentCaptor argumentDecoderCounters = + ArgumentCaptor.forClass(DecoderCounters.class); + verify(eventListener).onVideoEnabled(argumentDecoderCounters.capture()); + assertThat(argumentDecoderCounters.getValue().renderedOutputBufferCount).isEqualTo(3); + assertThat(argumentDecoderCounters.getValue().droppedInputBufferCount).isEqualTo(1); + assertThat(argumentDecoderCounters.getValue().droppedToKeyframeCount).isEqualTo(1); + } + @Test public void render_withBufferLimitEqualToNumberOfSamples_rendersLastFrameAfterEndOfStream() throws Exception {