1188812 - Retrieve CDM CanRender capability and store into MediaInfo for MDSM to create corresponding Sinks

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Kilik Kuo [:kikuo] (inactive)]

Per Bug 1146795 Comment 23, I would like to retrieve CDM CanRender capability from CDMProxy and store into MediaInfo for MDSM.
MDSM could create corresponding media sinks based on this info.

Comment 1

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Obtain_CDM_CanRender_Capability_and_store_in_MediaInfo_255922

Hi Chris,

According to Bug 1146795 Comment 24,
I added attribute |mCanExternalRender| in TrackInfo to indicate that if this track is gonna to be (decrypted)/decode/render in external component.
By this attribute, I could create corresponding Media Sink in MDSM (would be a follow-up if Bug 1188268 landed).

Also, I think there's would two follow-up to be created for this bug,

1. The attribute |mCanExternalRender| should also be updated somewhere in non-EME mode for a case(e.g. TV) that clear content is gonna decoded/rendered by other component.

2. If |mCanExternalRender| is true, there should be a way notifying MediaFormatReader not to create platform-decoder, because the decode/render jobs won't be handled by gecko, and EncodedData(non-EME)/EncryptedData(EME) should be queued to EncodedAudioSink/EncrytpedAudioSink (which will be created according to Bug 1188268) and be waiting for consumed.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8643457 [details] [diff] [review]
Obtain_CDM_CanRender_Capability_and_store_in_MediaInfo_255922

Review of attachment 8643457 [details] [diff] [review]:
-----------------------------------------------------------------

r=cpearce with comment & name of new field made consistent.

::: dom/media/MediaInfo.h
@@ +83,5 @@
>    int64_t mDuration;
>    int64_t mMediaTime;
>    CryptoTrack mCrypto;
>  
> +  // True if the track is gonna be (decrypted)/decoded and

The comment here implies the track *will* be rendered externally, but the name implies it *may* be externally rendered. Which is it? The name should match the comment.

If this being true means that the stream is definitely rendered externally, then you should call this mIsRenderedExternally.

Comment 3

[Kilik Kuo [:kikuo] (inactive)]

(In reply to Chris Pearce (:cpearce) from comment #2)
> Comment on attachment 8643457 [details] [diff] [review]
> Obtain_CDM_CanRender_Capability_and_store_in_MediaInfo_255922
> 
> Review of attachment 8643457 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=cpearce with comment & name of new field made consistent.
> 
> ::: dom/media/MediaInfo.h
> @@ +83,5 @@
> >    int64_t mDuration;
> >    int64_t mMediaTime;
> >    CryptoTrack mCrypto;
> >  
> > +  // True if the track is gonna be (decrypted)/decoded and
> 
> The comment here implies the track *will* be rendered externally, but the
> name implies it *may* be externally rendered. Which is it? The name should
> match the comment.
> 
> If this being true means that the stream is definitely rendered externally,
> then you should call this mIsRenderedExternally.

Thanks for the review, the intention of this variable is used for MDSM determining which type of MediaSink to create. The name you suggested "mIsRenderedExternally" is more appropriate. I'll use that. Thanks !

Comment 4

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Get CDM canRender capability and store in TrackInfo_r255922_v2

carry r+ from Comment 2, and addressed the issues. (Make variable name and comment consistent)

Comment 5

[Kilik Kuo [:kikuo] (inactive)]

try result - https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4c37a25c2e7

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bfb63119f1c

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7bfb63119f1c