524737 - trunk doesn't compile with --disable-ogg + --disable-wave

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Bartłomiej Brzozowiec (BartZilla)]

User-Agent:       Konqueror
Build Identifier: 

Compiling trunk with --disable-ogg results in the following error ("Nie ma takiego pliku ani katalogu" means "No such file or directory"):

In file included from /home/users/firefox/mozhg/src/content/base/src/nsDocument.cpp:172:
../../../dist/include/nsHTMLMediaElement.h:40:28: error: nsMediaDecoder.h: Nie ma takiego pliku ani katalogu
In file included from /home/users/firefox/mozhg/src/content/base/src/nsDocument.cpp:54:
../../../dist/include/nsINode.h:874: warning: 'virtual PRUint32 nsINode::GetScriptTypeID() const' was hidden
/home/users/firefox/mozhg/src/content/base/src/nsDocument.h:907: warning:   by 'virtual nsresult nsDocument::GetScriptTypeID(PRUint32*)'
In file included from /home/users/firefox/mozhg/src/content/base/src/nsDocument.cpp:172:
../../../dist/include/nsHTMLMediaElement.h:288: error: 'nsMediaDecoder' was not declared in this scope
../../../dist/include/nsHTMLMediaElement.h:288: error: template argument 1 is invalid
../../../dist/include/nsHTMLMediaElement.h:294: error: 'nsMediaDecoder' has not been declared
../../../dist/include/nsHTMLMediaElement.h:306: error: 'nsMediaDecoder' has not been declared
../../../dist/include/nsHTMLMediaElement.h:391: error: 'nsMediaDecoder' was not declared in this scope
../../../dist/include/nsHTMLMediaElement.h:391: error: template argument 1 is invalid


Reproducible: Always

Comment 1

[Bartłomiej Brzozowiec (BartZilla)]

Adding roc@ to CC list as he is the last person that changed nsDocument.cpp.

Comment 2

[Benjamin Smedberg]

I believe this should be WONTFIX and we should remove --disable-ogg

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

do we have OGG on all platforms including OS/2 etc. ports?

Comment 4

[Benjamin Smedberg]

I don't know, but at this point I don't really care: if you want gecko, you need to support ogg. So the port fixup would be to make ogg work, not disable it.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We shouldn't remove --disable-ogg. Some people are actually shipping Gecko builds with --disable-ogg because of patent concerns. We can argue about whether that's a good idea, but we here shouldn't decide it.

The "#include "nsHTMLMediaElement.h"" just needs to be wrapped in #ifdef MOZ_MEDIA, that's all.

Comment 6

[Bartłomiej Brzozowiec (BartZilla)]

Attachment: ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg + --disable-wave

(In reply to comment #5)
> (...)
> The "#include "nsHTMLMediaElement.h"" just needs to be wrapped in #ifdef
> MOZ_MEDIA, that's all.

Indeed, I figured it out already and prepared a patch. (Similar problem was also in nsNodeUtils.cpp.)
(Does such simple fix in trunk need also superreview?)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 408704 [details] [diff] [review]
ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg + --disable-wave

No sr required

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

BartZilla: do you need a commiter?

Comment 9

[Justin Dolske [:Dolske]]

Is this really the right problem? Our Windows CE builds currently have --disable-ogg, and they're building fine. [We should probably remove that, as the issue with building Ogg on ARM has been fixed afaik.]

Comment 10

[Bartłomiej Brzozowiec (BartZilla)]

I have also --disable-wave in my mozconfig. I changed Summary of the bug accordingly. (If I read configure.in correctly, then MOZ_MEDIA is indeed set even with --disable-ogg (but not with --disable-ogg AND --disable-wave at the same time; MOZ_WAVE is defined by default, so effectively MOZ_MEDIA is set too).)

Comment 11

[Bartłomiej Brzozowiec (BartZilla)]

(In reply to comment #8)
> BartZilla: do you need a commiter?

Umm, what do you mean exactly?

Comment 12

[:Gavin Sharp [email: [e-mail]]]

He means "do you need someone to commit this for you". Looks like the answer is yes, given the checkin-needed marking!

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

fixed as in http://hg.mozilla.org/mozilla-central/rev/03aea0cb3e60 .

Thanks BartZilla for your work!

Comment 14

[Brian Crowder]

Comment on attachment 408704 [details] [diff] [review]
ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg + --disable-wave

This patch never made it to 1.9.2, but it's pretty trivial and helps debugging on mobile platforms (where we need to be able to build with minimal features to have a debuggable libxul, anything larger than 32MB isn't usable on WinCE currently).  Can we have it there?

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch against 1.9.2

this is the patch against 1.9.2 head

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 424211 [details] [diff] [review]
patch against 1.9.2

Didn't make 1.9.2.2, we'll look at next time.

Comment 17

[LegNeato]

Comment on attachment 424211 [details] [diff] [review]
patch against 1.9.2

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we
are still working on 1.9.2.4 on the relbranch

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 424211 [details] [diff] [review]
patch against 1.9.2

We're past 1.9.2.5 and in the final week of being open to patches for 1.9.2.6. Please land ASAP if you want this on the branch, approvals will be minused next week.

Comment 19

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2a4412197a18