From 305ec3b4498efc0b5c9ff2b0c5acc5756568e73b Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 19 Nov 2020 16:32:15 +0000 Subject: [PATCH] Minimal fix for playback freezes when enabling tracks #8203 Background: 1. When the player has multiple audio renderers, by default they share a single AudioSink. 2. When any new renderer is enabled, all disabled renderers are reset prior to the new renderer being enabled. This is to give them a chance to free up resources in case the renderer being enabled needs them. These reset calls are expected to be no-ops for renderers that have never been enabled. The issue: The problematic case arises when there are two audio renderers and a third renderer (e.g., text) is being enabled. In this case, the disabled audio renderer's reset call ends up resetting the AudioSink that's shared with the enabled audio renderer. The enabled audio renderer is then unable to make progress, causing playback to freeze. This is a minimal fix that directly prevents the mentioned issue. There are multiple follow-ups that would probably make sense: 1. Having ExoPlayerImplInternal track which renderers need to be reset, and only resetting those renderers rather than all that are disabled. This seems like a good thing to do regardless, rather than relying on those calls being no-ops. 2. If we want to continue sharing AudioSink, we need to formalize this much better and make sure we have good test coverage. Messages like MSG_SET_VOLUME are also delivered to the AudioSink multiple times via each of the renderers, which works currently because DefaultAudioSink no-ops all but the first call in each case. This is pretty fragile though! Issue: #8203 PiperOrigin-RevId: 343296081 --- RELEASENOTES.md | 5 +++-- .../android/exoplayer2/audio/MediaCodecAudioRenderer.java | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 412fc77003..31fd1972d1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -8,6 +8,9 @@ ([#8106](https://github.com/google/ExoPlayer/issues/8106)). * Suppress ProGuard warnings caused by Guava's compile-only dependencies ([#8103](https://github.com/google/ExoPlayer/issues/8103)). + * Fix issue that could cause playback to freeze when selecting tracks, if + extension audio renderers are being used + ([#8203](https://github.com/google/ExoPlayer/issues/8203)). * UI: * Fix incorrect color and text alignment of the `StyledPlayerControlView` fast forward and rewind buttons, when used together with the @@ -121,8 +124,6 @@ * Upgrade IMA SDK dependency to 3.20.1. This brings in a fix for companion ads rendering when targeting API 29 ([#6432](https://github.com/google/ExoPlayer/issues/6432)). -* Metadata retriever: - * Parse Google Photos HEIC motion photos metadata. ### 2.12.0 (2020-09-11) ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/audio/MediaCodecAudioRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/audio/MediaCodecAudioRenderer.java index 2d034335c8..07c3541552 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/audio/MediaCodecAudioRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/audio/MediaCodecAudioRenderer.java @@ -97,6 +97,7 @@ public class MediaCodecAudioRenderer extends MediaCodecRenderer implements Media private long currentPositionUs; private boolean allowFirstBufferPositionDiscontinuity; private boolean allowPositionDiscontinuity; + private boolean audioSinkNeedsReset; private boolean experimentalKeepAudioTrackOnSeek; @@ -507,6 +508,7 @@ public class MediaCodecAudioRenderer extends MediaCodecRenderer implements Media @Override protected void onDisabled() { + audioSinkNeedsReset = true; try { audioSink.flush(); } finally { @@ -523,7 +525,10 @@ public class MediaCodecAudioRenderer extends MediaCodecRenderer implements Media try { super.onReset(); } finally { - audioSink.reset(); + if (audioSinkNeedsReset) { + audioSinkNeedsReset = false; + audioSink.reset(); + } } }