793274 - Underrun at the very beginning of the playback when using cubeb

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

STR:
- Open http://gr.ema.lv/andris/_soundTest_/audio/1.ogg (a short sine)
- Click play on the built-in controls

Expected:
- The sound plays fine, from the beginning to the end.

Actual result:
- There is almost always an underrun a the beginning (~25% of the time, it plays fine).
- If I disable cubeb, it works fine.

Also, I put a printf in the data callback (|printf("underrun: %d\n", bytesWanted / mBytesPerFrame);| at [1]), and I see that it is indeed an underrun.

It works fine with a file with a greater duration.

[1]: http://mxr.mozilla.org/mozilla-central/source/content/media/nsAudioStream.cpp#1223

Comment 1

[Paul Adenot (:padenot)]

Attachment: Broken patch

Matthew, the problem here is that we are starting the cubeb thread too early, when not enough frames have been written to the nsBufferedAudioStream circular buffer.

This broken patch fixes the behavior reported in comment 0, but the fix will not work for super tiny files (that have a duration lesser than |INITIAL_BUFFER_FILL|). This patch is mainly to show the problem, really.

We could really fix this by adding an method to the nsAudioStream, to make the audio loop explicitly start the cubeb stream when a certain amount of frames have been written to the stream, or if the queue is finished. Or maybe some other thing.

Comment 2

[Matthew Gregan [:kinetik]]

Good catch.  libcubeb tries to prefill buffers, but with the current design this happens at stream initialization time and since the BufferedAudioStream is always empty at this point, only serves to prefill with silence.

I think we should disable the prefill on stream init, and add a method to prefill at the appropriate time, and have that signal when it's safe to start.

Comment 3

[Paul Adenot (:padenot)]

Do you fill like doing it, or should I write a patch?

Comment 4

[Matthew Gregan [:kinetik]]

I'm happy to take care of it, but I won't get to it for a little while.  Maybe the approach suggested in your last paragraph is the way to go.

Comment 5

[Paul Adenot (:padenot)]

I'll give the idea I proposed in my last paragraph a shot.

Comment 6

[Paul Adenot (:padenot)]

Attachment: Make sure to have enough frames pushed to the AudioStream before starting it. r=

This implements comment 2 idea, and fixes the problem. I've chosen the prefill
value empirically, a lower value shows a tiny tiny underrun on my machine, and a
higher value induces latency, so I guess this is more or less OK.

Seems green on try: https://tbpl.mozilla.org/?tree=Try&rev=fa140696c827

Comment 7

[Paul Adenot (:padenot)]

Ha, it seems to cause problem on Mac, but I don't have a Mac handy and can't reproduce on other platforms. I've ordered one but maybe the problem is obvious to you, since it's probably a subtle difference in cubeb between windows/linux and mac.

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 685132 [details] [diff] [review]
Make sure to have enough frames pushed to the AudioStream before starting it. r=

Alright, does not work too well on Windows as well. I'm cancelling the review for now.

Comment 9

[Paul Adenot (:padenot)]

Attachment: Make sure to have enough frames pushed to the AudioStream before starting it. r=

This is now green on try: https://tbpl.mozilla.org/?tree=Try&rev=4956e51b7340

Comment 11

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1abf4c88f8f1

Comment 12

[Ed Morley [:emorley]]

Backed out on suspicion of causing permaorange win pgo-only mochitest-1 timeouts:
https://tbpl.mozilla.org/php/getParsedLog.php?id=18841611&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=18840712&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=18846633&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=18847209&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=18850103&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=18850099&tree=Mozilla-Inbound

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

Comment 13

[Paul Adenot (:padenot)]

After bisecting the queue on try, this patch has been proved to cause no problem [0], note the patch at the top of the queue that requests a PGO build. The problem remains in the other part of the queue [1].

This has been pushed as https://hg.mozilla.org/integration/mozilla-inbound/rev/4317e42ac807

[0]: https://tbpl.mozilla.org/?tree=Try&rev=516ddf4c963c
[1]: https://tbpl.mozilla.org/?tree=Try&rev=418c581b8023

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/4317e42ac807