Use MacOS's audio processing algorithm
Categories
(Core :: Audio/Video: cubeb, task, P2)
Tracking
()
People
(Reporter: padenot, Assigned: pehrsons)
References
(Blocks 2 open bugs)
Details
Attachments
(37 files, 2 obsolete files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
Bug 1404972 - Wire up NativeInputTrack to the AudioDataListenerInterface processing APIs. r?padenot!
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
| Reporter | ||
Updated•8 years ago
|
| Reporter | ||
Updated•8 years ago
|
Updated•3 years ago
|
| Assignee | ||
Updated•2 years ago
|
| Assignee | ||
Comment 1•2 years ago
|
||
| Assignee | ||
Updated•2 years ago
|
| Assignee | ||
Updated•2 years ago
|
Comment 3•2 years ago
|
||
| bugherder | ||
| Assignee | ||
Comment 4•2 years ago
|
||
| Assignee | ||
Comment 5•2 years ago
|
||
| Assignee | ||
Comment 6•2 years ago
|
||
| Assignee | ||
Comment 7•2 years ago
|
||
It is never read from so can live on the stack instead.
| Assignee | ||
Comment 8•2 years ago
|
||
| Assignee | ||
Comment 9•2 years ago
|
||
To allow for applying processing params between Init and Start.
This also captures this in addition to self in AudioInputSource lambdas, for
better readability.
| Assignee | ||
Comment 10•2 years ago
|
||
| Assignee | ||
Comment 11•2 years ago
|
||
| Assignee | ||
Comment 12•2 years ago
|
||
| Assignee | ||
Comment 13•2 years ago
|
||
| Assignee | ||
Comment 14•2 years ago
|
||
| Assignee | ||
Comment 15•2 years ago
|
||
| Assignee | ||
Comment 16•2 years ago
|
||
| Assignee | ||
Comment 17•2 years ago
|
||
This changes the API on AudioInputProcessing from the transparent
ApplyConfig/SetPassThrough, to the opaque SetDesiredConfig, which will handle
applying the APM config and enabling passthrough mode as appropriate.
This allows the next patch to factor applied processing params into the final
config that gets applied to the APM.
| Assignee | ||
Comment 18•2 years ago
|
||
This patch makes the mic source request all platform processing algorithms that
have been enabled by constraints. It will enable the APM software processing
just like before, until it receives a notification that platform process was
applied. When this happens the corresponding APM algorithms are turned off.
Should the requested platform processing params change from non-None to another
non-None variant, and applying the new set of params fails, all the APM
algorithms requested by constraints will be enabled, and the requested platform
processing params set to None. This disables platform processing and falls back
to the APM. Should the requested platform processing params change to another
non-None variant again, another attempt to apply these in the platform will be
made.
| Assignee | ||
Comment 19•2 years ago
|
||
| Assignee | ||
Comment 20•2 years ago
|
||
| Assignee | ||
Comment 21•2 years ago
|
||
| Assignee | ||
Comment 22•2 years ago
|
||
Its presilence check was inaccurate and started failing intermittently.
| Assignee | ||
Comment 23•2 years ago
|
||
It was slow.
| Assignee | ||
Comment 24•2 years ago
|
||
This is needed for using Result with gtest matchers.
| Assignee | ||
Comment 25•2 years ago
|
||
| Assignee | ||
Comment 26•2 years ago
|
||
| Assignee | ||
Comment 27•2 years ago
|
||
| Assignee | ||
Comment 28•2 years ago
|
||
| Assignee | ||
Comment 29•2 years ago
|
||
| Assignee | ||
Comment 30•2 years ago
|
||
| Assignee | ||
Comment 31•2 years ago
|
||
| Assignee | ||
Comment 32•2 years ago
|
||
This can help debugging.
| Assignee | ||
Comment 33•2 years ago
|
||
| Assignee | ||
Comment 34•2 years ago
|
||
| Assignee | ||
Comment 35•2 years ago
|
||
| Assignee | ||
Comment 36•2 years ago
|
||
| Assignee | ||
Comment 37•2 years ago
|
||
| Assignee | ||
Comment 38•2 years ago
|
||
This avoids some races involving MockCubebStream in manual mode, for instance
when a data callback races with cubeb_stream_stop.
| Assignee | ||
Comment 39•2 years ago
|
||
| Assignee | ||
Comment 40•2 years ago
|
||
Since https://hg.mozilla.org/mozilla-central/rev/c06840f719aa6b4f01b57abf9709b04b68444124
mPacketizerInput is only created when needed, so no assumptions can be made on
its existence.
Updated•2 years ago
|
Updated•2 years ago
|
| Assignee | ||
Comment 41•2 years ago
|
||
| Assignee | ||
Updated•2 years ago
|
Comment 42•2 years ago
|
||
Comment 43•2 years ago
|
||
Backed out for causing cppunit assertion failures in mozilla/Result.h.
- Backout link
- Push with failures
- Failure Log
- Failure line: Assertion failure: isErr(), at /builds/worker/workspace/obj-build/dist/include/mozilla/Result.h:582
| Assignee | ||
Updated•2 years ago
|
Comment 44•2 years ago
|
||
Comment 45•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/17d682bf877f
https://hg.mozilla.org/mozilla-central/rev/cf2b33b90431
https://hg.mozilla.org/mozilla-central/rev/b4d6f36516a1
https://hg.mozilla.org/mozilla-central/rev/19edbc37fc5f
https://hg.mozilla.org/mozilla-central/rev/0556bca66630
https://hg.mozilla.org/mozilla-central/rev/e8e5b74a4ee9
https://hg.mozilla.org/mozilla-central/rev/1cdbcf4eb4ac
https://hg.mozilla.org/mozilla-central/rev/4aacf15793ac
https://hg.mozilla.org/mozilla-central/rev/40d06a3fc080
https://hg.mozilla.org/mozilla-central/rev/6b9a71060950
https://hg.mozilla.org/mozilla-central/rev/f731c7b63f8e
https://hg.mozilla.org/mozilla-central/rev/7a788e264d94
https://hg.mozilla.org/mozilla-central/rev/b5fc48d87709
https://hg.mozilla.org/mozilla-central/rev/db45374981f0
https://hg.mozilla.org/mozilla-central/rev/7d32bf064229
https://hg.mozilla.org/mozilla-central/rev/2c3e5a943d30
https://hg.mozilla.org/mozilla-central/rev/4bd2675a4b11
https://hg.mozilla.org/mozilla-central/rev/c84e4e317194
https://hg.mozilla.org/mozilla-central/rev/93e17860ad7a
https://hg.mozilla.org/mozilla-central/rev/ded33b5c0b43
https://hg.mozilla.org/mozilla-central/rev/c339a79a1448
https://hg.mozilla.org/mozilla-central/rev/d4c5c6c41129
https://hg.mozilla.org/mozilla-central/rev/8b1861c0e6b5
https://hg.mozilla.org/mozilla-central/rev/a3fcf8a86071
https://hg.mozilla.org/mozilla-central/rev/79dfe47bfa37
https://hg.mozilla.org/mozilla-central/rev/4d727a37e18c
https://hg.mozilla.org/mozilla-central/rev/d81d48c159b4
https://hg.mozilla.org/mozilla-central/rev/0a9e175e58d4
https://hg.mozilla.org/mozilla-central/rev/f7aa4e868b0a
https://hg.mozilla.org/mozilla-central/rev/fd933da7403d
https://hg.mozilla.org/mozilla-central/rev/4111269589f3
https://hg.mozilla.org/mozilla-central/rev/f5155e405de0
https://hg.mozilla.org/mozilla-central/rev/0f2e7708c8ea
https://hg.mozilla.org/mozilla-central/rev/91aa189a5e62
https://hg.mozilla.org/mozilla-central/rev/fedac0c73475
Comment 46•2 years ago
|
||
Andreas, this is a very old issue and we are in our last week of nightly, doesn't it feel too risky for the release? Thanks
| Assignee | ||
Comment 47•2 years ago
|
||
I disagree, and I'll motivate why.
Bug 1670633 was risky because we changed to a new type of audio unit for mic capture on macOS. It landed early in 122 and there have been a number of fallout bugs due to issues in macOS platform libraries. These are mostly under control, but one class of them that remain is audio quality issues, like bug 1890426, because we run this audio unit in "bypass" mode which most other native apps, including Safari, does not. This bug integrates our gecko processing code so we can finally stop using "bypass" mode and use the audio unit the way it was intended ("bypass" has some issues suggesting it is not well tested by Apple).
This gecko integration is well tested with gtests, and as mentioned for now only affects macOS.
Overall I think it's fairly low risk and brings improvements we don't want to wait another cycle for. And if we need to disable all this it's a simple pref flip to uplift into beta.
Comment 48•2 years ago
|
||
OK thanks
| Assignee | ||
Comment 49•2 years ago
•
|
||
Note we found the platform processing with our config leaves a residual echo for some devices, so I'm flipping the pref to disable it in bug 1895787. Hopefully we can improve this and re-enable in 128.
Comment 50•2 years ago
|
||
Andreas, would this be good to have in a release note for Nightly (for 127 or 128)? If you think so, could you nominate it? Thanks!
https://wiki.mozilla.org/Release_Management/Release_Notes_Nomination
| Assignee | ||
Comment 51•2 years ago
|
||
For now it's been disabled. If we manage to ship this broadly, I'll nominate. Thanks for the reminder.
| Assignee | ||
Comment 52•2 years ago
|
||
Note that bug 1895787 disabled this in 127 but has enabled it again in 128. This request is for 128.
Release Note Request (optional, but appreciated)
[Why is this notable]: For microphone input with audio processing (echo cancellation, noise suppression, automatic gain control) we switch from using a software solution from libwebrtc that cancels echo only from the audio output of the current tab; to using Apple's voice processing that ships with macOS. This knows more about the user's system so can do a better job, for instance it can:
- do system-wide echo cancellation
- use microphone arrays for better quality where applicable, for instance for builtin mics
- and probably more. It's a bit of a black box. Generally people who I have heard from seem to like it better than the libwebrtc processing we used before.
[Suggested wording]: Microphone capture through getUserMedia on macOS will use system-provided voice processing when applicable.
[Links (documentation, blog post, etc)]: Nothing specific, but docs on getUserMedia: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia, there are no user friendly docs on Apple's voice processing.
Description
•