1913932 - Fix platform processing when using aggregated processing settings

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Andreas Pehrson [:pehrsons]]

Platform processing was disabled in bug 1908539 because Google Meet would request two getUserMedia tracks for the same device: one with default processing constraints, and one with all processing constraints being false.

Gecko will in such a case make both tracks share the device, so the processing applied in the platform will therefore be in an aggregate form, the intersection of the processing param set for both tracks, i.e. no platform processing since one track had requested it to be disabled.

This caused echo in Google Meet because Gecko has a bug where the result of applying the aggregate settings is ignored, leading to the sw processing not getting applied at all on the track having processing set.

I'll use this bug to fix that Gecko bug and let platform processing ride the trains again.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1913932 - Add gtests. r?padenot

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1913932 - Correctly document the thread access of DeviceInputTrack::NotifySetRequestedProcessingParamsResult. r?padenot

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1913932 - Add an event and a generation id to handle multiple concurrent platform processing params. r?padenot

As described, this patch adds two things:
A new event where an audio processing track can get notified that new
processing params are about to get applied to the source. This is to handle use
cases involving aggregate processing params, i.e. when two tracks want to apply
different processing params to the same device. A second such track would
otherwise cause the aggregate params to get applied before the first track gets
notified async through the result event, possibly causing a glitch -- if
platform params are relaxed before compensated by the track. And,

A generation id of the processing params getting applied to a particular device.
Since the event informing a processing track of the result of applying platform
processing params is async from the new event informing the processing track of
the platform processing params about to get applied, described above, there can
in theory be any number of requests to apply processing params in flight at any
given time. The generation id is needed to distinguish two distinct requests to
apply identical processing params to a given device, where neither of the
requests have signaled their result yet. This allows the processing track to
avoid relaxing the software processing params prematurely, which could cause a
glitch similar to that described for the new event above.

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1913932 - Enable platform processing to release. r?padenot

Comment 5

[Andreas Pehrson [:pehrsons]]

A profile showing this issue being hit on Google Meet, with the fixes applied: https://share.firefox.dev/3WSKDfk
AEC fallback to the libwebrtc sw one is confirmed working by ear.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2267ffd93e3
Add gtests. r=padenot
https://hg.mozilla.org/integration/autoland/rev/21cbc12ab797
Correctly document the thread access of DeviceInputTrack::NotifySetRequestedProcessingParamsResult. r=padenot
https://hg.mozilla.org/integration/autoland/rev/bfa185b0b771
Add an event and a generation id to handle multiple concurrent platform processing params. r=padenot

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e2267ffd93e3
https://hg.mozilla.org/mozilla-central/rev/21cbc12ab797
https://hg.mozilla.org/mozilla-central/rev/bfa185b0b771

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b2d48bc3698
Enable platform processing to release. r=padenot

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/2b2d48bc3698

Comment 10

[Andreas Pehrson [:pehrsons]]

This is meant to mark this bug as the root cause of bug 1908539, not the other way around.