Config change on encrypted audio fails with MojoAudioDecoder and MediaCodecAudioDecoder |
|||||
Issue descriptionCurrent state: 1. When doing decoder reinitialization, we are not passing in the CdmContext [1], with the assumption that decoders already got the CdmContext during the initial initialization. 2. MojoAudioDecoder doesn't support reinitialization without a valid CdmContext [2]. 3. In MediaCodecAudioDecoder::Initialize() we always call SetCdm() for Initialize() [3]. Proposed fix: - For (1), we should probably always pass in a valid CdmContext even for reinitialization and let decoders to decide how to use the CdmContext during reinit. - Then (2) will be solved. - For (3), we can choose only set the CDM when the stream is encrypted and the CDM has not been set before. [1] https://cs.chromium.org/chromium/src/media/filters/decoder_stream.cc?rcl=1483734086&l=684 [2] https://cs.chromium.org/chromium/src/media/mojo/clients/mojo_audio_decoder.cc?rcl=1483734086&l=56 [3] https://cs.chromium.org/chromium/src/media/filters/android/media_codec_audio_decoder.cc?rcl=0&l=85
,
Jan 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674 commit f953aeb70bcde5e3aeda9d9aefeff4ad209e3674 Author: xhwang <xhwang@chromium.org> Date: Sat Jan 14 06:53:10 2017 media: Always provide CdmContext when selecting decoder in DecoderStream BUG= 679095 Review-Url: https://codereview.chromium.org/2625333003 Cr-Commit-Position: refs/heads/master@{#443794} [modify] https://crrev.com/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674/content/renderer/media/cdm/ppapi_decryptor.cc [modify] https://crrev.com/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674/content/renderer/media/cdm/ppapi_decryptor.h [modify] https://crrev.com/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674/media/filters/decoder_selector.cc [modify] https://crrev.com/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674/media/filters/decoder_stream.cc [modify] https://crrev.com/f953aeb70bcde5e3aeda9d9aefeff4ad209e3674/media/filters/decoder_stream.h
,
Jan 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a02ac7ae09857022954bd2627f74416e683fce3 commit 2a02ac7ae09857022954bd2627f74416e683fce3 Author: xhwang <xhwang@chromium.org> Date: Thu Jan 19 23:25:29 2017 media: Fix MojoAudioDecoder reinitialization BUG= 679095 TEST=New test added. Review-Url: https://codereview.chromium.org/2624213006 Cr-Commit-Position: refs/heads/master@{#444888} [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/clients/mojo_audio_decoder.cc [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/clients/mojo_audio_decoder.h [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/clients/mojo_audio_decoder_unittest.cc [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/interfaces/audio_decoder.mojom [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/services/mojo_audio_decoder_service.cc [modify] https://crrev.com/2a02ac7ae09857022954bd2627f74416e683fce3/media/mojo/services/mojo_audio_decoder_service.h
,
Jan 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c1f8ed028edcb44c954cb2a0625a8f278933481 commit 2c1f8ed028edcb44c954cb2a0625a8f278933481 Author: xhwang <xhwang@chromium.org> Date: Sat Jan 21 07:57:55 2017 media: Fix MediaCodecAudioDecoder reinitialization According to media::AudioDecoder documentation, a decoder can be reinitialized with a different config after it's previously initialized. In this case, the decoder should drop all internal buffers and reset to a clean state. Currently this is not supported by MediaCodecAudioDecoder and running the newly added test would cause DCHECKs to fire. This CL fixes MediaCodecAudioDecoder to support reinitialization, where it will always call SetInitialConfiguration() and CreateMediaCodecLoop() to initialize the decoder with the new config. New unit tests are added to cover this case. As part of the process I also made some cleanup changes in the unit test file. BUG= 679095 TEST=New test added. Review-Url: https://codereview.chromium.org/2642823004 Cr-Commit-Position: refs/heads/master@{#445280} [modify] https://crrev.com/2c1f8ed028edcb44c954cb2a0625a8f278933481/media/filters/android/media_codec_audio_decoder.cc [modify] https://crrev.com/2c1f8ed028edcb44c954cb2a0625a8f278933481/media/filters/audio_decoder_unittest.cc
,
Jan 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b9df60e4c5993c6802cf994bc40c75935e5cba7 commit 1b9df60e4c5993c6802cf994bc40c75935e5cba7 Author: xhwang <xhwang@chromium.org> Date: Wed Jan 25 05:21:11 2017 media: Add logs in MojoAudioDecoder::Initialize() TBR=sanfin@chromium.org BUG= 679095 Review-Url: https://codereview.chromium.org/2654943002 Cr-Commit-Position: refs/heads/master@{#445957} [modify] https://crrev.com/1b9df60e4c5993c6802cf994bc40c75935e5cba7/media/mojo/clients/mojo_audio_decoder.cc
,
Jan 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95f1b49f981dd91d79c08d52481103f669db55f0 commit 95f1b49f981dd91d79c08d52481103f669db55f0 Author: xhwang <xhwang@chromium.org> Date: Thu Jan 26 01:19:00 2017 media: Pass CdmContext in decoder reinitialization Fix DecoderStream::ReinitializeDecoder to always pass in the CdmContext. Also added unittests to cover the case where neither DecryptionVideoDecoder nor DecryptingDemuxerStream is selected for encrypted config, but another VideoDecoder that supports encrypted config is selected, initialized, and reinitialized. In practice, this could be the case when the CDM doesn't support the Decryptor interface, e.g. MediaDrmBridge on Android. BUG= 679095 Review-Url: https://codereview.chromium.org/2650823006 Cr-Commit-Position: refs/heads/master@{#446190} [modify] https://crrev.com/95f1b49f981dd91d79c08d52481103f669db55f0/media/filters/decoder_stream.cc [modify] https://crrev.com/95f1b49f981dd91d79c08d52481103f669db55f0/media/filters/fake_video_decoder.cc [modify] https://crrev.com/95f1b49f981dd91d79c08d52481103f669db55f0/media/filters/fake_video_decoder.h [modify] https://crrev.com/95f1b49f981dd91d79c08d52481103f669db55f0/media/filters/video_frame_stream_unittest.cc
,
Jan 31 2017
,
Jan 31 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2d0fbfd1f7c4fc522c344b839dd97a28ea596177 commit 2d0fbfd1f7c4fc522c344b839dd97a28ea596177 Author: xhwang <xhwang@chromium.org> Date: Thu Feb 02 20:00:15 2017 media: Fix MediaCodecAudioDecoder reinitialization According to media::AudioDecoder documentation, a decoder can be reinitialized with a different config after it's previously initialized. In this case, the decoder should drop all internal buffers and reset to a clean state. Currently this is not supported by MediaCodecAudioDecoder and running the newly added test would cause DCHECKs to fire. This CL fixes MediaCodecAudioDecoder to support reinitialization, where it will always call SetInitialConfiguration() and CreateMediaCodecLoop() to initialize the decoder with the new config. New unit tests are added to cover this case. As part of the process I also made some cleanup changes in the unit test file. NOTRY=true NOPRESUBMIT=true TBR=dalecurtis@chromium.org,timav@chromium.org BUG= 679095 TEST=New test added. Review-Url: https://codereview.chromium.org/2642823004 Cr-Commit-Position: refs/heads/master@{#445280} (cherry picked from commit 2c1f8ed028edcb44c954cb2a0625a8f278933481) Review-Url: https://codereview.chromium.org/2669253003 Cr-Commit-Position: refs/branch-heads/2987@{#278} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/2d0fbfd1f7c4fc522c344b839dd97a28ea596177/media/filters/android/media_codec_audio_decoder.cc [modify] https://crrev.com/2d0fbfd1f7c4fc522c344b839dd97a28ea596177/media/filters/audio_decoder_unittest.cc
,
Feb 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b509b3c001a440d1972e49929d6a511242a18a26 commit b509b3c001a440d1972e49929d6a511242a18a26 Author: Xiaohan Wang <xhwang@chromium.org> Date: Thu Feb 02 21:02:29 2017 media: Pass CdmContext in decoder reinitialization Fix DecoderStream::ReinitializeDecoder to always pass in the CdmContext. Also added unittests to cover the case where neither DecryptionVideoDecoder nor DecryptingDemuxerStream is selected for encrypted config, but another VideoDecoder that supports encrypted config is selected, initialized, and reinitialized. In practice, this could be the case when the CDM doesn't support the Decryptor interface, e.g. MediaDrmBridge on Android. NOTRY=true NOPRESUBMIT=true TBR=dalecurtis@chromium.org BUG= 679095 Review-Url: https://codereview.chromium.org/2650823006 Cr-Commit-Position: refs/heads/master@{#446190} (cherry picked from commit 95f1b49f981dd91d79c08d52481103f669db55f0) Review-Url: https://codereview.chromium.org/2674783002 . Cr-Commit-Position: refs/branch-heads/2987@{#281} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/b509b3c001a440d1972e49929d6a511242a18a26/media/filters/decoder_stream.cc [modify] https://crrev.com/b509b3c001a440d1972e49929d6a511242a18a26/media/filters/fake_video_decoder.cc [modify] https://crrev.com/b509b3c001a440d1972e49929d6a511242a18a26/media/filters/fake_video_decoder.h [modify] https://crrev.com/b509b3c001a440d1972e49929d6a511242a18a26/media/filters/video_frame_stream_unittest.cc
,
Feb 2 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by xhw...@chromium.org
, Jan 7 2017