Closed Bug 1371200 Opened 3 years ago Closed 3 years ago

Add a structure so it is easier to pass more data to the MediaDecoder constructor

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(5 files)

1. we want to avoid 2-stage initialization whenever possible.
2. it is easier to add new data to the constructor without modifying each constructor of its sub-classes.
Assignee: nobody → jwwang
Priority: -- → P3
Attachment #8875619 - Flags: review?(cpearce)
Attachment #8875620 - Flags: review?(cpearce)
Attachment #8875621 - Flags: review?(cpearce)
Attachment #8875622 - Flags: review?(cpearce)
Attachment #8875623 - Flags: review?(cpearce)
Blocks: 1371202
Blocks: 1369263
Comment on attachment 8875620 [details]
Bug 1371200. P2 - add more fields to MediaDecoderInit.

https://reviewboard.mozilla.org/r/147038/#review152182

::: dom/html/HTMLMediaElement.cpp:2469
(Diff revision 1)
>      SetupSrcMediaStreamPlayback(stream);
>      return NS_OK;
>    }
>  
>    if (mMediaSource) {
> -    MediaDecoderInit decoderInit(this);
> +    MediaDecoderInit decoderInit(

You've written identical code to call MediaDecoderInit's constructor three times; it could be in a helper function.
Attachment #8875620 - Flags: review?(cpearce) → review+
Comment on attachment 8875619 [details]
Bug 1371200. P1 - add MediaDecoderInit and fix MediaDecoder constructor and its friends.

https://reviewboard.mozilla.org/r/147036/#review152184
Attachment #8875619 - Flags: review?(cpearce) → review+
Comment on attachment 8875621 [details]
Bug 1371200. P3 - remove unused code.

https://reviewboard.mozilla.org/r/147040/#review152186
Attachment #8875621 - Flags: review?(cpearce) → review+
Comment on attachment 8875622 [details]
Bug 1371200. P4 - constify some members.

https://reviewboard.mozilla.org/r/147042/#review152188
Attachment #8875622 - Flags: review?(cpearce) → review+
Comment on attachment 8875623 [details]
Bug 1371200. P5 - devirtualize some functions that don't have overrides.

https://reviewboard.mozilla.org/r/147044/#review152190
Attachment #8875623 - Flags: review?(cpearce) → review+
Comment on attachment 8875620 [details]
Bug 1371200. P2 - add more fields to MediaDecoderInit.

https://reviewboard.mozilla.org/r/147038/#review152182

> You've written identical code to call MediaDecoderInit's constructor three times; it could be in a helper function.

We will have different initializations for InitializeDecoderAsClone() and InitializeDecoderForChannel() soon. So I don't bother to extract the common code here.
Thanks for the review!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a12048e898ee
P1 - add MediaDecoderInit and fix MediaDecoder constructor and its friends. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/dca576ac7065
P2 - add more fields to MediaDecoderInit. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/4a76d18a41c6
P3 - remove unused code. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/d2fd192ad927
P4 - constify some members. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/9858be1746b6
P5 - devirtualize some functions that don't have overrides. r=cpearce
You need to log in before you can comment on or make changes to this bug.