Refactor MediaCodecBridge.queueSecureInputBuffer() #3854
Conversation
Modifies the body of MediaCodecBridge.queueSecureInputBuffer() to more closely resemble the Chromium version. b/352387646
osagie98
requested review from
jasonzhangxx,
xiaomings,
zhongqiliang and
borongc
starboard/android/apk/app/src/main/java/dev/cobalt/media/MediaCodecBridge.java
Show resolved
Hide resolved
| if (patternEncrypt != 0 && patternSkip != 0) { | ||
| if (usesCbcs) { | ||
| // Above platform check ensured that setting the pattern is indeed supported. | ||
| cryptoInfo.setPattern(new Pattern(patternEncrypt, patternSkip)); | ||
| } else { | ||
| Log.e(TAG, "Pattern encryption only supported for 'cbcs' scheme (CBC mode)."); | ||
| return MediaCodecStatus.ERROR; | ||
| } |
There was a problem hiding this comment.
Just wanted to double check that we have verified the new implementation against cbcs and cenc playbacks.
There was a problem hiding this comment.
Yes, cenc playbacks live on YouTube, cbcs and cenc streams from go/cobalt-media-features#encrypted-content
| Log.e(TAG, "Pattern encryption only supported for 'cbcs' scheme (CBC mode)."); | ||
| return MediaCodecStatus.ERROR; | ||
| if (patternEncrypt != 0 || patternSkip != 0) { | ||
| if (cipherMode == MediaCodec.CRYPTO_MODE_AES_CBC) { |
There was a problem hiding this comment.
As discussed offline, we are trying to make the implementation as close to the Chromium one as possible, but not apply any functional changes to the existing implementation.
In this particular case, we should add boolean usesCbcs = cipherMode == MediaCodec.CRYPTO_MODE_AES_CBC; in front of CryptoInfo cryptoInfo = new CryptoInfo(); and use usesCbcs here.
| TAG, | ||
| "Failed to queue secure input buffer, CryptoException with error code " | ||
| + e.getErrorCode()); | ||
| Log.e(TAG, "Failed to queue secure input buffer. Error code %d", errorCode, e); | ||
| return MediaCodecStatus.ERROR; | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
MediaCodec.CodecException is caught on the upstream, consider catching it here too if it can be thrown by the function.
| TAG, | ||
| "Failed to queue secure input buffer, CryptoException with error code " | ||
| + e.getErrorCode()); | ||
| Log.e(TAG, "Failed to queue secure input buffer. Error code %d", errorCode, e); |
There was a problem hiding this comment.
The upstream seems to use a different formatting (which can probably be refined), and the same to line 966. Try to check if the upstream can be updated with an improved formatting (i.e. if its default code formater will generate a better version).
Modifies the body of MediaCodecBridge.queueSecureInputBuffer() to more closely resemble the Chromium version.
b/352387646