938450 - decodeAudioData skips frames when resampling

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Karl Tomlinson (:karlt)]

Adding MOZ_ASSERT(inSamples == audioData->mFrames); at
http://hg.mozilla.org/mozilla-central/annotate/7b014f0f3b03/content/media/webaudio/MediaBufferDecoder.cpp#l365
fails when resampling between 48000 and 44100, which happens during test_mediaDecoding.html

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: process all input samples in decodeAudioData

Comment 2

[Karl Tomlinson (:karlt)]

Applies on top of patches in bug 937057.  This of course makes test_mediaDecoding.html fail because it is testing that our resampling code doesn't change.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: test lengths of decoded audio and change 48k wav files to 48k

With this patch, the 48kHz tests fail comparisons with the expected files when
run in a 44.1kHz context and the 44.1kHz tests fail these comparisons when run
in a 48kHz context.

Before this patch, all tests failed these comparisons when run in a 48kHz
context.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: rename compareBuffers to compareChannels & add compareBuffers

I want to use compareBuffers in fixing test_mediaDecoding.html.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: reduce the number of offset parameters to compareChannels()

While I'm here.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: load the expected file only once and remove unused monoTest parameter

Some refactoring of test_mediaDecoding.html to make fixing it easier.

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: use an OfflineAudioContext to test decodeAudioData at different sample rates

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: test resampling by resampling back to original sample rate

Comment 9

[Paul Adenot (:padenot)]

Comment on attachment 8340936 [details] [diff] [review]
use an OfflineAudioContext to test decodeAudioData at different sample rates

Review of attachment 8340936 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/test/test_mediaDecoding.html
@@ +245,5 @@
>  }
>  
>  function runTest(test, response, callback) {
>    var expectCallback = false;
> +  var cx = new OfflineAudioContext(1, 1, test.sampleRate);

I would expect this to work, yes.

Comment 10

[(no longer active)]

Comment on attachment 8340934 [details] [diff] [review]
reduce the number of offset parameters to compareChannels()

Review of attachment 8340934 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/test/test_scriptProcessorNode.html
@@ +94,5 @@
>        is(e.outputBuffer.numberOfChannels, 2, "Correct number of channels for the output buffer");
>        is(e.outputBuffer.length, 2048, "Correct length for the output buffer");
>        is(e.outputBuffer.sampleRate, context.sampleRate, "Correct sample rate for the output buffer");
>  
>        var firstNonZero = findFirstNonZeroSample(e.inputBuffer);

Please add a test to make sure firstNonZero < 2048 for sanity checking.

Comment 11

[(no longer active)]

Comment on attachment 8340932 [details] [diff] [review]
rename compareBuffers to compareChannels & add compareBuffers

Review of attachment 8340932 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 12

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ba0897b47f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a57fd3a56a9
https://hg.mozilla.org/integration/mozilla-inbound/rev/780a0671719b
https://hg.mozilla.org/integration/mozilla-inbound/rev/91d16fd9fffc
https://hg.mozilla.org/integration/mozilla-inbound/rev/00c412574447
https://hg.mozilla.org/integration/mozilla-inbound/rev/b962d48690b3

Haven't landed the resampling test in attachment 8340937 [details] [diff] [review] because that depends on bug 937057.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9ba0897b47f9
https://hg.mozilla.org/mozilla-central/rev/8a57fd3a56a9
https://hg.mozilla.org/mozilla-central/rev/780a0671719b
https://hg.mozilla.org/mozilla-central/rev/91d16fd9fffc
https://hg.mozilla.org/mozilla-central/rev/00c412574447
https://hg.mozilla.org/mozilla-central/rev/b962d48690b3

Comment 14

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3296ae716782

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3296ae716782