From 7474547e0ff0ceb768860d42541a2f39b3698ae1 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 6 Jul 2020 16:41:31 +0100 Subject: [PATCH] Don't clear the exception in SimpleDecoder.flush() Clearing the exception puts the SimpleDecoder into a silent failure state - the decoder thread is dead (because decode() has returned false) but it's still possible to queue buffers to the decoder (they just never get decoded). This partially reverts https://github.com/google/ExoPlayer/commit/4107375c9d21136090d0071b04a498024eec6a4c Also always recreate the decoder when handling an error in TextRenderer This ensures we can try and decode a later subtitle sample after encountering a decode error. This behaviour is what nulling out the exception in SimpleDecoder.flush() was trying to achieve. We need to ensure we don't start passing data to the new decoder until we've hit the next key frame, so we throw away any non-keyframe samples inside TextRenderer#render(). Issue: #7590 PiperOrigin-RevId: 319785908 --- RELEASENOTES.md | 5 +++ .../exoplayer2/decoder/SimpleDecoder.java | 1 - .../android/exoplayer2/text/TextRenderer.java | 37 +++++++++++-------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 950d7539f0..a4347fd79f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -92,6 +92,8 @@ ongoing load should be canceled. Only supported by HLS streams so far. ([#2848](https://github.com/google/ExoPlayer/issues/2848)). * Remove throws clause from Renderer.stop. + * Don't clear `exception` in `SimpleDecoder#flush()` + ([#7590](https://github.com/google/ExoPlayer/issues/7590)). * Video: Pass frame rate hint to `Surface.setFrameRate` on Android R devices. * Track selection: * Add `Player.getTrackSelector`. @@ -138,6 +140,9 @@ text lines to grid of viewport lines, and ignore `Cue.lineAnchor`. * Check `CaptionManager.isEnabled()` before using it for user-specified font-scaling. + * Recreate the decoder when handling & swallowing decode errors in + `TextRenderer` + ([#7590](https://github.com/google/ExoPlayer/issues/7590)). * DRM: * Add support for attaching DRM sessions to clear content in the demo app. * Remove `DrmSessionManager` references from all renderers. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/decoder/SimpleDecoder.java b/library/core/src/main/java/com/google/android/exoplayer2/decoder/SimpleDecoder.java index beaea6cf37..da69924e03 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/decoder/SimpleDecoder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/decoder/SimpleDecoder.java @@ -153,7 +153,6 @@ public abstract class SimpleDecoder< while (!queuedOutputBuffers.isEmpty()) { queuedOutputBuffers.removeFirst().release(); } - exception = null; } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java index b8b4d7de6e..92e2ffc7c1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java @@ -82,6 +82,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { private boolean inputStreamEnded; private boolean outputStreamEnded; + private boolean waitingForKeyFrame; @ReplacementState private int decoderReplacementState; @Nullable private Format streamFormat; @Nullable private SubtitleDecoder decoder; @@ -145,15 +146,21 @@ public final class TextRenderer extends BaseRenderer implements Callback { if (decoder != null) { decoderReplacementState = REPLACEMENT_STATE_SIGNAL_END_OF_STREAM; } else { - decoder = decoderFactory.createDecoder(streamFormat); + initDecoder(); } } @Override protected void onPositionReset(long positionUs, boolean joining) { + clearOutput(); inputStreamEnded = false; outputStreamEnded = false; - resetOutputAndDecoder(); + if (decoderReplacementState != REPLACEMENT_STATE_NONE) { + replaceDecoder(); + } else { + releaseBuffers(); + decoder.flush(); + } } @Override @@ -239,12 +246,16 @@ public final class TextRenderer extends BaseRenderer implements Callback { if (result == C.RESULT_BUFFER_READ) { if (nextInputBuffer.isEndOfStream()) { inputStreamEnded = true; + waitingForKeyFrame = false; } else { nextInputBuffer.subsampleOffsetUs = formatHolder.format.subsampleOffsetUs; nextInputBuffer.flip(); + waitingForKeyFrame &= !nextInputBuffer.isKeyFrame(); + } + if (!waitingForKeyFrame) { + decoder.queueInputBuffer(nextInputBuffer); + nextInputBuffer = null; } - decoder.queueInputBuffer(nextInputBuffer); - nextInputBuffer = null; } else if (result == C.RESULT_NOTHING_READ) { return; } @@ -294,9 +305,14 @@ public final class TextRenderer extends BaseRenderer implements Callback { decoderReplacementState = REPLACEMENT_STATE_NONE; } + private void initDecoder() { + waitingForKeyFrame = true; + decoder = decoderFactory.createDecoder(streamFormat); + } + private void replaceDecoder() { releaseDecoder(); - decoder = decoderFactory.createDecoder(streamFormat); + initDecoder(); } private long getNextEventTime() { @@ -341,16 +357,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { */ private void handleDecoderError(SubtitleDecoderException e) { Log.e(TAG, "Subtitle decoding failed. streamFormat=" + streamFormat, e); - resetOutputAndDecoder(); - } - - private void resetOutputAndDecoder() { clearOutput(); - if (decoderReplacementState != REPLACEMENT_STATE_NONE) { - replaceDecoder(); - } else { - releaseBuffers(); - decoder.flush(); - } + replaceDecoder(); } }