Additional changes to PulseAudio cubeb backend

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

Trunk
mozilla21
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

7 years ago
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.
Assignee

Updated

7 years ago
Blocks: 751030
Assignee

Comment 1

7 years ago
(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.
Assignee

Updated

7 years ago
Depends on: 662417
Assignee

Updated

7 years ago
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Assignee

Comment 2

7 years ago
Posted patch patch v0 (obsolete) — Splinter Review
Tests pass locally, waiting for try results: https://tbpl.mozilla.org/?tree=Try&rev=562da81de3d8
Assignee

Updated

7 years ago
Blocks: 837563
Assignee

Comment 3

7 years ago
Posted 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
Assignee

Comment 4

7 years ago
Posted 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
Assignee

Comment 5

6 years ago
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)

Updated

6 years ago
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: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.