MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor, we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream initialization failed, it's still possible that we can handle the audio with a CDM in the browser. Declare demuxer ready now to try that path. Note there's no need to try DecryptingDemuxerStream for video here since it is impossible to handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.

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

[xhwang, 2016-01-21 21:03:30]

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

[xhwang, 2016-01-21 21:03:30]

PTAL

As discussed offline, the logic around this code is complicated and this fix
just make sure we don't fire CdmAttachedCB twice with false. I still want to
land this CL which is easily mergeable. The larger refactoring work is tracked
in http://crbug.com/580250

[ddorwin, 2016-01-22 00:58:36]

Please explain why "there's no reason to
keeping trying to initialize video DecryptigDemuxerStream" and why we "should
just declare demuxer ready."

Thanks.

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
File content/renderer/media/android/media_source_delegate.cc (right):

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
content/renderer/media/android/media_source_delegate.cc:157: 
Should we check all cb parameters?

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
content/renderer/media/android/media_source_delegate.cc:551: is_demuxer_ready_ =
true;
This is probably worth a comment (explaining why we say "ready" when we failed.

[xhwang, 2016-01-29 00:37:41]

Description was changed from

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

If audio DecryptigDemuxerStream initialization failed, there's no reason to
keeping trying to initialize video DecryptigDemuxerStream. In that case, we
should just declare demuxer ready and hope the browser side players can handle
it.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

to

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

If audio DecryptigDemuxerStream initialization failed (e.g. the CDM doesn't
support decrypt-only), it's still possible that we can decrypt and decode audio
in the browser. So we declare demuxer ready so we can try that path.

Note that in this case there's no need to try DecryptingDemuxerStream for video
since it is impossible to decrypt and decode audio in the browser and do
decrypt-only in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

[xhwang, 2016-01-29 00:46:35]

comments addressed

[xhwang, 2016-01-29 00:47:15]

Sorry for the delay. PTAL again!

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
File content/renderer/media/android/media_source_delegate.cc (right):

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
content/renderer/media/android/media_source_delegate.cc:157: 
On 2016/01/22 00:58:35, ddorwin wrote:
> Should we check all cb parameters?

Done.

https://chromiumcodereview.appspot.com/1613793002/diff/1/content/renderer/med...
content/renderer/media/android/media_source_delegate.cc:551: is_demuxer_ready_ =
true;
On 2016/01/22 00:58:35, ddorwin wrote:
> This is probably worth a comment (explaining why we say "ready" when we
failed.

Done.

[ddorwin, 2016-01-29 18:16:28]

LGTM with comments. Thanks.

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
File content/renderer/media/android/media_source_delegate.cc (right):

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
content/renderer/media/android/media_source_delegate.cc:160:
DCHECK(!set_cdm_ready_cb.is_null());
nit: This one is out of order.

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
content/renderer/media/android/media_source_delegate.cc:560: // it is impossible
to decrypt and decode audio in the browser and do
Thanks - this is helpful.
I believe the fundamental reason (regardless of an implementation limitations)
because this would involve using different CDMs. You might be explicit about
that here and in the description.

[xhwang, 2016-02-01 22:48:32]

rebase only

[xhwang, 2016-02-01 23:34:34]

Description was changed from

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

If audio DecryptigDemuxerStream initialization failed (e.g. the CDM doesn't
support decrypt-only), it's still possible that we can decrypt and decode audio
in the browser. So we declare demuxer ready so we can try that path.

Note that in this case there's no need to try DecryptingDemuxerStream for video
since it is impossible to decrypt and decode audio in the browser and do
decrypt-only in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

to

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor,
we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll
try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream
initialization failed, it's still possible that we can handle the audio with a
CDM in the browser. Declare demuxer ready now to try that path. Note there's no
need to try DecryptingDemuxerStream for video here since it is impossible to
handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

[xhwang, 2016-02-01 23:34:55]

comments addressed

[xhwang, 2016-02-01 23:35:25]

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
File content/renderer/media/android/media_source_delegate.cc (right):

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
content/renderer/media/android/media_source_delegate.cc:160:
DCHECK(!set_cdm_ready_cb.is_null());
On 2016/01/29 18:16:28, ddorwin wrote:
> nit: This one is out of order.

Done.

https://chromiumcodereview.appspot.com/1613793002/diff/20001/content/renderer...
content/renderer/media/android/media_source_delegate.cc:560: // it is impossible
to decrypt and decode audio in the browser and do
On 2016/01/29 18:16:28, ddorwin wrote:
> Thanks - this is helpful.
> I believe the fundamental reason (regardless of an implementation limitations)
> because this would involve using different CDMs. You might be explicit about
> that here and in the description.

Done.

[xhwang, 2016-02-01 23:35:40]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2016-02-01 23:35:41]

The patchset sent to the CQ was uploaded after l-g-t-m from ddorwin@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1613793002/#ps60001
(title: "comments addressed")

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

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

[commit-bot: I haz the power, 2016-02-02 00:54:44]

Description was changed from

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor,
we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll
try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream
initialization failed, it's still possible that we can handle the audio with a
CDM in the browser. Declare demuxer ready now to try that path. Note there's no
need to try DecryptingDemuxerStream for video here since it is impossible to
handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

to

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor,
we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll
try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream
initialization failed, it's still possible that we can handle the audio with a
CDM in the browser. Declare demuxer ready now to try that path. Note there's no
need to try DecryptingDemuxerStream for video here since it is impossible to
handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

[commit-bot: I haz the power, 2016-02-02 00:54:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-02 00:56:40]

Description was changed from

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor,
we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll
try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream
initialization failed, it's still possible that we can handle the audio with a
CDM in the browser. Declare demuxer ready now to try that path. Note there's no
need to try DecryptingDemuxerStream for video here since it is impossible to
handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.
==========

to

==========
MediaSourceDelegate: Fix DecryptingDemuxerStream initialization logic.

Different CDMs are supported differently. For CDMs that support a Decryptor,
we'll try to use DecryptingDemuxerStream in the render side. Otherwise, we'll
try to use the CDMs in the browser side. Therefore, if DecryptingDemuxerStream
initialization failed, it's still possible that we can handle the audio with a
CDM in the browser. Declare demuxer ready now to try that path. Note there's no
need to try DecryptingDemuxerStream for video here since it is impossible to
handle audio in the browser and handle video in the render process.

This also fixes the issue where CdmAttachedCB is fired twice with "false".

BUG=578203
TEST=No DCHECK when playing media/L3_audio_L3_video.mp4.mpd.

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

[commit-bot: I haz the power, 2016-02-02 00:56:41]

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