Closed
Bug 1188812
Opened 9 years ago
Closed 9 years ago
Retrieve CDM CanRender capability and store into MediaInfo for MDSM to create corresponding Sinks
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: kikuo, Assigned: kikuo)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
kikuo
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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•9 years ago
|
||
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.
Attachment #8643457 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(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 !
Assignee | ||
Comment 4•9 years ago
|
||
carry r+ from Comment 2, and addressed the issues. (Make variable name and comment consistent)
Attachment #8644160 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8643457 -
Attachment is obsolete: true
Assignee | ||
Comment 5•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 7•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•