Closed Bug 1539225 Opened 3 years ago Closed 2 years ago

media.cubeb.backend does not work with cubeb remote

Categories

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

66 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: achronop, Assigned: kinetik)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Audioipc client does not allow to set a backend name in audioipc_client init method [1], so it is impossible to choose a specific backend when the remote cubeb is on [2].

[1] https://searchfox.org/mozilla-central/source/media/audioipc/client/src/lib.rs#89
[2] https://searchfox.org/mozilla-central/source/dom/media/CubebUtils.cpp#465

Assignee: nobody → kinetik

The cubeb-rs wrapper doesn't support the backend_name argument (or backend selection at all), so we may also need to add support there too.

The other complication is that the audioipc server is already running in the parent process by the time we call audioipc_client_init in the child process. We could either allow the server to support multiple backends (each child process could request a different backend, retains a closer API mapping to cubeb) or limit the server to a single possible backend (simpler, mostly matches existing global singleton cubeb context behaviour).

Unless there's a good argument to support multiple backends at once, I'll fix this by allowing the server init to take a backend_name and only support one backend at a time. Changing backends would require changing sCubebBackendName and restarting Gecko.

(In reply to Matthew Gregan [:kinetik] from comment #1)

Unless there's a good argument to support multiple backends at once, I'll fix this by allowing the server init to take a backend_name and only support one backend at a time. Changing backends would require changing sCubebBackendName and restarting Gecko.

That's sufficient, thanks. Given that we use that pref mostly for debugging supporting multiple backend just for that would be too much.

I'll implement this soon, since once we land bug 1425788 we'll want the ability to switch the new Rust CoreAudio backend (bug 1530715) off when debugging issues related to the old vs new backend.

Blocks: 1425788

https://github.com/djg/audioipc-2/pull/65 for the AudioIPC side. The Gecko side also needs a small patch in CubebUtils.cpp to pass sCubebBackendName to audioipc_server_start().

The cubeb-rs wrapper doesn't support the backend_name argument (or backend selection at all), so we may also need to add support there too.

FWIW, I was wrong about this. Only cubeb_api::context::init is lacking the backend_name parameter (and AudioIPC doesn't use that API), the rest of the code supports it.

Depends on D38957

Blocks: 1488666
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e3dcc1a1bfcb
Pass cubeb backend name to AudioIPC server during init.  r=chunmin
https://hg.mozilla.org/integration/autoland/rev/d9eed0a39013
Update AudioIPC to pick up backend selection & context name support.  r=chunmin
https://hg.mozilla.org/integration/autoland/rev/790684b8a27c
Vendor Rust.  r=chunmin
Regressions: 1569762

== Change summary for alert #22098 (as of Wed, 24 Jul 2019 06:09:10 GMT) ==

Improvements:

9% ts_paint_webext linux64-shippable-qr opt e10s stylo 297.54 -> 271.33
7% ts_paint linux64-shippable-qr opt e10s stylo 293.58 -> 272.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22098

You need to log in before you can comment on or make changes to this bug.