1744364 - Improve device list change coalescing

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, task, P1)

Description

[Karl Tomlinson (:karlt)]

MediaManager and MediaDevices each have code to merge device list changes.
These can be merged into one set of logic.

Hardware device change events were first coalesced over 1s in MediaDevices, with the timer being extended by 1s each time a new hardware event was received. https://hg.mozilla.org/mozilla-central/rev/d7cb0a69d237b625a36505941644390c3c1e5378

A 0.1s main-thread-blocking sleep was then added in MediaManager before stopping tracks after every hardware event,
https://hg.mozilla.org/mozilla-central/rev/00af61b0dd2ff664ecc4b23d668219505930ee92
which was subsequently extended to 0.2s.
https://hg.mozilla.org/mozilla-central/rev/fb45e3d534cd

The MediaDevices timer was changed so as not to extend but always fire within 1s of a hardware event.
https://hg.mozilla.org/mozilla-central/rev/d6f3399cc0abc5c4c68e78a0a1f6a8e5b999b19a

The MediaManager sleep was then changed to a (still 0.2s) non-blocking timer, extended by 0.2s on each new hardware event.
https://hg.mozilla.org/mozilla-central/rev/098923480472

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 return GetUserMediaWindowListener devices by return value instead of side effect r?jib

https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs

Depends on D132888

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 remove unnecessary MediaManager::mDeviceIDs r?jib

GetUserMediaWindowListener() does not have active listeners for speakers.

Depends on D132889

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 move contents of delayed device change handling lambda to a method r?jib

This will simplify invocation from more than one call site.

Depends on D132890

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 limit coalescing of device list change handling to 1 second r?jib

Depends on D132891

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 use MediaManager device list change coalescing for MediaDevices r?jib

There is a chance that this may result in some associated events no longer
being coalesced into a single devicechange event, but there is little
disadvantage to this while there is an advantage to getting devicechange
events promptly.

Future changes for https://bugzilla.mozilla.org/show_bug.cgi?id=1732409 will
track exactly which devices have changed and so filter out no-op
notifications.

Depends on D132892

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1744364 test that devicechange event is fired even when hardware changes continue r?jib

Depends on D132909

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d408587b51d
return GetUserMediaWindowListener devices by return value instead of side effect r=jib
https://hg.mozilla.org/integration/autoland/rev/b7304bae2ec0
remove unnecessary MediaManager::mDeviceIDs r=jib
https://hg.mozilla.org/integration/autoland/rev/c02b2bfbd79c
move contents of delayed device change handling lambda to a method r=jib
https://hg.mozilla.org/integration/autoland/rev/4757b1d54993
limit coalescing of device list change handling to 1 second r=jib

Comment 8

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/8d408587b51d
https://hg.mozilla.org/mozilla-central/rev/b7304bae2ec0
https://hg.mozilla.org/mozilla-central/rev/c02b2bfbd79c
https://hg.mozilla.org/mozilla-central/rev/4757b1d54993

Comment 9

[Karl Tomlinson (:karlt)]

I'll move the remaining patches to bug 1753131.

Comment 10

[Phabricator Automation]

Comment on attachment 9253806 [details]
Bug 1744364 use MediaManager device list change coalescing for MediaDevices r?jib

Revision D132893 was moved to bug 1753131. Setting attachment 9253806 [details] to obsolete.

Comment 11

[Phabricator Automation]

Comment on attachment 9253823 [details]
Bug 1744364 test that devicechange event is fired even when hardware changes continue r?jib

Revision D132910 was moved to bug 1753131. Setting attachment 9253823 [details] to obsolete.