Closed Bug 1404972 Opened 7 years ago Closed 3 months ago

Use MacOS's audio processing algorithm

Categories

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

Unspecified
macOS
task

Tracking

()

RESOLVED FIXED
127 Branch
Tracking Status
relnote-firefox --- 128+
firefox127 --- disabled
firefox128 --- fixed

People

(Reporter: padenot, Assigned: pehrsons)

References

(Blocks 3 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
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
WebKit is reportedly only using this in Safari for their audio input, and WebKit developers seem very happy about the result.
Rank: 30
Priority: P3 → P4
Severity: normal → S3
See Also: → 1839885
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Type: enhancement → task
Rank: 30
Priority: P4 → P2
Summary: Investigate using OSX's AEC and other algorithms. → Use MacOS's audio processing algorithm
Depends on: 1874909
OS: Unspecified → macOS
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/696309dcffc6
Update cubeb-coreaudio-rs to a265c67d34. r=cubeb-reviewers,chunmin

It is never read from so can live on the stack instead.

To allow for applying processing params between Init and Start.

This also captures this in addition to self in AudioInputSource lambdas, for
better readability.

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.

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.

Its presilence check was inaccurate and started failing intermittently.

This is needed for using Result with gtest matchers.

This avoids some races involving MockCubebStream in manual mode, for instance
when a data callback races with cubeb_stream_stop.

Attachment #9393657 - Attachment is obsolete: true
Attachment #9393667 - Attachment is obsolete: true
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ed2fdc7b602d
Detect when SmartMockCubebStream::ManualDataCallback races against starting or stopping the stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0128d95e64ef
Avoid races in MockCubebStream with a Mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c3538c05fb0c
In manual MockCubeb tests wait for the stream to start before calling callbacks. r=padenot
https://hg.mozilla.org/integration/autoland/rev/8cf8b6f1a41e
Convert MockGraphInterface to modern MOCK_METHOD macros. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ea7779289030
Rename audio input prefs to be more consistent. r=padenot
https://hg.mozilla.org/integration/autoland/rev/08c9467086bf
Make AudioDataListenerInterface::RequestedInputChannelCount const. r=padenot
https://hg.mozilla.org/integration/autoland/rev/23d0f7682a5f
Split AudioInputSource::Start into Init and Start. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9595c4451fc1
Simplify per-device getters in MediaTrackGraphImpl::CheckDriver. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bd41873e1f03
Enable bitwise operations on cubeb_input_processing_params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c6462cae6c8b
Add APIs to AudioDataListenerInterface to signal requested and applied input processing params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0824e75bd26c
Wire up NonNativeInputTrack to the AudioDataListenerInterface processing APIs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/152a456a7af1
To Result add operator==. r=glandium
https://hg.mozilla.org/integration/autoland/rev/63765ab9e647
Wire up NativeInputTrack to the AudioDataListenerInterface processing APIs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/2e038afae950
Remove unused AudioCallbackDriver::OnThreadIdChanged. r=padenot
https://hg.mozilla.org/integration/autoland/rev/de46ffac7541
Add a pref for platform audio processing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6f3aebf42327
In MediaEngineWebRTCMicrophoneSource handle platform processing params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9de637c307ad
Make MockCubeb able to handle 0-frame iterations (and being told to drain after one). r=padenot
https://hg.mozilla.org/integration/autoland/rev/4a148ddfc1cd
Default AudioInputProcessing statistics string to the empty string, for when there are no statistics. r=padenot
https://hg.mozilla.org/integration/autoland/rev/20a15e46829b
Handle 0-frame iterations in AudioInputProcessing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/57e38c2622bf
Switch TestAudioTrackGraph::ReConnectDeviceInput to RunningMode::Manual. r=padenot
https://hg.mozilla.org/integration/autoland/rev/48832f645357
Switch TestAudioTrackGraph::AudioProcessingTrackDisabling to RunnningMode::Manual. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9be7b1767e46
Test input processing handling in AudioInputSource. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bc211f44281f
Fix typo in CubebInputStream.cpp comment. r=padenot
https://hg.mozilla.org/integration/autoland/rev/346a34408207
Unit test AudioCallbackDriver's input processing handling. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a63110e6bf55
From TestAudioCallbackDriver remove unused includes. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ab73b50399ef
In TestAudioCallbackDriver.cpp use MOZ_CAN_RUN_SCRIPT_BOUNDARY to not propagate the annotation upwards. r=padenot
https://hg.mozilla.org/integration/autoland/rev/257f310d5c8e
In TestAudioCallbackDriver.cpp construct threadInDriverIteration atomics with initializer lists. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3143c92d0e2e
In TestAudioInputSource.cpp don't try to scope macros by namespace. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f1cd90f5d1b6
Log when the fallback driver has stopped. r=padenot
https://hg.mozilla.org/integration/autoland/rev/15136efd0662
In TestAudioTrackGraph use gmock to mock AudioDataListeners. r=padenot
https://hg.mozilla.org/integration/autoland/rev/dacf1411a3d9
Add a platform processing gtest to TestAudioTrackGraph. r=padenot
https://hg.mozilla.org/integration/autoland/rev/02b7f8ee802c
Make ResetAudioProcessing idempotent wrt mPacketizerInput. r=karlt,padenot
https://hg.mozilla.org/integration/autoland/rev/abcec791bf5f
Add platform processing unit tests to TestAudioInputProcessing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/43134f8cd283
Update libcubeb to 74d6b05465. r=cubeb-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/703bcdb5aed1
Fix a type of race where AudioCallbackDriver::Start() inits the cubeb stream before tests have hooked up the StreamInitEvent. r=karlt

Backed out for causing cppunit assertion failures in mozilla/Result.h.

Flags: needinfo?(apehrson)
Flags: needinfo?(apehrson)
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/17d682bf877f
Detect when SmartMockCubebStream::ManualDataCallback races against starting or stopping the stream. r=padenot
https://hg.mozilla.org/integration/autoland/rev/cf2b33b90431
Avoid races in MockCubebStream with a Mutex. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b4d6f36516a1
In manual MockCubeb tests wait for the stream to start before calling callbacks. r=padenot
https://hg.mozilla.org/integration/autoland/rev/19edbc37fc5f
Convert MockGraphInterface to modern MOCK_METHOD macros. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0556bca66630
Rename audio input prefs to be more consistent. r=padenot
https://hg.mozilla.org/integration/autoland/rev/e8e5b74a4ee9
Make AudioDataListenerInterface::RequestedInputChannelCount const. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1cdbcf4eb4ac
Split AudioInputSource::Start into Init and Start. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4aacf15793ac
Simplify per-device getters in MediaTrackGraphImpl::CheckDriver. r=padenot
https://hg.mozilla.org/integration/autoland/rev/40d06a3fc080
Enable bitwise operations on cubeb_input_processing_params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6b9a71060950
Add APIs to AudioDataListenerInterface to signal requested and applied input processing params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f731c7b63f8e
Wire up NonNativeInputTrack to the AudioDataListenerInterface processing APIs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7a788e264d94
To Result add operator==. r=glandium
https://hg.mozilla.org/integration/autoland/rev/b5fc48d87709
Wire up NativeInputTrack to the AudioDataListenerInterface processing APIs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/db45374981f0
Remove unused AudioCallbackDriver::OnThreadIdChanged. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7d32bf064229
Add a pref for platform audio processing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/2c3e5a943d30
In MediaEngineWebRTCMicrophoneSource handle platform processing params. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4bd2675a4b11
Make MockCubeb able to handle 0-frame iterations (and being told to drain after one). r=padenot
https://hg.mozilla.org/integration/autoland/rev/c84e4e317194
Default AudioInputProcessing statistics string to the empty string, for when there are no statistics. r=padenot
https://hg.mozilla.org/integration/autoland/rev/93e17860ad7a
Handle 0-frame iterations in AudioInputProcessing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ded33b5c0b43
Switch TestAudioTrackGraph::ReConnectDeviceInput to RunningMode::Manual. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c339a79a1448
Switch TestAudioTrackGraph::AudioProcessingTrackDisabling to RunnningMode::Manual. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d4c5c6c41129
Test input processing handling in AudioInputSource. r=padenot
https://hg.mozilla.org/integration/autoland/rev/8b1861c0e6b5
Fix typo in CubebInputStream.cpp comment. r=padenot
https://hg.mozilla.org/integration/autoland/rev/a3fcf8a86071
Unit test AudioCallbackDriver's input processing handling. r=padenot
https://hg.mozilla.org/integration/autoland/rev/79dfe47bfa37
From TestAudioCallbackDriver remove unused includes. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4d727a37e18c
In TestAudioCallbackDriver.cpp use MOZ_CAN_RUN_SCRIPT_BOUNDARY to not propagate the annotation upwards. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d81d48c159b4
In TestAudioCallbackDriver.cpp construct threadInDriverIteration atomics with initializer lists. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0a9e175e58d4
In TestAudioInputSource.cpp don't try to scope macros by namespace. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f7aa4e868b0a
Log when the fallback driver has stopped. r=padenot
https://hg.mozilla.org/integration/autoland/rev/fd933da7403d
In TestAudioTrackGraph use gmock to mock AudioDataListeners. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4111269589f3
Add a platform processing gtest to TestAudioTrackGraph. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f5155e405de0
Make ResetAudioProcessing idempotent wrt mPacketizerInput. r=karlt,padenot
https://hg.mozilla.org/integration/autoland/rev/0f2e7708c8ea
Add platform processing unit tests to TestAudioInputProcessing. r=padenot
https://hg.mozilla.org/integration/autoland/rev/91aa189a5e62
Update libcubeb to 74d6b05465. r=cubeb-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/fedac0c73475
Fix a type of race where AudioCallbackDriver::Start() inits the cubeb stream before tests have hooked up the StreamInitEvent. r=karlt

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

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch

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

Flags: needinfo?(apehrson)

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.

Flags: needinfo?(apehrson)

OK thanks

Regressions: 1895787

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.

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

Flags: needinfo?(apehrson)

For now it's been disabled. If we manage to ship this broadly, I'll nominate. Thanks for the reminder.

Flags: needinfo?(apehrson)

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.

Added to the Fx128 relnotes.

Blocks: 1867766
Duplicate of this bug: 1890426
Depends on: 1902989
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: