Open Bug 1425788 Opened 5 years ago Updated 5 months ago

Enable AudioIPC on macOS

Categories

(Core :: Audio/Video: cubeb, enhancement, P2)

All
macOS
enhancement

Tracking

()

REOPENED
98 Branch
Tracking Status
firefox97 --- unaffected
firefox98 --- affected

People

(Reporter: kinetik, Assigned: kinetik)

References

(Blocks 2 open bugs, Regressed 3 open bugs)

Details

(Keywords: perf-alert)

Attachments

(1 file, 3 obsolete files)

This bug tracks enabling libcubeb remoting/AudioIPC support on macOS.
The existing sandboxing on macOS prevents remoting from working as-is because AudioIPC relies on establishing the initial server connection via a Unix domain socket in the temp directory.  Bug 1405877 addresses this by moving AudioIPC to a model where the server connection is established in the chrome process and then passed to the requesting content process via fd passing in Gecko's IPC.
Attachment #8937384 - Flags: review?(dglastonbury)
Attachment #8937384 - Flags: review?(dglastonbury) → review+
Comment on attachment 8937384 [details] [diff] [review]
Enable AudioIPC on macOS.  r?kamidphish

Review of attachment 8937384 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't realize the remoting was basically ready to go on macOS, this is great!

Once this lands, are we good to lock down the sandbox policy for audio on macOS, or would you prefer we wait a bit for this to bake?

::: toolkit/library/rust/gkrust-features.mozbuild
@@ +22,5 @@
>  
>  if CONFIG['MOZ_RUST_SIMD']:
>      gkrust_features += ['simd-accel']
>  
>  # This feature is only supported on Linux and this check needs to

This comment needs to be updated.
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #3)
> I didn't realize the remoting was basically ready to go on macOS, this is
> great!
> 
> Once this lands, are we good to lock down the sandbox policy for audio on
> macOS, or would you prefer we wait a bit for this to bake?

We'd prefer to wait a bit.  The macOS audio backend has much tighter latency (and other) constraints than the Linux/PulseAudio backend, so there's a reasonable chance of issues.  At this stage we're enabling this in nightly to shake it down, there's a chance we'll have to pref it off pretty quickly if there's breakage.

> >  # This feature is only supported on Linux and this check needs to
> 
> This comment needs to be updated.

Good catch, thanks.
Great, thanks for the insights.
Rank: 20
Priority: -- → P2
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bded93a4777e
Enable audio remoting (AudioIPC for cubeb) on macOS.  r=kamidphish
https://hg.mozilla.org/mozilla-central/rev/bded93a4777e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Depends on: 1426791
This has degraded audio quality pretty badly, leaving webrtc calls nearly unusable on OS X.
Per bug 1429847 comment 6, we'll disable remoting while we investigate solutions for the WebRTC issues.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Keywords: leave-open
Attached patch bug1425788_disable.patch (obsolete) — Splinter Review
Attachment #8942806 - Flags: review?(dglastonbury)
Attachment #8942806 - Flags: review?(dglastonbury) → review+
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/89c02d32a35c
Disable AudioIPC on macOS while investigating fallout.  r=kamidphish

The leave-open keyword is there and there is no activity for 6 months.
:kinetik, maybe it's time to close this bug?

Flags: needinfo?(kinetik)

I'm working on having another go at this.

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #14)

The leave-open keyword is there and there is no activity for 6 months.
:kinetik, maybe it's time to close this bug?

There have been several fixes to resolve the issues that resulted in the original backout, so this bug will be landed again. I've cleared leave-open and marked it as ASSIGNED for now.

Status: REOPENED → ASSIGNED
Flags: needinfo?(kinetik)
Keywords: leave-open

I see a number of failures, even with two threads, but I can repro locally so I'll fix soon. Hopefully it's all the same bug.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44433c9e7372e1b5ba6740be0cf23f473a75c061

Depends on: 1539225
Depends on: 1565575
Depends on: 1574284
Depends on: 1570145
Depends on: 1590249
No longer depends on: 1590249
Target Milestone: mozilla59 → 93 Branch
Depends on: 1726275
Target Milestone: 93 Branch → Future
Attachment #8937384 - Attachment is obsolete: true
Attachment #8942806 - Attachment is obsolete: true
Attached file GitHub Pull Request (obsolete) —
Comment on attachment 9253101 [details] [review]
GitHub Pull Request

Oops, intended to attach this to bug 1726275.
Attachment #9253101 - Attachment is obsolete: true
Depends on: 1746689
Depends on: 1747213
Attachment #9255625 - Attachment description: WIP: Bug 1425788 - Enable AudioIPC for macOS. → Bug 1425788 - Enable AudioIPC for macOS. r?chunmin
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/153a98aa1de7
Enable AudioIPC for macOS. r=chunmin
Status: ASSIGNED → RESOLVED
Closed: 5 years ago7 months ago
Resolution: --- → FIXED
Severity: normal → --
Target Milestone: Future → 97 Branch
Regressions: 1747932

== Change summary for alert #32848 (as of Thu, 30 Dec 2021 17:18:31 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
4% webaudio macosx1014-64-shippable-qr webrender 126.17 -> 121.42

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32848

Keywords: perf-alert
Regressions: 1749790
Regressions: 1750282
Regressions: 1750424
Blocks: 1750938

Considering this feature was disable, based on bug 1750938, shoudn't we reopen this bug and mark it affected on Fx 97 and Fx 98?

Flags: needinfo?(kinetik)
Status: RESOLVED → REOPENED
Flags: needinfo?(kinetik)
Resolution: FIXED → ---
Target Milestone: 97 Branch → 98 Branch
OS: Unspecified → macOS
Hardware: Unspecified → All

The continued efforts on this are appreciated. It's important for our sandbox on Mac because it makes us need to allow microphone access in content processes. When the remoting pref is turned on, the sandbox rules don't allow access to the microphone and other audio services. In case it changes the strategy with the implementation, the microphone is the most important restriction and removing access to it is the main benefit driving this with respect to security.

Severity: -- → N/A
You need to log in before you can comment on or make changes to this bug.