913854 - AudioBufferSourceNodes scheduled in a sequence aren't seamless when buffer sample rate doesn't match context

Status: VERIFIED FIXED

(Core :: Web Audio, defect, P1)

Description

[Jukka Jylänki]

User Agent: Opera/9.80 (Windows NT 6.1; WOW64) Presto/2.12.388 Version/12.16

Steps to reproduce:

Press 'Start Audio' on this page: https://dl.dropboxusercontent.com/u/40949268/Bugs/beep.html


Actual results:

Web Audio API playback produces a constant snapping sound along the output tone.


Expected results:

The playback should be error-free, like in the Youtube video linked in the page.

Comment 1

[Jukka Jylänki]

Tested on Firefox Nightly 26.0a1 (2013-08-21).

Comment 2

[Paul Silaghi, QA [:pauly]]

Confirmed 27.0a1 (2013-10-06)

Comment 4

[Karl Tomlinson (:karlt)]

48 kHz plays fine with large enough sample frame size, which is how far this testcase buffers in advance.

The suggestion in Bug 886381 comment 2 would improve things for other frame rates.
Note that bug 886653 has changed things a bit from what was described in that comment.

Comment 6

[Karl Tomlinson (:karlt)]

Keeping this separate from Bug 886381 because the examples there are using requestAnimationFrame.

Comment 7

[(no longer active)]

If you add a printf statement here <http://dxr.mozilla.org/mozilla-central/source/content/media/AudioStream.cpp#l887> to print underrunFrames, do you see underruns happening in this test case?

Comment 8

[Karl Tomlinson (:karlt)]

There are occasionally underruns (on Linux debug build), but very much less frequently than the snaps between buffers.

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: experimental patch - extend.buffer.input

Changes in bug 937057 will hopefully mean that buffers chopped at zero crossings should be close to seamless.

This patch won't apply without some work because this was on top of a patch for bug 937057 which has now changed.

With some tweaks to (real) time to (integer) sample number rounding, this sometimes played seamlessly for buffers chopped at non-zero points when the buffer sample rate was half the context sample rate, suggesting that with some more careful rounding we could behave similarly to chromium.  I'm not sure that is a valuable goal though.

For buffer sample rates that are not an integer fraction of the context sample rate, things are more complex and chromium doesn't produce seamless results.

A fundamental problem is that the buffer start time is rounded to the nearest sample and the duration is then also rounded to the nearest sample.  The end point is therefore rounded twice, which is inconsistent with the start time of the next buffer.  That means that buffers can overlap by one sample or have a missing sample between them.

To deal with that perfectly in the implementation, we'd need to be able to start play back at an intersample time, and the speex resampler doesn't bother to provide such an option.  Perhaps it would be easier to make things sound better if buffers overlap a little, each having extra samples at the end, and AudioBufferSourceNode.stop(when) is used to stop the buffer playback exactly when the next buffer starts.  We may also need to prefill the resampler, as in this patch, so as not to interpolate from zero when starting each new buffer.  That may work well enough without needing extra samples at the beginning for interpolation and an offset in the start() call.

However AudioBufferSourceNode.stop(when) is not precise when resampling and that is a bug we should fix.

Comment 10

[Karl Tomlinson (:karlt)]

Jukka, if this is important then please speak up and we can adjust priorities.

We can add a method to the Speex resampler to set samp_frac_num for a specific
channel index, so that the start time can be used to ensure that sub-sample
start times of buffer playback align with the ends of the previous buffers.
This requires passing the double precision start time through to the audio
thread.  

The Speex resampler is a linear function, so adding the appropriately aligned
resampled output of subsequent buffers can give seamless results.  We need to
ensure that the resampled output of the buffers overlaps sufficiently to cover
the effects of the resampler's filter.  We would no longer call
speex_resampler_skip_zeros, which actually chops off some initial non-zeros in
the output, but instead start the resampler early using
speex_resampler_get_input_latency and setting samp_frac_num appropriately.
We also need to feed 2 * input_latency - 1 samples into the resampler at the
end to extract all the trailing effects of the filter.

Comment 11

[Jukka Jylänki]

This is still a problem, more so after I understood that the audio context sampling frequency was not hardcoded in both Chrome and Firefox to 48kHz, but can change between systems, which means that it's not possible to author audio data in that one known working sampling frequency, but if the user happens to get bad sampling frequency, then he'll get snapping audio.

