From a879bae1ee2c684e8100f50bcff39655d46a2e8d Mon Sep 17 00:00:00 2001 From: rohks Date: Tue, 3 Oct 2023 10:04:21 -0700 Subject: [PATCH] Add nullness annotations to `DecoderVideoRenderer` Also fixed a bug where format queue was polled with wrong timestamp value. #fixit PiperOrigin-RevId: 570420304 --- .../exoplayer/video/DecoderVideoRenderer.java | 80 ++++++++++--------- .../video/MediaCodecVideoRenderer.java | 1 - 2 files changed, 44 insertions(+), 37 deletions(-) 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 b7242928df..c0f7e74723 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 @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer.video; +import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Util.msToUs; import static androidx.media3.exoplayer.DecoderReuseEvaluation.DISCARD_REASON_DRM_SESSION_CHANGED; import static androidx.media3.exoplayer.DecoderReuseEvaluation.DISCARD_REASON_REUSE_NOT_IMPLEMENTED; @@ -116,16 +117,16 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { private final TimedValueQueue formatQueue; private final DecoderInputBuffer flagsOnlyBuffer; - private Format inputFormat; - private Format outputFormat; + @Nullable private Format inputFormat; + @Nullable private Format outputFormat; @Nullable private Decoder< DecoderInputBuffer, ? extends VideoDecoderOutputBuffer, ? extends DecoderException> decoder; - private DecoderInputBuffer inputBuffer; - private VideoDecoderOutputBuffer outputBuffer; + @Nullable private DecoderInputBuffer inputBuffer; + @Nullable private VideoDecoderOutputBuffer outputBuffer; private @VideoOutputMode int outputMode; @Nullable private Object output; @Nullable private Surface outputSurface; @@ -175,13 +176,13 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { this.allowedJoiningTimeMs = allowedJoiningTimeMs; this.maxDroppedFramesToNotify = maxDroppedFramesToNotify; joiningDeadlineMs = C.TIME_UNSET; - clearReportedVideoSize(); formatQueue = new TimedValueQueue<>(); flagsOnlyBuffer = DecoderInputBuffer.newNoDataInstance(); eventDispatcher = new EventDispatcher(eventHandler, eventListener); decoderReinitializationState = REINITIALIZATION_STATE_NONE; outputMode = C.VIDEO_OUTPUT_MODE_NONE; firstFrameState = C.FIRST_FRAME_NOT_RENDERED_ONLY_ALLOWED_IF_STARTED; + decoderCounters = new DecoderCounters(); } // BaseRenderer implementation. @@ -324,7 +325,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { @Override protected void onDisabled() { inputFormat = null; - clearReportedVideoSize(); + reportedVideoSize = null; lowerFirstFrameState(C.FIRST_FRAME_NOT_RENDERED_ONLY_ALLOWED_IF_STARTED); try { setSourceDrmSession(null); @@ -365,7 +366,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { outputBuffer.release(); outputBuffer = null; } - decoder.flush(); + checkNotNull(decoder).flush(); decoderReceivedBuffers = false; } } @@ -403,7 +404,8 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { if (decoder == null) { maybeInitDecoder(); - eventDispatcher.inputFormatChanged(inputFormat, /* decoderReuseEvaluation= */ null); + eventDispatcher.inputFormatChanged( + checkNotNull(inputFormat), /* decoderReuseEvaluation= */ null); return; } @@ -412,12 +414,12 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { evaluation = new DecoderReuseEvaluation( decoder.getName(), - oldFormat, + checkNotNull(oldFormat), newFormat, REUSE_RESULT_NO, DISCARD_REASON_DRM_SESSION_CHANGED); } else { - evaluation = canReuseDecoder(decoder.getName(), oldFormat, newFormat); + evaluation = canReuseDecoder(decoder.getName(), checkNotNull(oldFormat), newFormat); } if (evaluation.result == REUSE_RESULT_NO) { @@ -430,7 +432,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { maybeInitDecoder(); } } - eventDispatcher.inputFormatChanged(inputFormat, evaluation); + eventDispatcher.inputFormatChanged(checkNotNull(inputFormat), evaluation); } /** @@ -598,9 +600,9 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { } else { maybeNotifyVideoSizeChanged(outputBuffer.width, outputBuffer.height); if (renderYuv) { - outputBufferRenderer.setOutputBuffer(outputBuffer); + checkNotNull(outputBufferRenderer).setOutputBuffer(outputBuffer); } else { - renderOutputBufferToSurface(outputBuffer, outputSurface); + renderOutputBufferToSurface(outputBuffer, checkNotNull(outputSurface)); } consecutiveDroppedFrameCount = 0; decoderCounters.renderedOutputBufferCount++; @@ -715,11 +717,11 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { try { long decoderInitializingTimestamp = SystemClock.elapsedRealtime(); - decoder = createDecoder(inputFormat, cryptoConfig); + decoder = createDecoder(checkNotNull(inputFormat), cryptoConfig); setDecoderOutputMode(outputMode); long decoderInitializedTimestamp = SystemClock.elapsedRealtime(); eventDispatcher.decoderInitialized( - decoder.getName(), + checkNotNull(decoder).getName(), decoderInitializedTimestamp, decoderInitializedTimestamp - decoderInitializingTimestamp); decoderCounters.decoderInitCount++; @@ -749,10 +751,11 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { } } + DecoderInputBuffer inputBuffer = checkNotNull(this.inputBuffer); if (decoderReinitializationState == REINITIALIZATION_STATE_SIGNAL_END_OF_STREAM) { inputBuffer.setFlags(C.BUFFER_FLAG_END_OF_STREAM); - decoder.queueInputBuffer(inputBuffer); - inputBuffer = null; + checkNotNull(decoder).queueInputBuffer(inputBuffer); + this.inputBuffer = null; decoderReinitializationState = REINITIALIZATION_STATE_WAIT_END_OF_STREAM; return false; } @@ -767,12 +770,12 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { case C.RESULT_BUFFER_READ: if (inputBuffer.isEndOfStream()) { inputStreamEnded = true; - decoder.queueInputBuffer(inputBuffer); - inputBuffer = null; + checkNotNull(decoder).queueInputBuffer(inputBuffer); + this.inputBuffer = null; return false; } if (waitingForFirstSampleInFormat) { - formatQueue.add(inputBuffer.timeUs, inputFormat); + formatQueue.add(inputBuffer.timeUs, checkNotNull(inputFormat)); waitingForFirstSampleInFormat = false; } if (inputBuffer.timeUs < getLastResetPositionUs()) { @@ -781,11 +784,11 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { inputBuffer.flip(); inputBuffer.format = inputFormat; onQueueInputBuffer(inputBuffer); - decoder.queueInputBuffer(inputBuffer); + checkNotNull(decoder).queueInputBuffer(inputBuffer); buffersInCodecCount++; decoderReceivedBuffers = true; decoderCounters.queuedInputBufferCount++; - inputBuffer = null; + this.inputBuffer = null; return true; default: throw new IllegalStateException(); @@ -805,7 +808,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { private boolean drainOutputBuffer(long positionUs, long elapsedRealtimeUs) throws ExoPlaybackException, DecoderException { if (outputBuffer == null) { - outputBuffer = decoder.dequeueOutputBuffer(); + outputBuffer = checkNotNull(decoder).dequeueOutputBuffer(); if (outputBuffer == null) { return false; } @@ -828,7 +831,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { boolean processedOutputBuffer = processOutputBuffer(positionUs, elapsedRealtimeUs); if (processedOutputBuffer) { - onProcessedOutputBuffer(outputBuffer.timeUs); + onProcessedOutputBuffer(checkNotNull(outputBuffer).timeUs); outputBuffer = null; } return processedOutputBuffer; @@ -850,7 +853,9 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { initialPositionUs = positionUs; } - long earlyUs = outputBuffer.timeUs - positionUs; + VideoDecoderOutputBuffer outputBuffer = checkNotNull(this.outputBuffer); + long bufferTimeUs = outputBuffer.timeUs; + long earlyUs = bufferTimeUs - positionUs; if (!hasOutput()) { // Skip frames in sync with playback, so we'll be at the right frame if the mode changes. if (isBufferLate(earlyUs)) { @@ -860,14 +865,19 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { return false; } - long presentationTimeUs = outputBuffer.timeUs - outputStreamOffsetUs; - Format format = formatQueue.pollFloor(presentationTimeUs); + Format format = formatQueue.pollFloor(bufferTimeUs); if (format != null) { outputFormat = format; + } else if (outputFormat == null) { + // After a stream change or after the initial start, there should be an input format change + // which we've not found. Check the Format queue in case the corresponding presentation + // timestamp is greater than bufferTimeUs + outputFormat = formatQueue.pollFirst(); } + long presentationTimeUs = bufferTimeUs - outputStreamOffsetUs; if (shouldForceRender(earlyUs)) { - renderOutputBuffer(outputBuffer, presentationTimeUs, outputFormat); + renderOutputBuffer(outputBuffer, presentationTimeUs, checkNotNull(outputFormat)); return true; } @@ -886,7 +896,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { } if (earlyUs < 30000) { - renderOutputBuffer(outputBuffer, presentationTimeUs, outputFormat); + renderOutputBuffer(outputBuffer, presentationTimeUs, checkNotNull(outputFormat)); return true; } @@ -925,7 +935,7 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { } private void onOutputRemoved() { - clearReportedVideoSize(); + reportedVideoSize = null; lowerFirstFrameState(C.FIRST_FRAME_NOT_RENDERED); } @@ -950,20 +960,18 @@ public abstract class DecoderVideoRenderer extends BaseRenderer { private void maybeNotifyRenderedFirstFrame() { if (firstFrameState != C.FIRST_FRAME_RENDERED) { firstFrameState = C.FIRST_FRAME_RENDERED; - eventDispatcher.renderedFirstFrame(output); + if (output != null) { + eventDispatcher.renderedFirstFrame(output); + } } } private void maybeRenotifyRenderedFirstFrame() { - if (firstFrameState == C.FIRST_FRAME_RENDERED) { + if (firstFrameState == C.FIRST_FRAME_RENDERED && output != null) { eventDispatcher.renderedFirstFrame(output); } } - private void clearReportedVideoSize() { - reportedVideoSize = null; - } - private void maybeNotifyVideoSizeChanged(int width, int height) { if (reportedVideoSize == null || reportedVideoSize.width != width 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 37c6f8ed82..86b5ca2b5d 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 @@ -408,7 +408,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer implements Video decodedVideoSize = VideoSize.UNKNOWN; tunnelingAudioSessionId = C.AUDIO_SESSION_ID_UNSET; firstFrameState = C.FIRST_FRAME_NOT_RENDERED_ONLY_ALLOWED_IF_STARTED; - reportedVideoSize = null; } @Override