From 913f6da08305b36798c84d5134d19b2d11affdd2 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 4 Dec 2023 07:29:53 -0800 Subject: [PATCH] MCR: Ensure `mediaCrypto` and `codec` are atomically non-null `mediaCrypto` is initialized before `codec` in `maybeInitCodecOrBypass`. Before this change, it was possible for `maybeInitCodecOrBypass` to complete with `mediaCrypto != null` and `codec == null`, in particular if it was run as part of clearing the player surface (since in that case, no video codec is initialized). This inconsistent state then causes issues during a later invocation of `maybeInitCodecOrBypass`, when `mediaCrypto` is still non-null, and `mediaCryptoRequiresSecureDecoder = true`, but the content has been changed to unencrypted with no associated DRM session. This results in a playback error, because a secure decoder is initialized but there's no DRM session available to work with it. This change ensures that when `maybeInitCodecOrBypass` completes, either both `mediaCrypto != null` and `codec != null` (i.e. codec initialization was completed) or `mediaCrypto == null` and `codec == null` (i.e. codec initialization was not completed). We also ensure that when nulling out `mediaCrypto` we also set `maybeInitCodecOrBypass = false`. A later change should be able to demote `maybeInitCodecOrBypass` from a field to a local in order to remove any risk of that part of state becoming out of sync. This resolves the issue, because during the second invocation of `maybeInitCodecOrBypass` an insecure decoder is now (correctly) initialized and the unencrypted content is successfully played. #minor-release PiperOrigin-RevId: 587713911 --- RELEASENOTES.md | 3 + .../mediacodec/MediaCodecRenderer.java | 110 ++++++++++-------- 2 files changed, 66 insertions(+), 47 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 17e588cf63..879edb2af4 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -59,6 +59,9 @@ * Extend workaround for spurious ClearKey `https://default.url` license URL to API 33+ (previously the workaround only applied on API 33 exactly) ([#837](https://github.com/androidx/media/pull/837)). + * Fix `ERROR_DRM_SESSION_NOT_OPENED` when switching from encrypted to + clear content without a surface attached to the player. The error was + due to incorrectly using a secure decoder to play the clear content. * Effect: * Muxers: * IMA extension: 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 3df7e2da09..993418d278 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 @@ -89,6 +89,7 @@ import java.nio.ByteOrder; import java.util.ArrayDeque; import java.util.List; import java.util.Objects; +import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** An abstract renderer that uses {@link MediaCodec} to decode samples for rendering. */ // @@ -545,55 +546,19 @@ public abstract class MediaCodecRenderer extends BaseRenderer { } setCodecDrmSession(sourceDrmSession); - - String mimeType = checkNotNull(inputFormat).sampleMimeType; - @Nullable DrmSession codecDrmSession = this.codecDrmSession; - if (codecDrmSession != null) { - @Nullable CryptoConfig cryptoConfig = codecDrmSession.getCryptoConfig(); - if (mediaCrypto == null) { - if (cryptoConfig == null) { - @Nullable DrmSessionException drmError = codecDrmSession.getError(); - if (drmError != null) { - // Continue for now. We may be able to avoid failure if a new input format causes the - // session to be replaced without it having been used. - } else { - // The drm session isn't open yet. - return; - } - } else if (cryptoConfig instanceof FrameworkCryptoConfig) { - FrameworkCryptoConfig frameworkCryptoConfig = (FrameworkCryptoConfig) cryptoConfig; - try { - mediaCrypto = - new MediaCrypto(frameworkCryptoConfig.uuid, frameworkCryptoConfig.sessionId); - } catch (MediaCryptoException e) { - throw createRendererException( - e, inputFormat, PlaybackException.ERROR_CODE_DRM_SYSTEM_ERROR); - } - mediaCryptoRequiresSecureDecoder = - !frameworkCryptoConfig.forceAllowInsecureDecoderComponents - && mediaCrypto.requiresSecureDecoderComponent(checkStateNotNull(mimeType)); - } - } - if (FrameworkCryptoConfig.WORKAROUND_DEVICE_NEEDS_KEYS_TO_CONFIGURE_CODEC - && cryptoConfig instanceof FrameworkCryptoConfig) { - @DrmSession.State int drmSessionState = codecDrmSession.getState(); - if (drmSessionState == DrmSession.STATE_ERROR) { - DrmSessionException drmSessionException = - Assertions.checkNotNull(codecDrmSession.getError()); - throw createRendererException( - drmSessionException, inputFormat, drmSessionException.errorCode); - } else if (drmSessionState != DrmSession.STATE_OPENED_WITH_KEYS) { - // Wait for keys. - return; - } + if (codecDrmSession == null || initMediaCryptoIfDrmSessionReady()) { + try { + maybeInitCodecWithFallback(mediaCrypto, mediaCryptoRequiresSecureDecoder); + } catch (DecoderInitializationException e) { + throw createRendererException( + e, inputFormat, PlaybackException.ERROR_CODE_DECODER_INIT_FAILED); } } - - try { - maybeInitCodecWithFallback(mediaCrypto, mediaCryptoRequiresSecureDecoder); - } catch (DecoderInitializationException e) { - throw createRendererException( - e, inputFormat, PlaybackException.ERROR_CODE_DECODER_INIT_FAILED); + if (mediaCrypto != null && codec == null) { + // mediaCrypto was created, but a codec wasn't, so release the mediaCrypto before returning. + mediaCrypto.release(); + mediaCrypto = null; + mediaCryptoRequiresSecureDecoder = false; } } @@ -1035,6 +1000,57 @@ public abstract class MediaCodecRenderer extends BaseRenderer { return false; } + /** + * Checks whether {@link #codecDrmSession} is ready for playback, and if so initializes {@link + * #mediaCrypto} if needed. + * + * @return {@code true} if codec initialization should continue, or {@code false} if it should be + * aborted. + */ + @RequiresNonNull("this.codecDrmSession") + private boolean initMediaCryptoIfDrmSessionReady() throws ExoPlaybackException { + checkState(mediaCrypto == null); + DrmSession codecDrmSession = this.codecDrmSession; + String mimeType = checkNotNull(inputFormat).sampleMimeType; + @Nullable CryptoConfig cryptoConfig = codecDrmSession.getCryptoConfig(); + if (FrameworkCryptoConfig.WORKAROUND_DEVICE_NEEDS_KEYS_TO_CONFIGURE_CODEC + && cryptoConfig instanceof FrameworkCryptoConfig) { + @DrmSession.State int drmSessionState = codecDrmSession.getState(); + if (drmSessionState == DrmSession.STATE_ERROR) { + DrmSessionException drmSessionException = + Assertions.checkNotNull(codecDrmSession.getError()); + throw createRendererException( + drmSessionException, inputFormat, drmSessionException.errorCode); + } else if (drmSessionState != DrmSession.STATE_OPENED_WITH_KEYS) { + // Wait for keys. + return false; + } + } + if (cryptoConfig == null) { + @Nullable DrmSessionException drmError = codecDrmSession.getError(); + if (drmError != null) { + // Continue for now. We may be able to avoid failure if a new input format causes the + // session to be replaced without it having been used. + return true; + } else { + // The drm session isn't open yet. + return false; + } + } else if (cryptoConfig instanceof FrameworkCryptoConfig) { + FrameworkCryptoConfig frameworkCryptoConfig = (FrameworkCryptoConfig) cryptoConfig; + try { + mediaCrypto = new MediaCrypto(frameworkCryptoConfig.uuid, frameworkCryptoConfig.sessionId); + } catch (MediaCryptoException e) { + throw createRendererException( + e, inputFormat, PlaybackException.ERROR_CODE_DRM_SYSTEM_ERROR); + } + mediaCryptoRequiresSecureDecoder = + !frameworkCryptoConfig.forceAllowInsecureDecoderComponents + && mediaCrypto.requiresSecureDecoderComponent(checkStateNotNull(mimeType)); + } + return true; + } + private void maybeInitCodecWithFallback( @Nullable MediaCrypto crypto, boolean mediaCryptoRequiresSecureDecoder) throws DecoderInitializationException {