devicechange event is not fired if the script doesn't call getusermedia or enumerateDevices

RESOLVED FIXED in Firefox 52

Status

()

defect
P2
normal
Rank:
27
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mchiang, Assigned: mchiang)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

According to the spec, if the script sets navigator.mediaDevices.ondevicechange and the permission state is "always granted", the User Agent MUST fires a devicechange event when a new media input device is made available even though the script never call getusermedia or enumerateDevices.

Currently Nightly won't fire the event in this condition because camera engine is not setup via getusermedia or enumerateDevices.

We need to ensure camera engine setup when the script set ondevicechange handler.
Assignee: nobody → mchiang
Don't forget .addEventListener and microphones (do we have microphones yet?)
Comment on attachment 8798327 [details]
Bug 1308114 - Setup camera engine when the script sets navigator.mediaDevices.ondevicechange;

https://reviewboard.mozilla.org/r/83846/#review82476

Up to you if you want to land this with a nit fix, or refactor by adding an IPDL EnsureInitialized() (SendEnsureInitialized/RecvEnsureinitialized).  

Also, is there any easy way to know that we don't need to do this IPC again?  Not a big issue, since this is only on AddDeviceChangeCallback, so the total overhead is very small - Only consider this if it's trivially available, which it probably is not.

::: dom/media/systemservices/CamerasChild.cpp:164
(Diff revision 1)
> +  // CamerasaParent api, e.g., RecvNumberOfCaptureDevices(), is called.
> +
> +  // So here we call NumberOfCaptureDevices in order to setup camera engine
> +  // via EnsureInitialized(aCapEngine)
> +
> +  NumberOfCaptureDevices(CameraEngine);

Your comment is good, though you're relying on a side-effect.  Better (though not mandatory) would be to add an ipdl API to tell CamerasParent to initialize - perhaps simply expose EnsureInitialized(CameraEngine).

Otherwise, to make it slightly more clear, add "Unused <<" or "(void)" to make it clear we're ignoring the return.
Attachment #8798327 - Flags: review?(rjesup) → review+
(In reply to Jan-Ivar Bruaroey [:jib] from comment #2)
> Don't forget .addEventListener
Yes, addEventListener case is also covered.

> and microphones (do we have microphones yet?)
Yes, we listen to both camera and microphone plug in/out event in the camera engine.

Mac: The AVFoundation observer AVCaptureDeviceWasConnectedNotification monitor both audio & video devices.
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_info_objc.mm#245

Linux: We monitor /dev/v4l/by-path/ for camera and /dev/snd/by-path/ for microphone
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc#146

Windows: We monitor the event WM_DEVICECHANGE for both audio & video devices.
https://hg.mozilla.org/mozilla-central/rev/18722be58f18#l2.33
Rank: 27
Priority: -- → P2
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/719f5341fc95
Setup camera engine when the script sets navigator.mediaDevices.ondevicechange; r=jesup
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/719f5341fc95
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.