752401 - Additional changes to PulseAudio cubeb backend

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

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.

Comment 1

[Matthew Gregan [:kinetik]]

(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.

Comment 2

[Matthew Gregan [:kinetik]]

Attachment: patch v0

Tests pass locally, waiting for try results: https://tbpl.mozilla.org/?tree=Try&rev=562da81de3d8

Comment 3

[Matthew Gregan [:kinetik]]

Attachment: patch v1

Pushed the wrong version.  Second attempt: https://tbpl.mozilla.org/?tree=Try&rev=1eb9ab68d921

Comment 4

[Matthew Gregan [:kinetik]]

Attachment: patch v2

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

Comment 5

[Matthew Gregan [:kinetik]]

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.

Comment 6

[Matthew Gregan [:kinetik]]

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

Comment 7

[Ed Morley [:emorley]]

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