Closed Bug 524737 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Audio/Video, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- .7-fixed

People

(Reporter: bartml, Assigned: bartml)

References

Details

Attachments

(2 files)

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
Adding roc@ to CC list as he is the last person that changed nsDocument.cpp.
I believe this should be WONTFIX and we should remove --disable-ogg
do we have OGG on all platforms including OS/2 etc. ports?
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.
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.
(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?)
Attachment #408704 - Flags: review?(roc)
Comment on attachment 408704 [details] [diff] [review]
ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg + --disable-wave

No sr required
Attachment #408704 - Flags: review?(roc) → review+
BartZilla: do you need a commiter?
Assignee: nobody → bartml
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.]
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).)
Summary: trunk doesn't compile with --disable-ogg → trunk doesn't compile with --disable-ogg + --disable-wave
(In reply to comment #8)
> BartZilla: do you need a commiter?

Umm, what do you mean exactly?
Attachment #408704 - Attachment description: ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg → ifdef nsHTMLMediaElement.h to fix compilation with --disable-ogg + --disable-wave
Keywords: checkin-needed
He means "do you need someone to commit this for you". Looks like the answer is yes, given the checkin-needed marking!
fixed as in http://hg.mozilla.org/mozilla-central/rev/03aea0cb3e60 .

Thanks BartZilla for your work!
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.3a1
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?
Attachment #408704 - Flags: approval1.9.2.1?
this is the patch against 1.9.2 head
Attachment #424211 - Flags: approval1.9.2.1?
Attachment #408704 - Flags: approval1.9.2.1?
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.
Attachment #424211 - Flags: approval1.9.2.2? → approval1.9.2.3?
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
Attachment #424211 - Flags: approval1.9.2.4? → approval1.9.2.5+
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.
Attachment #424211 - Flags: approval1.9.2.5+ → approval1.9.2.6+
You need to log in before you can comment on or make changes to this bug.