Closed Bug 752401 Opened 8 years ago Closed 8 years ago

Additional changes to PulseAudio cubeb backend

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: kinetik, Assigned: kinetik)

References

Details

Attachments

(1 file, 2 obsolete files)

Following on from bug 751030, there's some additional work to be done.  See the comments from Michael and Arun in that bug for details, but the short version is:

- fix the deadlock caused by mainloop lock vs nsBufferedAudioStream lock ordering in the data_callback vs get_position calls.

- make bad stream state and PA API failures non fatal and signal them via state_callback with CUBEB_STREAM_ERROR.

- change minreq calculation to tlength / 4, and add a comment explaining the voodoo.

- fix the errors in the stream refill callback's calculation of buffer_fill/buffer_time.
Blocks: 751030
(In reply to Matthew Gregan [:kinetik] from comment #0)
> - fix the deadlock caused by mainloop lock vs nsBufferedAudioStream lock
> ordering in the data_callback vs get_position calls.

This one ceases to be a problem with the patch from bug 757707, since that avoids holding the stream lock over calls to cubeb.
Depends on: 662417
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Attached patch patch v0 (obsolete) — Splinter Review
Tests pass locally, waiting for try results: https://tbpl.mozilla.org/?tree=Try&rev=562da81de3d8
Blocks: 837563
Attached patch patch v1 (obsolete) — Splinter Review
Pushed the wrong version.  Second attempt: https://tbpl.mozilla.org/?tree=Try&rev=1eb9ab68d921
Attachment #709549 - Attachment is obsolete: true
Attached patch patch v2Splinter Review
Unregister state callbacks during destruction.  Since the state callbacks were changed to depend on cubeb-allocated userdata (cubeb and cubeb_stream), it's no longer safe for the state callback to run after the userdata has been freed.  Try: https://tbpl.mozilla.org/?tree=Try&rev=967bf9ccda40
Attachment #709563 - Attachment is obsolete: true
Comment on attachment 710018 [details] [diff] [review]
patch v2

This is nice and green.

I've included Randell as a reviewer for the WebRTC signaling tests makefile.  I changed it to match the ALSA/PulseAudio logic in libxul's makefile.
Attachment #710018 - Flags: review?(rjesup)
Attachment #710018 - Flags: review?(chris.double)
Attachment #710018 - Flags: review?(chris.double) → review+
Attachment #710018 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/b721fde69bf0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.