Closed Bug 793274 Opened 12 years ago Closed 12 years ago

Underrun at the very beginning of the playback when using cubeb

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(1 file, 2 obsolete files)

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
Attached patch Broken patch (obsolete) (deleted) — Splinter Review
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.
Assignee: nobody → paul
Status: NEW → ASSIGNED
Flags: needinfo?(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.
Flags: needinfo?(kinetik)
Do you fill like doing it, or should I write a patch?
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.
I'll give the idea I proposed in my last paragraph a shot.
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
Attachment #685132 - Flags: review?(kinetik)
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 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.
Attachment #685132 - Flags: review?(kinetik)
Attachment #685132 - Attachment is obsolete: true
Attachment #681236 - Attachment is obsolete: true
Attachment #696035 - Flags: review?(kinetik) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: