Chrome can decode this wave file without any problems...

This is a 4-channel 16-bit pcm wav file with no channel mapping. We don't play it because we don't support multichannel output or stereo downmix for wav files. How does chrome interpret the channels without a channel mapping?

http://chromium.googlecode.com/svn/trunk/samples/audio/impulse-responses/spreader50-65ms.wav looks like the same limitation.

(In reply to comment #1) > This is a 4-channel 16-bit pcm wav file with no channel mapping. > > We don't play it because we don't support multichannel output or stereo downmix > for wav files. > > How does chrome interpret the channels without a channel mapping? I'm not sure, but you get an AudioBuffer if you pass this to AudioContext.decodeAudioData/createBuffer. For Web Audio, we should not be attempting to map channels to anything, we should just give back the raw PCM version of each channel, I think.

We need to fix this before we ship Web Audio, since convolution buffers may come from wave files with more than two channels. Ralph, do you know what's going to be involved in fixing this bug?

Great, bugzilla outage cleared needinfo? but lost my comment. Ehsan, to fix this properly we need to support multichannel playback. How important is the 4-channel ConvolverNode buffer? That's a more invasive change than I'd like to make to Aurora. Currently we downmix multichannel Opus files in the decoder and reject all other multichannel files. We'd need to move the downmix code to the audio output layer. Bug 709327 is the tracking bug; it's been low priority because game dev feedback was that actual surround output wasn't important. Then we have the issue of channel map. This file *should* fail in normal <audio> playback because there's no channel map, thus no way to play directly. Maybe we could plumb a flag which just asks the decoder to return multichannel data regardless for webaudio purposes?

(In reply to Ralph Giles (:rillian) from comment #6) > Great, bugzilla outage cleared needinfo? but lost my comment. > > Ehsan, to fix this properly we need to support multichannel playback. How > important is the 4-channel ConvolverNode buffer? That's a more invasive > change than I'd like to make to Aurora. > > Currently we downmix multichannel Opus files in the decoder and reject all > other multichannel files. We'd need to move the downmix code to the audio > output layer. Bug 709327 is the tracking bug; it's been low priority because > game dev feedback was that actual surround output wasn't important. (Wrong bug #?) Hmm, I was hoping to not require multi-channel playback as part of this. Can't we just special case the decoding done by the AudioContext to decode each channel as it is, bypassing the downmix spec > Then we have the issue of channel map. This file *should* fail in normal > <audio> playback because there's no channel map, thus no way to play > directly. Maybe we could plumb a flag which just asks the decoder to return > multichannel data regardless for webaudio purposes? Yeah, that's exactly what I would hope that we should do here. Is that easier than the multichannel playback you mentioned above?

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #7) > > Then we have the issue of channel map. This file *should* fail in normal > > <audio> playback because there's no channel map, thus no way to play > > directly. Maybe we could plumb a flag which just asks the decoder to return > > multichannel data regardless for webaudio purposes? > > Yeah, that's exactly what I would hope that we should do here. Is that > easier than the multichannel playback you mentioned above? Yes, that's easier. I'd recommend that unless Paul particular wants to fix multichannel playback in the timeframe we're looking at.

That's the approach I'm going to take, yes. It's going to be somewhat hacky, but well...

Chris, could you look at this one? The idea here is that we want to be able to decode 4 channel files for WebAudio, and the WaveReader currently errors out while reading the metadata, when the file has four channels.

Test for multichannel file decoding. This tests both that WebAudio can decode the file, and <audio> cannot.

Comment on attachment 796762 [details] [diff] [review] Allow decoding files we know we can't play, in the context of WebAudio. r= Review of attachment 796762 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. ::: content/media/MediaDecoderReader.h @@ +467,5 @@ > // decode thread could start up and run in future. > virtual void OnDecodeThreadFinish() {} > > + // Tell the reader that the data decoded are not for direct playback, so it > + // can accept more file, in particular those which have more channels than "...accept more files." @@ +554,5 @@ > + > + // Whether we should accept media that we know we can't play > + // directly, because they have a number of channel higher than > + // what we support. > + bool mIgnoreAudioOutputFormat; I would have called it mDecodeAllChannels, but that's not any shorter. ::: content/media/wave/WaveReader.cpp @@ +61,5 @@ > // supported by AudioStream. > static const uint16_t WAVE_FORMAT_ENCODING_PCM = 1; > > +// Maximum number of channels supported by the audio output code. We return with > +// an error now if we are going to play back this media directly. "We reject files with more than this number of channels if we're decoding for playback." @@ +436,4 @@ > unsigned int actualFrameSize = sampleFormat == 8 ? 1 : 2 * channels; > if (rate < 100 || rate > 96000 || > + (((channels < 1 || channels > MAX_CHANNELS) || > + (frameSize != 1 && frameSize != 2 && frameSize != 4)) && I think the direct frameSize checks are covered by the 'frameSize != actualFrameSize' check below, and technically it's wrong if MAX_CHANNELS isn't 2, but that's a separate bug. When we add multichannel playback it will show up soon enough, so I wouldn't worry about fixing it.

Comment on attachment 796764 [details] [diff] [review] Test that we can decode multichannel WAVE file only using decodeAudioData. r= What happens if both SimpleText.finish() calls in decodeUsingAudioElement are invoked?

Comment on attachment 796762 [details] [diff] [review] Allow decoding files we know we can't play, in the context of WebAudio. r= Review of attachment 796762 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/MediaBufferDecoder.cpp @@ +512,5 @@ > > + // Tell the decoder reader that we are not going to play the data directly, > + // and that we should not reject files with more channels than the audio > + // bakend support. > + mDecoderReader->SetIgnoreAudioOutputFormat(); Shouldn't we technically call this before OnDecodeThreadStart()?

Comment on attachment 796764 [details] [diff] [review] Test that we can decode multichannel WAVE file only using decodeAudioData. r= Review of attachment 796764 [details] [diff] [review]: ----------------------------------------------------------------- r=me sans the double finish bug that Ralph pointed out.

Comment on attachment 796762 [details] [diff] [review] Allow decoding files we know we can't play, in the context of WebAudio. r= Review of attachment 796762 [details] [diff] [review]: ----------------------------------------------------------------- Chris Double wrote our wave decoder, he should be the one reviewing this.

Comment on attachment 796762 [details] [diff] [review] Allow decoding files we know we can't play, in the context of WebAudio. r= Matthew Gregan was the original author of the wave decoder.

Comment on attachment 796762 [details] [diff] [review] Allow decoding files we know we can't play, in the context of WebAudio. r= Review of attachment 796762 [details] [diff] [review]: ----------------------------------------------------------------- To clarify: the Wave decoder is limited to mono or stereo because it only supports the original Wave header and has no support for WAVE_FORMAT_EXTENSIBLE. Supporting multichannel files reliably will require extending WaveReader to support WAVE_FORMAT_EXTENSIBLE.

https://hg.mozilla.org/integration/mozilla-inbound/rev/09a51539cea1 https://hg.mozilla.org/integration/mozilla-inbound/rev/00611894f123

Backed out for breaking the build: https://tbpl.mozilla.org/php/getParsedLog.php?id=27165267&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=27165241&tree=Mozilla-Inbound remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e992506c2a0 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ec9bf60a4b2e

Re-pushed with actually valid code. remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d14cd9307c3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/807e7c9465eb

https://hg.mozilla.org/mozilla-central/rev/1d14cd9307c3 https://hg.mozilla.org/mozilla-central/rev/807e7c9465eb

https://hg.mozilla.org/releases/mozilla-aurora/rev/df1504b82610 https://hg.mozilla.org/releases/mozilla-aurora/rev/1a1aeff8ab00

While testing for the pre-beta sign-off of this feature, I tried to verify this bug, so here are my questions and results: - both http://chromium.googlecode.com/svn/trunk/samples/audio/impulse-responses/spatialized5.wav and the link from comment 2, work with latest Aurora (build ID: 20130902004002) on a Mac OSX 10.9 in 32bit mode machine, but don't work with Chrome (please see the attached screenshot) Does anyone have any suggestions/thoughts? Thanks!

Paul, can you please answer Manuela's question in comment 24?

I'm not Paul, but the screenshot from comment 24 shows playback in the quicktime plugin, which isn't what this bug is about. You need to verify that 4-channel wav files can be passed to the web audio API context's decodeAudioData() method. See content/media/webaudio/test/test_decodeMultichannel.html (mochitest) for an example.

Thanks Ralph. Manuela please test using the example provided in comment 26 as a guide.

Nevermind, that would just be repeating what we already have in mochitest. Marking [qa-].