810177 - Refactor nsMediaDecoder and friends so that they do not depend on nsHTMLMediaElement

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[(no longer active)]

I have a set of patches which refactor nsMediaDecoder and friends to not depend on nsHTMLMediaElement.  The basic idea is to create an nsMediaDecoderCallback interface which nsHTMLMediaElement would implement, and put everything needed by nsMediaDeocder on that interface.  In addition to that, there are a number of other cleanup patches which enabled me to eliminate many of the nsHTMLMediaElement.h #includes in the media code.

I need these patches for bug 792263.

Comment 1

[(no longer active)]

Attachment: Part 1: Move NextFrameStatus to nsMediaDecoder

Comment 2

[(no longer active)]

Attachment: Part 2: Break MetadataTags out of nsHTMLMediaElements

Comment 3

[(no longer active)]

Attachment: Part 3: Move the IsFormatEnabled methods from nsHTMLMediaElement to nsMediaDecoder

Comment 4

[(no longer active)]

Attachment: Part 4: Make nsMediaDecoder not depend on nsHTMLMediaElement

Comment 5

[(no longer active)]

https://tbpl.mozilla.org/?tree=Try&rev=98bd67898298

Comment 6

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

Comment on attachment 679949 [details] [diff] [review]
Part 4: Make nsMediaDecoder not depend on nsHTMLMediaElement

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

Thanks for doing this, this is a good idea.

nsMediaDecoderCallback isn't a great name, because the calls go both ways. How about calling it mozilla::MediaDecoderOwner? I'm moving everything else into mozilla namespace, so that'd be one less thing for me to move in. And of course s/callback/owner/g everywhere then. 

And can you put MediaDecoderOwnewr in its own header file? Hopefully that would reduce the amount of stuff nsHTMLMediaElement pulls in when it includes this definition.

Comment 7

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

Comment on attachment 679949 [details] [diff] [review]
Part 4: Make nsMediaDecoder not depend on nsHTMLMediaElement

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

Let's call that r+ with my comments addressed.

Comment 8

[Mozilla RelEng Bot]

Try run for 98bd67898298 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=98bd67898298
Results (out of 18 total builds):
    success: 9
    failure: 9
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-98bd67898298

Comment 9

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a61507b712eb
https://hg.mozilla.org/integration/mozilla-inbound/rev/145d08002c5a
https://hg.mozilla.org/integration/mozilla-inbound/rev/43ead32e1f4c
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc8bbf49b63a

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/a61507b712eb
https://hg.mozilla.org/mozilla-central/rev/145d08002c5a
https://hg.mozilla.org/mozilla-central/rev/43ead32e1f4c
https://hg.mozilla.org/mozilla-central/rev/bc8bbf49b63a