From 5fcc7433a1f188c9644d4430a828a49d06c993cb Mon Sep 17 00:00:00 2001 From: ibaker Date: Tue, 25 Jun 2024 06:52:19 -0700 Subject: [PATCH] Use `MediaCodec.stop()` before `release()` for surface switching bug ExoPlayer used to call `stop()` before `release()`. This was removed in . A framework bug introduced in Android 11 (API 30) resulted in some DRM -> clear transitions failing during `MediaCodec.configure()`. An investigation in Issue: google/ExoPlayer#8696 and b/191966399 identified that this was due to `release()` returning 'too early' and the subsequent `configure()` call was then trying to re-use a `Surface` that hadn't been fully detached from the previous codec. This was fixed in Android 13 (API 33) with http://r.android.com/2094347. ExoPlayer worked around the framework bug by adding an arbitrary 50ms sleep after a failed codec initialization, followed by retrying. This was enough to resolve the problem in the test scenario on a OnePlus AC2003. Issue: androidx/media#1497 points out that 50ms might not be the appropriate delay for all devices, so it's an incomplete fix. They suggested re-adding the `MediaCodec.stop()` call instead. This also reliably resolves the issue on the OnePlus AC2003 (with neither workaround in place, the problem repros almost immediately). PiperOrigin-RevId: 646461943 --- .../AsynchronousMediaCodecAdapter.java | 14 ++++++++++++-- .../exoplayer/mediacodec/MediaCodecRenderer.java | 16 +--------------- .../mediacodec/SynchronousMediaCodecAdapter.java | 12 +++++++++++- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/AsynchronousMediaCodecAdapter.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/AsynchronousMediaCodecAdapter.java index f98b40deb5..e2445b518a 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/AsynchronousMediaCodecAdapter.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/AsynchronousMediaCodecAdapter.java @@ -259,8 +259,18 @@ import java.nio.ByteBuffer; state = STATE_SHUT_DOWN; } finally { if (!codecReleased) { - codec.release(); - codecReleased = true; + try { + // Stopping the codec before releasing it works around a bug on APIs 30, 31 and 32 where + // MediaCodec.release() returns too early before fully detaching a Surface, and a + // subsequent MediaCodec.configure() call using the same Surface then fails. See + // https://github.com/google/ExoPlayer/issues/8696 and b/191966399. + if (Util.SDK_INT >= 30 && Util.SDK_INT < 33) { + codec.stop(); + } + } finally { + codec.release(); + codecReleased = true; + } } } } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java index 979ed7a21f..f6faa72871 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/MediaCodecRenderer.java @@ -1128,27 +1128,13 @@ public abstract class MediaCodecRenderer extends BaseRenderer { } ArrayDeque availableCodecInfos = checkNotNull(this.availableCodecInfos); - MediaCodecInfo preferredCodecInfo = availableCodecInfos.peekFirst(); while (codec == null) { MediaCodecInfo codecInfo = checkNotNull(availableCodecInfos.peekFirst()); if (!shouldInitCodec(codecInfo)) { return; } try { - try { - initCodec(codecInfo, crypto); - } catch (Exception e) { - if (codecInfo == preferredCodecInfo) { - // If creating the preferred decoder failed then sleep briefly before retrying. - // Workaround for [internal b/191966399]. - // See also https://github.com/google/ExoPlayer/issues/8696. - Log.w(TAG, "Preferred decoder instantiation failed. Sleeping for 50ms then retrying."); - Thread.sleep(/* millis= */ 50); - initCodec(codecInfo, crypto); - } else { - throw e; - } - } + initCodec(codecInfo, crypto); } catch (Exception e) { Log.w(TAG, "Failed to initialize decoder: " + codecInfo, e); // This codec failed to initialize, so fall back to the next codec in the list (if any). We diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/SynchronousMediaCodecAdapter.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/SynchronousMediaCodecAdapter.java index d45a6d5035..405013fbf9 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/SynchronousMediaCodecAdapter.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/mediacodec/SynchronousMediaCodecAdapter.java @@ -172,7 +172,17 @@ public final class SynchronousMediaCodecAdapter implements MediaCodecAdapter { public void release() { inputByteBuffers = null; outputByteBuffers = null; - codec.release(); + try { + if (Util.SDK_INT >= 30 && Util.SDK_INT < 33) { + // Stopping the codec before releasing it works around a bug on APIs 30, 31 and 32 where + // MediaCodec.release() returns too early before fully detaching a Surface, and a + // subsequent MediaCodec.configure() call using the same Surface then fails. See + // https://github.com/google/ExoPlayer/issues/8696 and b/191966399. + codec.stop(); + } + } finally { + codec.release(); + } } @Override