1300468 - Implement mediaDevices.ondevicechange for Windows

Status: RESOLVED FIXED

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

Comment 1

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

It seems we need to migrate to Media Foundation to support this feature.

Comment 2

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1300468 - implement mediaDevices.ondevicechange for Windows;

Review commit: https://reviewboard.mozilla.org/r/83232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83232/

Comment 3

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

It turns out comment 1 was wrong. We don't need to migrate to Media foundation to support this api.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8797541 [details]
Bug 1300468 - implement mediaDevices.ondevicechange for Windows;

https://reviewboard.mozilla.org/r/83232/#review82090

r+ with nits.  Note I'm not really a windows-API guy

::: media/webrtc/trunk/webrtc/modules/video_capture/windows/device_info_ds.cc:51
(Diff revision 1)
> +LRESULT CALLBACK WndProc(HWND hWnd, UINT uiMsg, WPARAM wParam, LPARAM lParam)
> +{
> +  if (uiMsg == WM_DEVICECHANGE)
> +  {
> +    DeviceInfoDS* dsInfo = DeviceInfoDSSingleton::GetInfo();
> +    if (dsInfo != NULL) { dsInfo->DeviceChange(); }

no single-line if's (see below)

::: media/webrtc/trunk/webrtc/modules/video_capture/windows/device_info_ds.cc:139
(Diff revision 1)
> +    _wndClass.lpszClassName = TEXT("DeviceInfoDS");
> +    _wndClass.hInstance = _hInstance;
> +
> +    if (RegisterClass(&_wndClass))
> +    {
> +        _hwnd = CreateWindow(_wndClass.lpszClassName, NULL, 0, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, _hInstance, NULL);

please wrap the line

::: media/webrtc/trunk/webrtc/modules/video_capture/windows/device_info_ds.cc:150
(Diff revision 1)
>      RELEASE_AND_CLEAR(_dsDevEnum);
>      if (_CoUninitializeIsRequired)
>      {
>          CoUninitialize();
>      }
> +    if (_hwnd != NULL) { DestroyWindow(_hwnd); }

No single-line if's please.  In this file, use:
if (...)
{
   ...
}

Comment 5

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8797541 [details]
Bug 1300468 - implement mediaDevices.ondevicechange for Windows;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/83232/diff/1-2/

Comment 6

[Carsten Book [:Tomcat]]

has problems to apply:

patching file dom/media/tests/mochitest/mochitest.ini
Hunk #1 FAILED at 35
1 out of 1 hunks FAILED -- saving rejects to file dom/media/tests/mochitest/mochitest.ini.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

Comment 7

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug-1300468-implement-mediaDevices-ondevicechange-for-Windows.patch

Please apply the rebased patch.

Comment 8

[Carsten Book [:Tomcat]]

will do, thanks for the rebased patch!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18722be58f18
implement mediaDevices.ondevicechange for Windows; r=jesup

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/18722be58f18

Comment 11

[Eric Shepherd [:sheppy]]

Added info about Windows compat arriving in Firefox 52 to the following pages:

https://developer.mozilla.org/en-US/docs/Web/Events/devicechange
https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/ondevicechange
https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices
https://developer.mozilla.org/en-US/Firefox/Experimental_features
https://developer.mozilla.org/en-US/Firefox/Releases/52

Comment 12

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Fyi, I think this is still behind a pref? media.ondevicechange.enabled