Closed Bug 1581193 Opened 5 years ago Closed 5 years ago

devicechange event stopped working when unplugging/replugging device

Categories

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

69 Branch
Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 - wontfix
firefox70 + verified
firefox71 + verified

People

(Reporter: jib, Assigned: dminor)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(1 file)

STRs on Windows:

  1. Attach a USB camera
  2. Open https://jan-ivar.github.io/dummy/enumerate.html
  3. Click Start Camera! button, check ☑ Remember this decision and hit Allow
  4. Unplug USB camera

Expected result:

  • The text "device change!" is output

Actual result:

  • No text is output

Regression range:

[Tracking Requested - why for this release]: Regressed behavior on Windows.

I'll take this. Alex, if you have a fix in progress, please let me know.

Assignee: nobody → dminor

I've filed Bug 1581527 to track upstreaming our local modifications to the video_capture code to support devicechanged events. This is not the first time this has ended up broken after an update. We don't have test coverage, and adding it would require creating a means of plugging and unplugging fake cameras, so I think our best bet is to try to get it landed upstream.

Getting manual test cases for this into QA's manual regression test plan would also be good.

This restores the code for generating devicechange events that was
accidentally removed as part of updating the Windows video capture code
in Bug 1552755.

Andrei, is this something you can help get into the standard test plan (from comment 4)?

Flags: needinfo?(andrei.vaida)

(In reply to Liz Henry (:lizzard) from comment #6)

Andrei, is this something you can help get into the standard test plan (from comment 4)?

We'll add this to our WebRTC test suite. I'm forwarding it to Bogdan, for prioritization.

Flags: needinfo?(bogdan.maris)
Flags: needinfo?(andrei.vaida)
Flags: in-qa-testsuite?(bogdan.maris)

I have been testing further that code and I found out that notifications are triggered with any device change, not just video and audio devices. Interestingly, I noticed that it creates a device change notification when my USB flash disk is plugged/unplugged. I have been working on limiting the device notifications to audio and video input devices only. I would like to push that fix here, on top of the existing patch, in order to upstream/uplift altogether. Dan, are you ok with that? Could you review it?

Flags: needinfo?(dminor)
Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/840c4edafa02 Fix devicechange event on Windows; r=achronop

(In reply to Alex Chronopoulos [:achronop] from comment #8)

I have been testing further that code and I found out that notifications are triggered with any device change, not just video and audio devices. Interestingly, I noticed that it creates a device change notification when my USB flash disk is plugged/unplugged. I have been working on limiting the device notifications to audio and video input devices only. I would like to push that fix here, on top of the existing patch, in order to upstream/uplift altogether. Dan, are you ok with that? Could you review it?

Sorry, I missed seeing this. I'd be happy to review it, but I guess it will have to land separately.

Flags: needinfo?(dminor)
Regressions: 1581806

No problem, I've created Bug 1581806.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Comment on attachment 9093045 [details]
Bug 1581193 - Fix devicechange event on Windows; r=achronop

Beta/Release Uplift Approval Request

  • User impact if declined: Websites will not receive devicechange events when a camera or microphone is unplugged and so will not be able to do things like ask for permission to use a different device. We also want to pick up the changes from Bug 1581806, which ensures that the events are only triggered for camera and microphone rather than every USB device.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Steps are mentioned in comment 0.
  • List of other uplifts needed: Bug 1581806
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is low risk because it restores previously working code that was accidentally deleted while doing an update from upstream webrtc.org.
  • String changes made/needed: None
Attachment #9093045 - Flags: approval-mozilla-beta?
Flags: qe-verify+

(In reply to Andrei Vaida [:avaida] from comment #7)

(In reply to Liz Henry (:lizzard) from comment #6)

Andrei, is this something you can help get into the standard test plan (from comment 4)?

We'll add this to our WebRTC test suite. I'm forwarding it to Bogdan, for prioritization.

We added a testcase for this scenario in our WebRTC test suite.

QA Whiteboard: [qa-triaged]
Flags: needinfo?(bogdan.maris)
Flags: in-qa-testsuite?(bogdan.maris)
Flags: in-qa-testsuite+

I have managed to reproduce the issue using Fx71.0a1 buildID: 20190913214459.
The issue is verified fixed using Fx71.0a1 buildID: 20190922213341 on windows 10, macOS 10.14.6 and ubuntu 18.04. The 'device change!' message is displayed when the usb camera is unplugged.

Waiting on the beta uplift to fully verify the ticket.

Comment on attachment 9093045 [details]
Bug 1581193 - Fix devicechange event on Windows; r=achronop

Fix for recent regression in WebRTC, OK for uplift for beta 10.

Attachment #9093045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Tested using Fx 70.0b10 buildID: 20190924213629, on windows 10, macOS 10.14.6 and Ubuntu 18.04. The text is correctly displayed when disconnecting the USB camera.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: