1134487 - GMPAudioDecoder and GMPVideoDecoder use delegated constructors which are not supported by GCC 4.6

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Brian Birtles (:birtles)]

e.g.
http://hg.mozilla.org/mozilla-central/file/06e956a91995/dom/media/fmp4/gmp/GMPAudioDecoder.h#l62

https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code currently says these shouldn't be used

Comment 1

[Brian Birtles (:birtles)]

Attachment: Patch

Comment 2

[Brian Birtles (:birtles)]

Attachment: Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

Comment 3

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

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

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

r+ in case Bob Owen wants to land this in order to make gecko 37/38 build with old GCC... (see discussion in bug 1121258)

Comment 4

[Bob Owen (:bobowen)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a9b586aeae23

Comment 5

[Bob Owen (:bobowen)]

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

Approval Request Comment
[Feature/regressing bug #]:
The patch for Bug 1121258 used delegated constructors, which aren't supported in gcc-4.6 (our officially supported min version).

[User impact if declined]:
Package maintainers and people in general won't be able to build beta versions of Fx37 with gcc-4.6.
We are hoping to drop 4.6 soon, but doing so in Fx37 is a bit late notice.
Particularly as our MDN pages specifically state that gcc-4.6 is the min version for Fx37.

[Describe test coverage new/current, TreeHerder]:
The GMP code is covered by mochitests.
I have confirmed that Fx37 does build with gcc-4.6 (on Ubuntu 12.04) with this patch applied.

[Risks and why]:
Low - Functionally this change should make no difference.

[String/UUID change made/needed]:
None

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a9b586aeae23

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

Yes. We should take this low risk fix. If we're dropping support for 4.6 in 38, is this change required on Aurora?

Comment 8

[Bob Owen (:bobowen)]

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #7)
> Comment on attachment 8566858 [details] [diff] [review]
> Remove delegated constructors in GMP{Audio,Video}Decoder since they're not
> supported by GCC 4.6
> 
> Yes. We should take this low risk fix. If we're dropping support for 4.6 in
> 38, is this change required on Aurora?

Thanks and I suppose not at the moment for Aurora, but there are still some questions around gcc-4.6 and Android / B2G builds, so we might have to take it at some point.
But with the (possible partial) back-out for bug 1102195, this would be the least of our problems. :-)

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

Clearing Aurora approval request based on comment 8.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/64c87e9b4fbe

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cd91ede303a9

Comment 12

[Bob Owen (:bobowen)]

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/cd91ede303a9

Re-flagging to keep things in sync as this hit Aurora anyway ... RyanVM is just too damn efficient.
Probably best to keep everything consistent anyway, given that EME should ship in Fx38.


I see it has also been merged to mozilla-b2g37_v2_2, so I assume I don't have to flag for that.

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8566858 [details] [diff] [review]
Remove delegated constructors in GMP{Audio,Video}Decoder since they're not supported by GCC 4.6

Ok, in that case marking (post-landing) as approved, for consistency.