media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start initialization if any stream is encrypted. Since CDM is available, we can simply pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more. Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on the result of audio and video Renderer/Decoder/DecryptingDemuxerStream initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part 2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.

Committed: https://crrev.com/a935e44754f619957614ff16a98c1e9c73783676
Cr-Commit-Position: refs/heads/master@{#374835}

[xhwang, 2016-02-03 07:35:24]

xhwang@chromium.org changed reviewers:
+ ddorwin@chromium.org, jrummell@chromium.org

[xhwang, 2016-02-03 07:35:24]

xhwang@chromium.org changed required reviewers:
+ jrummell@chromium.org

[xhwang, 2016-02-03 07:35:24]

This CL is still WIP. Now all encrypted-media layout tests pass, and some EME
test pages are playing well. But I still need to refine it and fix unittests.

I also need to fix the Android part (WMPA el al). But since this CL is getting
large, I am thinking of reverting the change in DDS, so that DDS + Android can
be done in a separate CL.

ddorwin / jrummell: Please do a high level review. Mainly changes in
cdm_context.h, renderer_impl.* and DDS/DAD/DVD/GVD. If it looks good I'll go
ahead polish this CL and fix tests.

[jrummell, 2016-02-03 23:32:28]

LG

https://codereview.chromium.org/1666653002/diff/1/media/filters/decrypting_au...
File media/filters/decrypting_audio_decoder.cc (right):

https://codereview.chromium.org/1666653002/diff/1/media/filters/decrypting_au...
media/filters/decrypting_audio_decoder.cc:85:
base::ResetAndReturn(&init_cb_).Run(false);
In SetCdm() state_ was set to kError. Was it intentional to not change the state
so that DAD can be initialized again?

https://codereview.chromium.org/1666653002/diff/1/media/filters/decrypting_au...
File media/filters/decrypting_audio_decoder.h (right):

https://codereview.chromium.org/1666653002/diff/1/media/filters/decrypting_au...
media/filters/decrypting_audio_decoder.h:15: #include "media/base/cdm_context.h"
nit: Since you've removed the #include and added "class CdmContext" everywhere
else, you should probably do it here too.

(Actually, it looks like it was done in files before this one, but not in
subsequent files.)

[xhwang, 2016-02-09 22:10:59]

Description was changed from

==========
media: Remove SetCdmReadyCB and CdmReadyCB.

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/DecryptingDemuxerStream initialization.

BUG=580250
TEST=Updated unittests.
==========

to

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

BUG=580250
TEST=Updated unittests.
==========

[xhwang, 2016-02-09 22:23:58]

PTAL again!

https://chromiumcodereview.appspot.com/1666653002/diff/1/media/filters/decryp...
File media/filters/decrypting_audio_decoder.cc (right):

https://chromiumcodereview.appspot.com/1666653002/diff/1/media/filters/decryp...
media/filters/decrypting_audio_decoder.cc:85:
base::ResetAndReturn(&init_cb_).Run(false);
On 2016/02/03 23:32:28, jrummell wrote:
> In SetCdm() state_ was set to kError. Was it intentional to not change the
state
> so that DAD can be initialized again?

Yeah, this is a bit messy. I don't think it matters though and this is
consistent with line 69 and line 75.

https://chromiumcodereview.appspot.com/1666653002/diff/1/media/filters/decryp...
File media/filters/decrypting_audio_decoder.h (right):

https://chromiumcodereview.appspot.com/1666653002/diff/1/media/filters/decryp...
media/filters/decrypting_audio_decoder.h:15: #include "media/base/cdm_context.h"
On 2016/02/03 23:32:28, jrummell wrote:
> nit: Since you've removed the #include and added "class CdmContext" everywhere
> else, you should probably do it here too.
> 
> (Actually, it looks like it was done in files before this one, but not in
> subsequent files.)

Done in the .cc file.

[xhwang, 2016-02-10 17:10:48]

xhwang@chromium.org changed reviewers:
+ dalecurtis@chromium.org
- ddorwin@chromium.org

[xhwang, 2016-02-10 17:10:48]

s/ddorwin/dalecurtis since ddorwin is OOO and this change is more in the media
stack than in the EME stack.

This CL looks big but it's actually pretty straightforward. Please start from
RendererImpl, then the Decoder interfaces, then decoders that supports encrypted
streams (DecryptingAudioDecoder, DecryptingVideoDecoder and GpuVideoDecoder).
Changes elsewhere should be trivial.

jrummell / dalecurtis: PTAL!

[DaleCurtis, 2016-02-10 18:01:54]

lgtm % nit

https://chromiumcodereview.appspot.com/1666653002/diff/100001/media/filters/v...
File media/filters/video_decoder_selector_unittest.cc (right):

https://chromiumcodereview.appspot.com/1666653002/diff/100001/media/filters/v...
media/filters/video_decoder_selector_unittest.cc:72: LOG(ERROR) << __FUNCTION__;
Remove?

[jrummell, 2016-02-10 22:28:02]

lgtm

[xhwang, 2016-02-10 22:35:06]

Description was changed from

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

BUG=580250
TEST=Updated unittests.
==========

to

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.
==========

[xhwang, 2016-02-10 22:35:56]

TBRing alokp@chromium.org and bbudge@chromium.org for trivial changes in
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc and
content/renderer/pepper/video_decoder_shim.cc.

https://codereview.chromium.org/1666653002/diff/100001/media/filters/video_de...
File media/filters/video_decoder_selector_unittest.cc (right):

https://codereview.chromium.org/1666653002/diff/100001/media/filters/video_de...
media/filters/video_decoder_selector_unittest.cc:72: LOG(ERROR) << __FUNCTION__;
On 2016/02/10 18:01:54, DaleCurtis wrote:
> Remove?

Done.

[xhwang, 2016-02-10 22:36:06]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2016-02-10 22:36:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
jrummell@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1666653002/#ps120001
(title: "remove log")

[commit-bot: I haz the power, 2016-02-10 22:44:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1666653002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1666653002/120001

[commit-bot: I haz the power, 2016-02-10 23:10:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-10 23:10:43]

Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[xhwang, 2016-02-11 01:02:29]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2016-02-11 01:02:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jrummell@chromium.org, dalecurtis@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1666653002/#ps140001 (title: "rebase and
fix compile errors")

[commit-bot: I haz the power, 2016-02-11 01:03:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1666653002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1666653002/140001

[commit-bot: I haz the power, 2016-02-11 02:23:01]

Description was changed from

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.
==========

to

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.
==========

[commit-bot: I haz the power, 2016-02-11 02:23:02]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-02-16 22:34:01]

Description was changed from

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.
==========

to

==========
media: Remove SetCdmReadyCB and CdmReadyCB (part 1).

RendererImpl and MediaSourceDelegate wait for CDM to be available to start
initialization if any stream is encrypted. Since CDM is available, we can simply
pass the CdmContext down and thus avoid the SetCdmReadyCB/CdmReadyCB/SetCdm
round trip.

Also decoder initialization will not be associated with CdmAttachedCB any more.
Instead, we'll only handle it in RendererImpl and MediaSourceDelegate based on
the result of audio and video Renderer/Decoder/DecryptingDemuxerStream
initialization.

This CL is part 1 where all decoders/renderers are fixed. In the next CL (part
2), DecryptingDemuxerStream and MediaSourceDelegate will be fixed.

TBR=alokp@chromium.org,bbudge@chromium.org
BUG=580250
TEST=Updated unittests.

Committed: https://crrev.com/a935e44754f619957614ff16a98c1e9c73783676
Cr-Commit-Position: refs/heads/master@{#374835}
==========

[commit-bot: I haz the power, 2016-02-16 22:34:02]

Patchset 8 (id:??) landed as
https://crrev.com/a935e44754f619957614ff16a98c1e9c73783676
Cr-Commit-Position: refs/heads/master@{#374835}