Last Comment Bug 449315 - Support WAV format in <audio> element
: Support WAV format in <audio> element
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
: -- enhancement with 2 votes (vote)
: ---
Assigned To: Matthew Gregan [:kinetik]
:
Mentors:
Depends on: 463537 465165
Blocks: 92110 video 463929
  Show dependency treegraph
 
Reported: 2008-08-05 17:41 PDT by Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
Modified: 2008-11-16 05:37 PST (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v0 (36.72 KB, patch)
2008-10-16 22:19 PDT, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v1 (57.74 KB, patch)
2008-11-02 13:19 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2 (64.86 KB, patch)
2008-11-03 19:25 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.1 (64.86 KB, patch)
2008-11-03 19:39 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.2 (65.04 KB, patch)
2008-11-03 21:36 PST, Matthew Gregan [:kinetik]
roc: review+
roc: superreview+
Details | Diff | Splinter Review
patch v2.3 (65.07 KB, patch)
2008-11-04 00:18 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.4 (66.95 KB, patch)
2008-11-04 01:31 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.5 (65.29 KB, patch)
2008-11-04 13:52 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.6 (65.09 KB, patch)
2008-11-04 13:58 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
test wave file (21.58 KB, audio/x-wav)
2008-11-04 20:41 PST, Matthew Gregan [:kinetik]
no flags Details
fake audio backend (3.36 KB, patch)
2008-11-05 20:16 PST, Matthew Gregan [:kinetik]
roc: review+
roc: superreview+
Details | Diff | Splinter Review
fake audio backend v2 (3.39 KB, patch)
2008-11-05 21:04 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review
patch v2.7 (66.71 KB, patch)
2008-11-05 21:09 PST, Matthew Gregan [:kinetik]
no flags Details | Diff | Splinter Review

Description Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-08-05 17:41:11 PDT
The HTML5 spec says we should support WAV, and it makes sense. We need an internal decoder backend for WAV. This should actually be slightly higher priority than the platform framework backends.
Comment 1 Matthew Gregan [:kinetik] 2008-10-16 22:19:25 PDT
Created attachment 343507 [details] [diff] [review]
patch v0

Preliminary patch for early review.
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-10-17 02:06:15 PDT
 #ifdef MOZ_OGG
 #include "nsOggDecoder.h"
+#include "nsWaveDecoder.h"
 #endif

This should not be inside MOZ_OGG, should it?

nsWaveDecoder could use a lot of comments. Also, the methods that override nsMediaDecoder should be marked "virtual".

Maybe we should make the buffering parameters preferences and share them between backends as much as possible? Or do we need different parameter values for different backends?

nsWaveStateMachine needs lots of comments about the fields.

+      monitor.Wait();
+    }

This naked Wait in STATE_END is probably wrong. You'll want a do { Wait } while (state != SHUTDOWN), I think.

+      }
+    }
+      break;

Weird indentation.

+  nsresult rv = mStream->Read(buf, sizeof(buf), &got);
+  if (NS_FAILED(rv) || got != sizeof(buf)) {
+    NS_WARNING("Stream read failed");
+    return PR_FALSE;

So we don't have to worry about short reads? is that documented somewhere?

Could simplify things a bit by making Uint16/32LE take &p and automatically advance p.

Basically looks good. We should think about what we could do to increase code sharing here with Ogg, but we have to be careful not to share too much or too little.
Comment 3 Matthew Gregan [:kinetik] 2008-11-02 13:19:49 PST
Created attachment 345969 [details] [diff] [review]
patch v1

Addresses review comments except for code sharing with Ogg backend and prefs for buffering wait values.  I agree that we should do at least the first, and probably the second.
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-02 14:52:16 PST
+#if !defined(nsAudioStream_h___)
+#define nsAudioStream_h___

At most one trailing underscore.

You need to update your changes to nsAudioStream to account for the fact that nsAudioStream is now responsible for scaling sample values according to mVolume.

There are also a few little changes due to the landing of standalone video/audio documents.

+#include "algorithm"
+#include "limits"

Get rid of these.

+  // Our audio backend.  Created on demand when entering playback state.  It
+  // is destroyed when seeking begins and will not be reinitialized until
+  // playback resumes, so it is possible for this to be null.
+  nsAutoPtr<nsAudioStream> mAudioStream;

I think "Our audio stream" would be more accurate?

+      if (LoadRIFFChunk() && LoadFormatChunk() && FindDataOffset()) {
+        nsCOMPtr<nsIRunnable> event =
+          NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, MetadataLoaded);
+        NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);

