1582637 - Try to move devicechange event internals to higher order functions

Status: RESOLVED FIXED

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

Description

[Andreas Pehrson [:pehrsons]]

With bug 1572281 we'll have two separate paths triggering devicechange events through MediaManager (CamerasChild and CubebDeviceEnumerator). I think these paths would benefit by going from the manual, raw listener handling they're using today, to higher order functionality like MediaEvents. This bug is meant to investigate whether that would work in a nice fashion.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Ensure MediaManager::sSingleton usage is thread safe by guarding it with a mutex. r?jib

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Inject an already-initialized media thread to the MediaManager ctor. r?jib

This lets us dispatch tasks already in the MediaManager ctor.

Depends on D48512

Comment 3

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Remove cameras:: from most of MediaEngineWebRTC.cpp. r?jib

Depends on D48513

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Remove unnecessary MediaEngineWebRTC::mMutex. r?jib

Depends on D48514

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Move internal DeviceChange events to higher order functions. r?jib!,achronop!

This does three major things:

1. Moves the DeviceChange events from manual callbacks/listeners to
   MediaEventSource/MediaEventListener. This is the reason this patch is so
   large, as it traverses a lot of files.
   There are four layers (from low to high):
   • CamerasChild for camera device list changes, and CubebDeviceEnumerator for
     microphone and speaker device list changes
   • MediaEngineWebRTC, which gathers these into a single listener
   • MediaManager, which owns the MediaEngineWebRTC backend
   • MediaDevices, where the events from MediaManager are exposed to js
2. Changes the fake event triggering from starting a 30-event burst on setting
   the js event listener, to a toggle, so that while the pref is on the events
   keep coming.
3. Moves the fake event generation from CamerasChild to MediaEngineWebRTC, since
   that's the lowest level where we are aware of both video and audio events.
   The fake event generation is also greatly simplified. From being a dedicated
   thread with periodic runnables, it is now a periodic timer on main thread
   that fires while fake events are enabled. MediaEventProducer gracefully
   handles thread safety.

Depends on D48515

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Reduce devicechange fake event and test timeout times. r?jib

This allows the mochitest to finish several seconds faster.

Depends on D48516

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Inline MediaDevices' FuzzTimer callback and invert coalescing logic. r?jib

If MediaDevices received recurring devicechange events from MediaManager, the
FuzzTimer would be restarted for each one, to coalesce them into the same js
event. If the internal events kept coming sooner than the fuzz timer timeout,
the fuzz timer would never fire.

This patch inverts the logic, so that the first scheduled fuzz timer fires, and
any intermediate internal events are ignored. After it has fired, a new internal
event triggers a new fuzz timer.

Depends on D48517

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Fix minor devicechange mochitest issues. r?jib

This makes rejections reject with an Error so that stacks make sense on failure.
It also changes some let to const since they're never assigned to.

Depends on D48519

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Remove pref to enable devicechange in mochitest since it's enabled by default. r?jib

Depends on D48520

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1582637 - Fix devicechange mochitest failing sometimes only (!) and use it also with loopback devices. r?jib

Depends on D48521

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d40bb93b5d83
Ensure MediaManager::sSingleton usage is thread safe by guarding it with a mutex. r=jib
https://hg.mozilla.org/integration/autoland/rev/bc24be7e8e9c
Inject an already-initialized media thread to the MediaManager ctor. r=jib
https://hg.mozilla.org/integration/autoland/rev/5e4230f6c609
Remove cameras:: from most of MediaEngineWebRTC.cpp. r=jib
https://hg.mozilla.org/integration/autoland/rev/2652d06847e6
Remove unnecessary MediaEngineWebRTC::mMutex. r=jib
https://hg.mozilla.org/integration/autoland/rev/c5d1b1d238f7
Move internal DeviceChange events to higher order functions. r=jib,achronop
https://hg.mozilla.org/integration/autoland/rev/dc65cac4d9fd
Reduce devicechange fake event and test timeout times. r=jib
https://hg.mozilla.org/integration/autoland/rev/d6f3399cc0ab
Inline MediaDevices' FuzzTimer callback and invert coalescing logic. r=jib
https://hg.mozilla.org/integration/autoland/rev/1d6e24866948
Fix minor devicechange mochitest issues. r=jib
https://hg.mozilla.org/integration/autoland/rev/350b2c44853c
Remove pref to enable devicechange in mochitest since it's enabled by default. r=jib
https://hg.mozilla.org/integration/autoland/rev/02ea16355197
Fix devicechange mochitest failing sometimes only (!) and use it also with loopback devices. r=jib

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d40bb93b5d83
https://hg.mozilla.org/mozilla-central/rev/bc24be7e8e9c
https://hg.mozilla.org/mozilla-central/rev/5e4230f6c609
https://hg.mozilla.org/mozilla-central/rev/2652d06847e6
https://hg.mozilla.org/mozilla-central/rev/c5d1b1d238f7
https://hg.mozilla.org/mozilla-central/rev/dc65cac4d9fd
https://hg.mozilla.org/mozilla-central/rev/d6f3399cc0ab
https://hg.mozilla.org/mozilla-central/rev/1d6e24866948
https://hg.mozilla.org/mozilla-central/rev/350b2c44853c
https://hg.mozilla.org/mozilla-central/rev/02ea16355197