To recap: In Emscripten streamed audio synthesis is used in the SDL and OpenAL backends. OpenAL allows playing back variable-length clips as indidivual audio files (and also allows streaming), but the whole SDL API is based on streaming fixed-length audio data, i.e. SDL soft-mixes its own audio data on the CPU. This issue therefore hits any port of C/C++ projects that use SDL as their audio backend, or projects that use OpenAL and stream audio through that. OpenAL projects playing single audio clips don't give snaps. 

The two Emscripten projects I know of in the wild that use SDL are HTML5ScummVM: http://clb.demon.fi/html5scummvm/ and JSMESS: http://jsmess.textfiles.com . Both exhibit audio issues due to this bug. The JSMESS people are probably the ones currently battling hardest through this issue, to the point that they expressed their wish that Mozilla Audio Data API were still supported, since they report it is working much better than Web Audio API.

The above comment 10 makes it sound like the snapping issue could be resolved by smarter logic in internal resampler? (i.e "just" an internal code change) I think the big worry was whether that really is the case, or if the fix needs to go all the way to the Web Audio spec to make sense.

So the P1 question here is probably that - does something need to be changed in the Web Audio spec itself to make sure that this streaming audio use case is covered?

Comment 12

[Karl Tomlinson (:karlt)]

I should be able to make this work without a spec change.

Comment 13

[(no longer active)]

(In reply to comment #12)
> I should be able to make this work without a spec change.

That would be awesome!!

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: add speex_resampler_set_skip_frac_num

This allows a client to align output samples consistently for independent
resampling of contiguous input buffers.

A method to explicitly set the fractional offset makes it clearer that this
doesn't affect sub-sample alignment at any precision tighter than provided by
ratio_den (or ratio_num, if considering output samples), and it suits my needs nicely.  If this is depending too much on the resampler implementation, then perhaps I could make the method take a floating point argument in [0,1], or in [0, 1/in_rate], with the resampler selecting the nearest subsample.

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: part 2: capture the full extent of the resampling filter

The resampling filter means that the buffer influences a greater number of
samples than indicated by just its length.  Including the full influence of
the linear filter means that adjacent buffers aligned appropriately will
behave as if they were one extended buffer.

The buffers are not yet aligned more carefully than track ticks, so buffers
play back seamlessly only if their sample rates and lengths are such that
their duration is an integer number of track ticks.

Knowing how far the filter extends before the start time requires
initializing the resampler before buffer processing.

The patch also includes the input latency in the first resampler input
buffer sample count estimate to reduce the number to calls required
to start the resampler.

See documentation of speex_resampler_set_skip_frac_num in attachment 8377308 [details] [diff] [review] to explain some of the math.

Comment 16

[Karl Tomlinson (:karlt)]

Attachment: part 3: pass start time as double parameter and subsample align

The subsample alignment of resampled buffers provides seamless playback even
when buffer durations are not an integer number of track ticks.

Comment 17

[Karl Tomlinson (:karlt)]

Attachment: part 2 v1.1: capture the full extent of the resampling filter

Restoring some previous behavior:
Even simple values of playbackRate may be -ve, and so need sanitizing.

Comment 18

[Jean-Marc Valin]

Comment on attachment 8377308 [details] [diff] [review]
add speex_resampler_set_skip_frac_num

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

r=me

Comment 19

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/441506ef7062
https://hg.mozilla.org/integration/mozilla-inbound/rev/36e1f6623a6d
https://hg.mozilla.org/integration/mozilla-inbound/rev/78fc4cbf92f8

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/441506ef7062
https://hg.mozilla.org/mozilla-central/rev/36e1f6623a6d
https://hg.mozilla.org/mozilla-central/rev/78fc4cbf92f8

Comment 21

[Paul Silaghi, QA [:pauly]]

The initial problem is gone, but:
Press Start, Stop, then Start again - I hear a short snapping sound.
Thoughts ?

Comment 22

[Karl Tomlinson (:karlt)]

(In reply to Paul Silaghi, QA [:pauly] from comment #21)
> The initial problem is gone, but:
> Press Start, Stop, then Start again - I hear a short snapping sound.
> Thoughts ?

Yes, thanks.  A snapping sound is expected on stop (because the testcase cuts the sound abruptly.  A snapping sound is also expected on subsequent starts because the phase does not begin at zero.  There is also an expected glitch one block (fraction of a second) into the start of the signal because the first block runs a little out of sync with the other blocks.

Comment 23

[Paul Silaghi, QA [:pauly]]

Verified fixed on 30.0a1 (2014-03-03) Win 7 x64 based on comment 22.

Comment 24

[Karl Tomlinson (:karlt)]

Attachment: mochitest

Comment 25

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=be13182ca19c

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9712ddaa26a

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d9712ddaa26a