Shouldn't we check for SHUTDOWN state before firing this event?

+      if (mState != STATE_BUFFERING) {
+        nsCOMPtr<nsIRunnable> event =
+          NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, BufferingStopped);
+        NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);

Ditto. (Maybe after the Wait(1000) we should check for SHUTDOWN and bail out there if necessary.)

+      if (mExpectMoreData == PR_FALSE && mStream->Available() < mSampleSize) {

if (!mExpectMoreData

+            if (RoundDownToSample(len) != len) {
+              NS_WARNING("PCM data does not end with complete sample");
+            }

Shouldn't we set len to RoundDownToSample(len) here to avoid sending garbage to the audio device?

+        // Sleep until approximately half of the currently buffered audio has
+        // been consumed by the audio hardware.

What's the rationale for this?

+        nsCOMPtr<nsIRunnable> startEvent =
+          NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, SeekingStarted);
+        NS_DispatchToMainThread(startEvent, NS_DISPATCH_SYNC);

Need to check SHUTDOWN state after this.

+      if (mAudioStream) {
+        mAudioStream->Drain();
+      }

We should not be holding the monitor while we do this (as discussed F2F). We should also check for SHUTDOWN state before firing the following event.

+    case STATE_ERROR:
+      do {
+        monitor.Wait();
+      } while (mState != STATE_SHUTDOWN);
+      break;

This could be simplified to just a single monitor.Wait().

+      return NS_OK;
+      break;

No need for break here.

+static PRUint32
+ReadChunkName(const char** aBuffer)
+{
+  PRUint32 result =
+    PRUint8((*aBuffer)[0]) << 24 |
+    PRUint8((*aBuffer)[1]) << 16 |
+    PRUint8((*aBuffer)[2]) << 8 |
+    PRUint8((*aBuffer)[3]);
+  *aBuffer += sizeof(PRUint32);
+  return result;
+}

Can't we call this ReadUint32BE?

+  do {
+    PRUint32 read = 0;
+    nsresult rv = mStream->Read(buf + got, sizeof(buf) - got, &read);
+    if (NS_FAILED(rv)) {
+      NS_WARNING("Stream read failed");
+      return PR_FALSE;
+    }
+    got += read;
+  } while (got != sizeof(buf));

Factor this into some kind of ReadAll helper and use it from a few places.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-02 14:56:05 PST
> +#include "limits"
> 
> Get rid of these.

I guess we need to keep this one.
Comment 6 Matthew Gregan [:kinetik] 2008-11-03 19:25:56 PST
Created attachment 346188 [details] [diff] [review]
patch v2

Here's the latest version.  Changes:

- update to tip (adjusted nsAudioStream changes for volume handling, reworked changes to nsHTMLMediaElement for media selection and loading with existing channels)
- address review comments
- resurrect nsAudioStream::{Pause,Resume,GetTime} (recently removed by doublec when he stopped using them)
- add a test (more coming, will submit them separately)
- fixed some bugs
- inverted logic in configure.in so Wave is enabled by default

This has been built and tested on OS X, Linux, and Windows.
Comment 7 Matthew Gregan [:kinetik] 2008-11-03 19:39:10 PST
Created attachment 346191 [details] [diff] [review]
patch v2.1

Missed a line when inverting the configure logic for Wave.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-03 20:54:00 PST
+      monitor.Exit();
+
+      {
+        nsCOMPtr<nsIRunnable> startEvent =
+          NS_NEW_RUNNABLE_METHOD(nsWaveDecoder, mDecoder, SeekingStarted);
+        NS_DispatchToMainThread(startEvent, NS_DISPATCH_SYNC);
+
+        if (mState == STATE_SHUTDOWN) {
+          break;
+        }

You can't check mState without holding the monitor...
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-03 20:55:22 PST
You should probably check mState after seeking and before firing SeekingStopped, too.
Comment 10 Matthew Gregan [:kinetik] 2008-11-03 21:36:01 PST
Created attachment 346200 [details] [diff] [review]
patch v2.2

Nice catch.  Latest patch fixes those two issues.
Comment 11 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-03 22:04:51 PST
Comment on attachment 346200 [details] [diff] [review]
patch v2.2

The 'break's inside {} can just stand alone. Whoever  checks this in can fix that, if necessary.

Why are you removing test_start.html from the Makefile?
Comment 12 Matthew Gregan [:kinetik] 2008-11-04 00:18:02 PST
Created attachment 346220 [details] [diff] [review]
patch v2.3

Sorry, removing test_start.html was not intentional, it was accidentally picked up in a qrefresh while poking at some other tests.

I didn't remove the braces around the breaks because it'd make the rest of the file inconsistent--I included braces around every single statement conditional/loop.
Comment 14 cajbir (:cajbir) 2008-11-04 00:58:21 PST
Backed out:

http://hg.mozilla.org/mozilla-central/rev/18403769ec19

/builds/moz2_slave/mozilla-central-linux-debug/build/content/html/content/src/nsHTMLMediaElement.cpp: In member function ‘PRBool nsHTMLMediaElement::CreateDecoder(const nsACString_internal&)’:
/builds/moz2_slave/mozilla-central-linux-debug/build/content/html/content/src/nsHTMLMediaElement.cpp:702: error: cannot allocate an object of abstract type ‘nsWaveDecoder’
../../../../dist/include/content/nsWaveDecoder.h:141: note:   because the following virtual functions are pure within ‘nsWaveDecoder’:
../../../../dist/include/content/nsMediaDecoder.h:149: note: 	virtual void nsMediaDecoder::SetSeekable(PRBool)
../../../../dist/include/content/nsMediaDecoder.h:152: note: 	virtual PRBool nsMediaDecoder::GetSeekable()
gmake[7]: *** [nsHTMLMediaElement.o] Error 1
Comment 15 Robert Sayre 2008-11-04 01:01:00 PST
This is a pretty big patch to take with no tests. Shouldn't it be able to play rheeet at the very least?
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-04 01:07:25 PST
There is a test:
+++ b/content/media/video/test/test_wav_ended1.html

and there are some more tests coming (WAV versions of most of the Ogg tests).
Comment 17 Matthew Gregan [:kinetik] 2008-11-04 01:31:00 PST
Created attachment 346228 [details] [diff] [review]
patch v2.4

Rebase against current tip--the duration changes for the Ogg backend that landed shortly before this patch added a pure virtual to the nsMediaDecoder base class.  Fix is to add a trivial implementation to nsWaveDecoder, so no additional review required.
Comment 18 Matthew Gregan [:kinetik] 2008-11-04 01:31:44 PST
Sadly we can't play ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/libraries/bonus-tracks/rheet.wav because the audio backends only support 16-bit samples for now.
Comment 19 Axel Hecht [pto-Aug-30][:Pike] 2008-11-04 02:15:49 PST
The library checks in configure.in broke --disable-compile-environment. We don't need audio libs without compiling, which is what we're doing for l10n builds.

See http://l10n.mozilla.org/buildbot/builders/hg-langpack/builds/5115/steps/configure/logs/stdio for a sample failure log.

There are a bunch of examples in configure.in on how to wrap your code such that it's not run when COMPILE_ENVIRONMENT is off.
Comment 20 Matthew Gregan [:kinetik] 2008-11-04 13:52:45 PST
Created attachment 346328 [details] [diff] [review]
patch v2.5

Address Pike's issue, rebase against trunk (no change, but diff hunks moved around).  This includes the changes required for the patch in bug 449307 already (and will also build without that patch), so the two patches can be applied in any order.
Comment 21 Matthew Gregan [:kinetik] 2008-11-04 13:58:09 PST
Created attachment 346329 [details] [diff] [review]
patch v2.6

Remove setTimeout from test_wav_ended1.html like cpearce is doing for the existing media tests in bug 462933.
Comment 22 Matthew Gregan [:kinetik] 2008-11-04 20:41:04 PST
Created attachment 346396 [details]
test wave file

patch v2.6 includes a binary file (a test wave) encoded in extended hg/git patch format, which means the patch must be imported using hg import or the binary file will be ignored.

I've had a report from cpearce that hg import crashes on Win32 for him, so I'm attaching the file separately for anyone who runs into the same problem or wants to apply the patch to their tree with patch rather than hg import.
Comment 23 Justin Dolske [:Dolske] 2008-11-05 16:38:43 PST
This was landed, and subsequently backed out due to unit test timeouts.
Comment 24 Matthew Gregan [:kinetik] 2008-11-05 17:33:04 PST
moz2-win32-slave07 logged:

*** 27746 INFO Running /tests/content/media/video/test/test_wav_ended1.html...
[WAVE API] System driver not present error returned while executing opening audio device for playback
[WAVE API] Invalid device handle error returned while executing resetting audio device

This is easy to reproduce, just disable the sound card in device manager.  Because the audio backend fails to intialize, nsAudioStream::Available will always return 0.  This means the playback loop in nsWaveStateMachine::Run will never make progress, and thus we'll never transition to STATE_ENDED and fire the playbackEnded event.

My guess is that moz2-linux-slave08 timed out for the same reason.
Comment 25 Matthew Gregan [:kinetik] 2008-11-05 17:37:24 PST
If the audio backend fails to open, we won't be able report useful times for currentTime, so tests depending on that are going to fail too.
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-05 17:48:00 PST
I talked about this with Chris D. This is annoying, but we do have to handle this case ... even though audio's not playing, we still want to behave roughly as if we had a real audio device. Otherwise Web pages using these features could break in strange ways.

Maybe you can fix this at the nsAudioStream level, by detecting no audio device and faking it.
Comment 27 Matthew Gregan [:kinetik] 2008-11-05 20:16:06 PST
Created attachment 346607 [details] [diff] [review]
fake audio backend

Modify nsAudioStream to behave as if it is playing audio, even if the underlying backend failed to initialize.  This is necessary to provide correct behaviour on systems where the backend failed to initialize, such as machines without sound hardware/drivers.
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-05 20:25:38 PST
Comment on attachment 346607 [details] [diff] [review]
fake audio backend

Please don't overload mStartTime to be both a wall-clock time and a stream elapsed time --- use a separate variable for one of them. Otherwise looks good. Please do run the full set of Ogg and WAV tests with and without sound enabled, then attach a complete patch. Thanks!
Comment 29 Matthew Gregan [:kinetik] 2008-11-05 21:04:44 PST
Created attachment 346611 [details] [diff] [review]
fake audio backend v2

Address review comments.  Attaching for interdiff purposes only.  Rolled up patching coming up.
Comment 30 Matthew Gregan [:kinetik] 2008-11-05 21:09:22 PST
Created attachment 346612 [details] [diff] [review]
patch v2.7

Includes nsAudioStream changes reviewed separately in the fake audio backend patch.  Built and tested all content/media/video/test tests on Mac with working and fake audio backends.  Also tested test_wav_ended1.html with the fake audio backend on Win32 and the sound card disabled.
Comment 31 mdew 2008-11-06 14:03:54 PST
this patch causes,

c++ -o nsOggDecoder.o -c -I../../../../dist/include/system_wrappers -include ../../../../config/gcc_hidden.h -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_COM_OBSOLETE -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DZLIB_INTERNAL -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux  -I. -I. -I../../../../dist/include/xpcom -I../../../../dist/include/string -I../../../../dist/include/gfx -I../../../../dist/include/content -I../../../../dist/include/thebes -I../../../../dist/include/layout -I../../../../dist/include/widget -I../../../../dist/include/dom -I../../../../dist/include/js -I../../../../dist/include/locale -I../../../../dist/include/unicharutil -I../../../../dist/include/webshell -I../../../../dist/include/uriloader -I../../../../dist/include/htmlparser -I../../../../dist/include/necko -I../../../../dist/include/view -I../../../../dist/include/pref -I../../../../dist/include/docshell -I../../../../dist/include/xpconnect -I../../../../dist/include/xuldoc -I../../../../dist/include/caps -I../../../../dist/include/editor -I../../../../dist/include/imglib2 -I../../../../dist/include/mimetype -I../../../../dist/include/exthandler -I../../../../dist/include/uconv -I../../../../dist/include/intl -I../../../../dist/include/plugin -I../../../../dist/include/cairo -I../../../../dist/include/libpixman -I../../../../dist/include   -I../../../../dist/include/content -I/home/mdew/ff/mozilla-central/dist/include/nspr     -I/home/mdew/ff/mozilla-central/dist/sdk/include -I./../../../base/src -I./../../../html/content/src     -fPIC   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -O3 -msse2 -mmmx   -DMOZILLA_CLIENT -include ../../../../mozilla-config.h -Wp,-MD,.deps/nsOggDecoder.pp nsOggDecoder.cpp
nsOggDecoder.cpp:45:27: error: nsAudioStream.h: No such file or directory
nsOggDecoder.cpp: In member function ‘PRBool nsOggDecodeStateMachine::PlayAudio(nsOggDecodeStateMachine::FrameData*)’:
nsOggDecoder.cpp:664: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp:667: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp: In member function ‘void nsOggDecodeStateMachine::OpenAudioStream()’:
nsOggDecoder.cpp:676: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp:681: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp:682: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp: In member function ‘void nsOggDecodeStateMachine::CloseAudioStream()’:
nsOggDecoder.cpp:690: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
nsOggDecoder.cpp: In member function ‘void nsOggDecodeStateMachine::SetVolume(float)’:
nsOggDecoder.cpp:764: error: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: error: forward declaration of ‘struct nsAudioStream’
../../../../dist/include/xpcom/nsAutoPtr.h: In destructor ‘nsAutoPtr<T>::~nsAutoPtr() [with T = nsAudioStream]’:
nsOggDecoder.cpp:458:   instantiated from here
../../../../dist/include/xpcom/nsAutoPtr.h:104: warning: possible problem detected in invocation of delete operator:
../../../../dist/include/xpcom/nsAutoPtr.h:104: warning: invalid use of incomplete type ‘struct nsAudioStream’
../../../../dist/include/content/nsOggDecoder.h:269: warning: forward declaration of ‘struct nsAudioStream’
../../../../dist/include/xpcom/nsAutoPtr.h:104: note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.
../../../../dist/include/xpcom/nsAutoPtr.h: In member function ‘void nsAutoPtr<T>::assign(T*) [with T = nsAudioStream]’:
../../../../dist/include/xpcom/nsAutoPtr.h:134:   instantiated from ‘nsAutoPtr<T>& nsAutoPtr<T>::operator=(T*) [with T = nsAudioStream]’
nsOggDecoder.cpp:691:   instantiated from here
../../../../dist/include/xpcom/nsAutoPtr.h:71: warning: possible problem detected in invocation of delete operator:
../../../../dist/include/xpcom/nsAutoPtr.h:69: warning: ‘oldPtr’ has incomplete type
../../../../dist/include/content/nsOggDecoder.h:269: warning: forward declaration of ‘struct nsAudioStream’
../../../../dist/include/xpcom/nsAutoPtr.h:71: note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.
make[6]: *** [nsOggDecoder.o] Error 1
make[6]: Leaving directory `/home/mdew/ff/mozilla-central/content/media/video/src'
make[5]: *** [src_libs] Error 2
make[5]: Leaving directory `/home/mdew/ff/mozilla-central/content/media/video'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/home/mdew/ff/mozilla-central/content/media'
make[3]: *** [media_libs] Error 2
make[3]: Leaving directory `/home/mdew/ff/mozilla-central/content'
make[2]: *** [libs_tier_gecko] Error 2
make[2]: Leaving directory `/home/mdew/ff/mozilla-central'
make[1]: *** [tier_gecko] Error 2
make[1]: Leaving directory `/home/mdew/ff/mozilla-central'
make: *** [default] Error 2
Comment 32 Matthew Gregan [:kinetik] 2008-11-06 14:32:57 PST
(In reply to comment #31)
> this patch causes,
> 
> -O3 -msse2 -mmmx   -DMOZILLA_CLIENT -include ../../../../mozilla-config.h
> -Wp,-MD,.deps/nsOggDecoder.pp nsOggDecoder.cpp
> nsOggDecoder.cpp:45:27: error: nsAudioStream.h: No such file or directory

You need to regenerate configure and rerun it.
Comment 33 mdew 2008-11-06 16:15:28 PST
(In reply to comment #32)
> (In reply to comment #31)
> > this patch causes,
> > 
> > -O3 -msse2 -mmmx   -DMOZILLA_CLIENT -include ../../../../mozilla-config.h
> > -Wp,-MD,.deps/nsOggDecoder.pp nsOggDecoder.cpp
> > nsOggDecoder.cpp:45:27: error: nsAudioStream.h: No such file or directory
> 
> You need to regenerate configure and rerun it.

confirmed, thanks :)
Comment 34 Matthew Gregan [:kinetik] 2008-11-06 17:04:47 PST
http://hg.mozilla.org/mozilla-central/rev/3dea3415b003
Comment 35 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-11-08 06:05:07 PST
Is this now working in current trunk build? Can the checkin-needed keyword be removed?
I tried to get this working, using: http://martijn.martijn.googlepages.com/test_wav.html
But I don't get to hear any sound, am I doing something wrong?
Comment 36 Matthew Gregan [:kinetik] 2008-11-08 10:47:40 PST
Yes, thanks for the reminder.  As for your testcase, audio/video elements don't play by default, so you need to either set the autoplay attribute, or call the play method on the element.  In this case, block.wav is 8-bit, so we can't play it back anyway because the audio backend (libsydneyaudio) doesn't support playback of 8-bit samples.
Comment 37 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-11-08 11:06:47 PST
Ok, thanks, do you know of an example .wav file where I do get sound with then? I tried the one from http://mxr.mozilla.org/mozilla-central/source/content/media/video/test/r11025_s16_c1.wav but that doesn't seem to generate sound either?
I guess the 8-bit .wav no sound thing is a bug, right?
Comment 38 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-09 13:09:45 PST
Seems like there's quite a bit of 8-bit WAV around, perhaps we should implement that for 3.1.
Comment 39 Matthew Gregan [:kinetik] 2008-11-09 14:02:20 PST
r11025_s16_c1.wav is a silent test file, so at most you'll hear a click as it
plays.

We should treat the lack of 8-bit sample support as a bug.  I've opened bug
463929 for this.

Note You need to log in before you can comment on or make changes to this bug.