From dcc69056bfe57d32ddd6e5979a78ba02f16a9f09 Mon Sep 17 00:00:00 2001 From: ibaker Date: Tue, 7 Dec 2021 11:25:22 +0000 Subject: [PATCH] Fix how drop-to-keyframe events are recorded in `DecoderCounters` The existing code creates an imbalance between `inputBufferCount` and `droppedBufferCount` by adding 'dropped source buffers' to `droppedBufferCount` but not to `inputBufferCount`. This results in assertion failures in `DashTestRunner`. PiperOrigin-RevId: 414672175 --- .../media3/exoplayer/DecoderCounters.java | 34 +++++++++++++++---- .../exoplayer/video/DecoderVideoRenderer.java | 25 +++++++++----- .../video/MediaCodecVideoRenderer.java | 31 ++++++++++------- .../playback/gts/DashTestRunner.java | 14 ++------ .../test/utils/DecoderCountersUtil.java | 18 +++++++++- 5 files changed, 81 insertions(+), 41 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/DecoderCounters.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/DecoderCounters.java index 6306387027..5ccd8376b4 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/DecoderCounters.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/DecoderCounters.java @@ -33,12 +33,12 @@ public final class DecoderCounters { public int decoderInitCount; /** The number of times a decoder has been released. */ public int decoderReleaseCount; - /** The number of queued input buffers. */ + /** The number of input buffers queued to the decoder. */ public int inputBufferCount; /** * The number of skipped input buffers. * - *

A skipped input buffer is an input buffer that was deliberately not sent to the decoder. + *

A skipped input buffer is an input buffer that was deliberately not queued to the decoder. */ public int skippedInputBufferCount; /** The number of rendered output buffers. */ @@ -46,16 +46,28 @@ public final class DecoderCounters { /** * The number of skipped output buffers. * - *

A skipped output buffer is an output buffer that was deliberately not rendered. + *

A skipped output buffer is an output buffer that was deliberately not rendered. This + * includes buffers that were never dequeued from the decoder and instead skipped while 'inside' + * the codec due to a flush. */ public int skippedOutputBufferCount; /** * The number of dropped buffers. * - *

A dropped buffer is an buffer that was supposed to be decoded/rendered, but was instead + *

A dropped buffer is a buffer that was supposed to be decoded/rendered, but was instead * dropped because it could not be rendered in time. + * + *

This includes all of {@link #droppedInputBufferCount} in addition to buffers dropped after + * being queued to the decoder. */ public int droppedBufferCount; + /** + * The number of input buffers dropped. + * + *

A dropped input buffer is a buffer that was not queued to the decoder because it would not + * be rendered in time. + */ + public int droppedInputBufferCount; /** * The maximum number of dropped buffers without an interleaving rendered output buffer. * @@ -65,9 +77,16 @@ public final class DecoderCounters { /** * The number of times all buffers to a keyframe were dropped. * - *

Each time buffers to a keyframe are dropped, this counter is increased by one, and the - * dropped buffer counters are increased by one (for the current output buffer) plus the number of - * buffers dropped from the source to advance to the keyframe. + *

Each time buffers to a keyframe are dropped: + * + *

*/ public int droppedToKeyframeCount; /** @@ -114,6 +133,7 @@ public final class DecoderCounters { renderedOutputBufferCount += other.renderedOutputBufferCount; skippedOutputBufferCount += other.skippedOutputBufferCount; droppedBufferCount += other.droppedBufferCount; + droppedInputBufferCount += other.droppedInputBufferCount; maxConsecutiveDroppedBufferCount = max(maxConsecutiveDroppedBufferCount, other.maxConsecutiveDroppedBufferCount); droppedToKeyframeCount += other.droppedToKeyframeCount; diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DecoderVideoRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DecoderVideoRenderer.java index c0bb1c11b2..310f6bafe6 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DecoderVideoRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DecoderVideoRenderer.java @@ -488,7 +488,8 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { * @param outputBuffer The output buffer to drop. */ protected void dropOutputBuffer(VideoDecoderOutputBuffer outputBuffer) { - updateDroppedBufferCounters(1); + updateDroppedBufferCounters( + /* droppedInputBufferCount= */ 0, /* droppedDecoderBufferCount= */ 1); outputBuffer.release(); } @@ -509,21 +510,27 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { decoderCounters.droppedToKeyframeCount++; // We dropped some buffers to catch up, so update the decoder counters and flush the decoder, // which releases all pending buffers buffers including the current output buffer. - updateDroppedBufferCounters(buffersInCodecCount + droppedSourceBufferCount); + updateDroppedBufferCounters( + droppedSourceBufferCount, /* droppedDecoderBufferCount= */ buffersInCodecCount); flushDecoder(); return true; } /** - * Updates decoder counters to reflect that {@code droppedBufferCount} additional buffers were - * dropped. + * Updates local counters and {@link #decoderCounters} to reflect that buffers were dropped. * - * @param droppedBufferCount The number of additional dropped buffers. + * @param droppedInputBufferCount The number of buffers dropped from the source before being + * passed to the decoder. + * @param droppedDecoderBufferCount The number of buffers dropped after being passed to the + * decoder. */ - protected void updateDroppedBufferCounters(int droppedBufferCount) { - decoderCounters.droppedBufferCount += droppedBufferCount; - droppedFrames += droppedBufferCount; - consecutiveDroppedFrameCount += droppedBufferCount; + protected void updateDroppedBufferCounters( + int droppedInputBufferCount, int droppedDecoderBufferCount) { + decoderCounters.droppedInputBufferCount += droppedInputBufferCount; + int totalDroppedBufferCount = droppedInputBufferCount + droppedDecoderBufferCount; + decoderCounters.droppedBufferCount += totalDroppedBufferCount; + droppedFrames += totalDroppedBufferCount; + consecutiveDroppedFrameCount += totalDroppedBufferCount; decoderCounters.maxConsecutiveDroppedBufferCount = max(consecutiveDroppedFrameCount, decoderCounters.maxConsecutiveDroppedBufferCount); if (maxDroppedFramesToNotify > 0 && droppedFrames >= maxDroppedFramesToNotify) { 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 5622918934..325657ddff 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 @@ -1111,7 +1111,8 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer { TraceUtil.beginSection("dropVideoBuffer"); codec.releaseOutputBuffer(index, false); TraceUtil.endSection(); - updateDroppedBufferCounters(1); + updateDroppedBufferCounters( + /* droppedInputBufferCount= */ 0, /* droppedDecoderBufferCount= */ 1); } /** @@ -1131,29 +1132,35 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer { if (droppedSourceBufferCount == 0) { return false; } - decoderCounters.droppedToKeyframeCount++; // We dropped some buffers to catch up, so update the decoder counters and flush the codec, // which releases all pending buffers buffers including the current output buffer. - int totalDroppedBufferCount = buffersInCodecCount + droppedSourceBufferCount; if (treatDroppedBuffersAsSkipped) { - decoderCounters.skippedOutputBufferCount += totalDroppedBufferCount; + decoderCounters.skippedInputBufferCount += droppedSourceBufferCount; + decoderCounters.skippedOutputBufferCount += buffersInCodecCount; } else { - updateDroppedBufferCounters(totalDroppedBufferCount); + decoderCounters.droppedToKeyframeCount++; + updateDroppedBufferCounters( + droppedSourceBufferCount, /* droppedDecoderBufferCount= */ buffersInCodecCount); } flushOrReinitializeCodec(); return true; } /** - * Updates local counters and {@link DecoderCounters} to reflect that {@code droppedBufferCount} - * additional buffers were dropped. + * Updates local counters and {@link #decoderCounters} to reflect that buffers were dropped. * - * @param droppedBufferCount The number of additional dropped buffers. + * @param droppedInputBufferCount The number of buffers dropped from the source before being + * passed to the decoder. + * @param droppedDecoderBufferCount The number of buffers dropped after being passed to the + * decoder. */ - protected void updateDroppedBufferCounters(int droppedBufferCount) { - decoderCounters.droppedBufferCount += droppedBufferCount; - droppedFrames += droppedBufferCount; - consecutiveDroppedFrameCount += droppedBufferCount; + protected void updateDroppedBufferCounters( + int droppedInputBufferCount, int droppedDecoderBufferCount) { + decoderCounters.droppedInputBufferCount += droppedInputBufferCount; + int totalDroppedBufferCount = droppedInputBufferCount + droppedDecoderBufferCount; + decoderCounters.droppedBufferCount += totalDroppedBufferCount; + droppedFrames += totalDroppedBufferCount; + consecutiveDroppedFrameCount += totalDroppedBufferCount; decoderCounters.maxConsecutiveDroppedBufferCount = max(consecutiveDroppedFrameCount, decoderCounters.maxConsecutiveDroppedBufferCount); if (maxDroppedFramesToNotify > 0 && droppedFrames >= maxDroppedFramesToNotify) { diff --git a/libraries/test_exoplayer_playback/src/androidTest/java/androidx/media3/test/exoplayer/playback/gts/DashTestRunner.java b/libraries/test_exoplayer_playback/src/androidTest/java/androidx/media3/test/exoplayer/playback/gts/DashTestRunner.java index 772ed45de3..9f3f577770 100644 --- a/libraries/test_exoplayer_playback/src/androidTest/java/androidx/media3/test/exoplayer/playback/gts/DashTestRunner.java +++ b/libraries/test_exoplayer_playback/src/androidTest/java/androidx/media3/test/exoplayer/playback/gts/DashTestRunner.java @@ -361,18 +361,8 @@ import java.util.List; tag + AUDIO_TAG_SUFFIX, audioCounters, 0); DecoderCountersUtil.assertSkippedOutputBufferCount( tag + VIDEO_TAG_SUFFIX, videoCounters, 0); - // We allow one fewer output buffer due to the way that MediaCodecRenderer and the - // underlying decoders handle the end of stream. This should be tightened up in the future. - DecoderCountersUtil.assertTotalBufferCount( - tag + AUDIO_TAG_SUFFIX, - audioCounters, - audioCounters.inputBufferCount - 1, - audioCounters.inputBufferCount); - DecoderCountersUtil.assertTotalBufferCount( - tag + VIDEO_TAG_SUFFIX, - videoCounters, - videoCounters.inputBufferCount - 1, - videoCounters.inputBufferCount); + DecoderCountersUtil.assertTotalBufferCount(tag + AUDIO_TAG_SUFFIX, audioCounters); + DecoderCountersUtil.assertTotalBufferCount(tag + VIDEO_TAG_SUFFIX, videoCounters); } try { if (!shouldSkipDroppedOutputBufferPerformanceAssertions()) { diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/DecoderCountersUtil.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/DecoderCountersUtil.java index 4bfe720c88..af4fb08554 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/DecoderCountersUtil.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/DecoderCountersUtil.java @@ -34,7 +34,8 @@ public final class DecoderCountersUtil { */ public static int getTotalBufferCount(DecoderCounters counters) { counters.ensureUpdated(); - return counters.skippedOutputBufferCount + return counters.skippedInputBufferCount + + counters.skippedOutputBufferCount + counters.droppedBufferCount + counters.renderedOutputBufferCount; } @@ -49,6 +50,21 @@ public final class DecoderCountersUtil { .isEqualTo(expected); } + /** Asserts that the input and output values in {@code counters} are self-consistent. */ + public static void assertTotalBufferCount(String name, DecoderCounters counters) { + // We allow one fewer output buffer due to the way that MediaCodecRenderer and the + // underlying decoders handle the end of stream. This should be tightened up in the future. + int totalInputBufferCount = + counters.skippedInputBufferCount + + counters.droppedInputBufferCount + + counters.inputBufferCount; + assertTotalBufferCount( + name, + counters, + /* minCount= */ totalInputBufferCount - 1, + /* maxCount= */ totalInputBufferCount); + } + public static void assertTotalBufferCount( String name, DecoderCounters counters, int minCount, int maxCount) { int actual = getTotalBufferCount(counters);