Joe sent me a test case, and I can reproduce locally. Need to investigate what's going on...

Test case: http://people.mozilla.org/~eakhgari/webaudio/decode-864780.html

So what happens here is that we fail to read chunks out of the wave file here: <http://mxr.mozilla.org/mozilla-central/source/content/media/wave/WaveReader.cpp#406>. Here, aChunkSize is 20 and extra is 0. It seems like this wave file is kind of invalid, but I don't know enough about this code to see how we can work around it. Note that decoding this wave file through an <audio> element will result in the exact same failure.

FYI this file comes from a commercial music sample provider and can be read into various popular audio applications successfully (Audacity, Kontakt Player, Cubase, etc.). I don't know what program was used to create it originally.

Ralph, could you please look into this as well? Thanks!

Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.

Ralph -- Have you had a chance to look into this? I wonder how common this failure is. (If it's common, I'd want to make this a P1.) Thanks!

I haven't sorry. I don't think it can be common, but we should fix it. I'll take a look after the break.

Sorry I haven't been able to get to this. Best to give it to someone else.

I ran into this problem with a wav file I was trying to use and determined that the issue was that the "fmt" chunk was larger than 16 bytes. Here is the discussion on SO: http://stackoverflow.com/questions/26169678/why-certain-wav-files-cannot-be-decoded-in-firefox Here is my solution on jsfiddle: http://jsfiddle.net/2hav4yLc/11/

Hello, if you need to have some wav file that won't decode in Firefox (but will anywhere else, included Chrome & Audacity), I have plenty.

Also, https://wav.hya.io links at it.

(In reply to Cristiano Belloni from comment #10) > Hello, if you need to have some wav file that won't decode in Firefox (but > will anywhere else, included Chrome & Audacity), I have plenty. Christiano: Can I have such files to add them to my decoder testing tool? http://chinpen.net/decodethis

Hey Anthony -- This bug (and the linked bug 1074141) are both issues with our wav reader not handling wav files that various tools generate, and most audio tools can read with no problems. For example, https://wav.hya.io tells people basically to use chrome. I'm moving this to playback as it fails when these files are fed directly to an <audio> element. Note also the stackoverflow page linked to from the other bug.

Anyone want to attach an example WAV file to this bug.

Hello. I have a online suite of files which I use to check for browser codec compatibility. It uses the decodeAudioData in WebAudio. http://chinpen.net/decodethis/ All the audio files are here: https://github.com/notthetup/decodethis/tree/gh-pages/public/audio One of the wav files that fails for sure (it's 24bit) is https://raw.githubusercontent.com/notthetup/decodethis/gh-pages/public/audio/tom-1.wav

Comment on attachment 8689844 [details] [diff] [review] Added support for 24 bit wave files Review of attachment 8689844 [details] [diff] [review]: ----------------------------------------------------------------- Please encode a 24bit wav file, place it in dom/media/test/, add it to dom/media/test/mochitest.ini, and add it to gPlayTests in dom/media/manifest.js so that when you run `./mach mochitest dom/media/test/test_playback.html` we also hit your code. You need to `hg add $path/to/your/new/file` in order to include it in your patch. Also, Ehsan's original testcase that this bug was reported for still doesn't play. Can you figure that out? http://people.mozilla.org/~eakhgari/webaudio/decode-864780.html ::: dom/media/wave/WaveReader.cpp @@ +200,5 @@ > + > +template <> inline int16_t > +Signed24bIntToAudioSample<int16_t>(int32_t aValue) > +{ > + return aValue * 0x7fff / 0x800000; I'm not sure how this works. How does it work? You can download the Firefox for Android build on your try push and install that on your Android phone to test that 24bit wave files plays when this code is being used.

The original test fails because of an unexpected case in the metadata. Test Case: 00000000 52 49 46 46 0e 7e 01 00 57 41 56 45 66 6d 74 20 |RIFF.~..WAVEfmt | 00000010 14 00 00 00 01 00 01 00 44 ac 00 00 88 58 01 00 |........D....X..| 00000020 02 00 10 00 >00 00 00 00< 64 61 74 61 a2 7d 01 00 |........data.}..| 00000030 00 00 fe ff fd ff fa ff f7 ff f6 ff f4 ff f7 ff |................| Expected Metadata Example: 00000000 52 49 46 46 46 56 00 00 57 41 56 45 66 6d 74 20 |RIFFFV..WAVEfmt | 00000010 10 00 00 00 01 00 01 00 11 2b 00 00 22 56 00 00 |.........+.."V..| 00000020 02 00 10 00 64 61 74 61 22 56 00 00 ff 7f 00 80 |....data"V......| 00000030 00 00 00 80 00 80 00 80 00 80 00 80 00 80 00 80 |................| The decoder expects the first two bytes to tell it how far it should skip, so the bytes within > < would read 02 00 so that it would skip the other two bytes. This is not a problem with 24bit PCM, just with a specific format chunk.

Comment on attachment 8690605 [details] [diff] [review] bug-864780-fix.patch Review of attachment 8690605 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/wave/WaveReader.cpp @@ +431,5 @@ > extra += extra % 2; > > + static_assert(UINT16_MAX + (UINT16_MAX % 2) < UINT_MAX / sizeof(char), > + "chunkExtension array too large for iterator."); > + nsAutoArrayPtr<char> chunkExtension(new char[extra]); You're allocating memory based on an unsanitized value read from a file. In this case the allocation is it's limited to 2^16 bytes, which isn't big enough to be too dangerous, but it's still bad practice as a maliciously crafted file could exploit this. Since we're not doing anything with the data, there seems little point in reading/copying it. So instead of allocating memory and reading into it, you should skip over the data so that the next read start after it. Do this by adding: if (NS_FAILED(mResource.Seek(nsISeekableStream::NS_SEEK_CUR, extra))) { return false; } This seeks the read cursor forward "extra" bytes ahead of the current read cursor position, and returns false (i.e. indicates failure) if the seek fails.

sorry failed to apply: applying bug-864780-fix.patch patching file dom/media/test/manifest.js Hunk #1 FAILED at 148 1 out of 1 hunks FAILED -- saving rejects to file dom/media/test/manifest.js.rej patching file dom/media/wave/WaveReader.cpp Hunk #1 FAILED at 421 1 out of 1 hunks FAILED -- saving rejects to file dom/media/wave/WaveReader.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug-864780-fix.patch

Hi, this seems to have still problems - patching file dom/media/test/manifest.js Hunk #1 FAILED at 148 1 out of 1 hunks FAILED -- saving rejects to file dom/media/test/manifest.js.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug-864780-fix.patch

(In reply to Carsten Book [:Tomcat] from comment #27) > Hi, this seems to have still problems - patching file > dom/media/test/manifest.js > Hunk #1 FAILED at 148 > 1 out of 1 hunks FAILED -- saving rejects to file > dom/media/test/manifest.js.rej > patch failed, unable to continue (try -v) > patch failed, rejects left in working directory > errors during apply, please fix and qrefresh bug-864780-fix.patch oh never mind, this depened on the other bug

sorry had to back this out seems this or the other change caused test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=18535643&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/16563b744913

This patch needs to be applied after the patch uploaded to https://bugzilla.mozilla.org/show_bug.cgi?id=524109