New issue
Advanced search Search tips

Issue 821114 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove the static_cast to CdmContext

Project Member Reported by xhw...@chromium.org, Mar 12 2018

Issue description

Previously we only have one type of CdmContext on each platform, e.g. MediaDrmBridgeCdmContext on Android, and CastCdmContext on Cast. This is still mostly true, but there starts to emerge exceptions. For example, we could use AesDecrytpor in the remote process for testing (e.g. External Clear Key). For another example, we could have adapters/wrappers that wraps the current CdmContext to perform additional tasks. Currently we do have GetClassIdentifier() to prevent casting the wrong type, but this is essentially doing RTTI which is not recommended.

One solution is to change CdmContext to a mega container that can return different types of contexts for different platforms. Current it's already returning Decryptor and CdmProxyContext which is are good examples.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/996b4eb81c42c50f2d37261294204132cb789ddf

commit 996b4eb81c42c50f2d37261294204132cb789ddf
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Mar 13 00:41:53 2018

media: Add CdmContext::GetMediaDrmBridgeCdmContext

Add one extra level of redirection so that we can avoid the static
cast. This is following the existing model of GetDecryptor() and
GetCdmProxyContext(). See bug for details.

Will rename MediaDrmBridgeCdmContext in the next CL.

Bug: 821114
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8d58b294099d3b0fd9eb73db65a8ef566a5188ec
Reviewed-on: https://chromium-review.googlesource.com/958034
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542674}
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/media_drm_bridge.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/media_drm_bridge.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/media_drm_bridge_cdm_context.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/media_drm_bridge_cdm_context_impl.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/media_drm_bridge_cdm_context_impl.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/mock_media_drm_bridge_cdm_context.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/android/mock_media_drm_bridge_cdm_context.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/cdm_context.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/base/cdm_context.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/filters/android/media_codec_audio_decoder.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/filters/android/media_codec_audio_decoder.h
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/gpu/android/android_video_decode_accelerator.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/gpu/android/media_codec_video_decoder.cc
[modify] https://crrev.com/996b4eb81c42c50f2d37261294204132cb789ddf/media/gpu/android/media_codec_video_decoder_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7abdfa9f3c0235ec3c7728b1ecfccff74b72dc20

commit 7abdfa9f3c0235ec3c7728b1ecfccff74b72dc20
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Mar 13 18:14:38 2018

Revert "media: Do not allow both Decryptor and CDM ID on Android in MojoCdm"

This reverts commit 290bce09c68c3962db64725d4e03de89e44a62c4.

Now the proper fix is landed in
996b4eb81c42c50f2d37261294204132cb789ddf, we do not need this trick any
more.

Bug: 821114
Change-Id: Icc061572f36514c55d81e81000d085ce23833ea3
Reviewed-on: https://chromium-review.googlesource.com/959700
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542855}
[modify] https://crrev.com/7abdfa9f3c0235ec3c7728b1ecfccff74b72dc20/media/mojo/clients/mojo_cdm.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d420f7c12018e18f31b3f4b9133148366c3790c

commit 2d420f7c12018e18f31b3f4b9133148366c3790c
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Mar 13 21:32:23 2018

media: Rename MediaDrmBridgeCdmContext to MediaCryptoContext

The new name is shorter and better reflects what the class does:
providing the MediaCrypto object.

Bug: 821114
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I5226a8280a1342fb47a0847755b82a8be8ed52e4
Reviewed-on: https://chromium-review.googlesource.com/959698
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542916}
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/BUILD.gn
[rename] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/media_crypto_context.h
[add] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/media_crypto_context_impl.cc
[rename] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/media_crypto_context_impl.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/media_drm_bridge.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/media_drm_bridge.h
[delete] https://crrev.com/cbcb5ba6087e274152ada9f49d61fbef3563b225/media/base/android/media_drm_bridge_cdm_context_impl.cc
[rename] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/mock_media_crypto_context.cc
[add] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/android/mock_media_crypto_context.h
[delete] https://crrev.com/cbcb5ba6087e274152ada9f49d61fbef3563b225/media/base/android/mock_media_drm_bridge_cdm_context.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/cdm_context.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/base/cdm_context.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/filters/android/media_codec_audio_decoder.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/filters/android/media_codec_audio_decoder.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/android_video_decode_accelerator.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/android_video_decode_accelerator.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/avda_codec_allocator.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/media_codec_video_decoder.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/media_codec_video_decoder.h
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/gpu/android/media_codec_video_decoder_unittest.cc
[modify] https://crrev.com/2d420f7c12018e18f31b3f4b9133148366c3790c/media/mojo/README.md

Sign in to add a comment