Only one event is registered when plugging / unplugging camera on Ubuntu for mediaDevices.ondevicechange

RESOLVED FIXED in Firefox 54

Status

()

P1
normal
Rank:
15
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: asimonca, Assigned: mchiang)

Tracking

Trunk
mozilla54
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox53 wontfix, firefox54 fixed)

Details

Attachments

(1 attachment)

[Note]:
- This issue is reproducible only on Ubuntu
- The steps provided are for reproducing the issue with the same hardware: 1 webcam & 1 headset with microphone (in this case: webcam - Microsoft® LifeCam HD-3000, headset: Plantronics Audio 355)

[Affected versions]:
- Aurora 52.0a2 

[Affected platforms]:
- Ubuntu 16.04 x64

[Steps to reproduce]:
1. Go to https://jsfiddle.net/jib1/Lqo4paed/
2. Press "gUM!" and "Share the selected device" on the doorhanger.
3. Press "Enumerate!". 4 devices are found: videoinput: Microsoft® LifeCam HD-3000, audioinput: default: LifeCam HD-3000 Analog Mono, audioinput: LifeCam HD-3000 Analog Mono, audioinput: Built-in Audio Analog Stereo
4. Disconnect the webcam. 

[Expected result]:
- 2 "device change!" messages should appear informing the user that there was a change in "videoinput" and in "audioinput"

[Actual result]:
- Only one "device change!" message is displayed when connecting and when disconnecting the webcam.

[Regression range]:
- This is not a regression.

Updated

2 years ago
Rank: 23
Priority: -- → P2
Cannot repro the issue with my c920.
Could you help run below command and copy & paste the result?

ls -lR /dev/v4l/ ; ls -lR /dev/snd/

Please run it before and after you disconnect LifeCam HD-3000.
Flags: needinfo?(alexandru.simonca)
You can see the terminal output below. As you can see '/dev/v41/ is not a valid folder on my OS. I have run the snippet in Terminal on Ubuntu 16.04 x64.


Before disconnect:

ls: cannot access '/dev/v41/': No such file or directory
/dev/snd/:
total 0
drwxr-xr-x  2 root root       60 ian 18 14:08 by-id
drwxr-xr-x  2 root root       80 ian 18 14:08 by-path
crw-rw----+ 1 root audio 116,  2 ian 18 13:59 controlC0
crw-rw----+ 1 root audio 116, 11 ian 18 14:08 controlC1
crw-rw----+ 1 root audio 116,  9 ian 18 13:59 hwC0D0
crw-rw----+ 1 root audio 116, 10 ian 18 13:59 hwC0D2
crw-rw----+ 1 root audio 116,  4 ian 18 12:00 pcmC0D0c
crw-rw----+ 1 root audio 116,  3 ian 18 14:06 pcmC0D0p
crw-rw----+ 1 root audio 116,  5 ian 18 13:59 pcmC0D2c
crw-rw----+ 1 root audio 116,  6 ian 18 14:00 pcmC0D3p
crw-rw----+ 1 root audio 116,  7 ian 18 14:00 pcmC0D7p
crw-rw----+ 1 root audio 116,  8 ian 18 12:00 pcmC0D8p
crw-rw----+ 1 root audio 116, 12 ian 18 14:08 pcmC1D0c
crw-rw----+ 1 root audio 116,  1 ian 18 13:59 seq
crw-rw----+ 1 root audio 116, 33 ian 18 13:59 timer

/dev/snd/by-id:
total 0
lrwxrwxrwx 1 root root 12 ian 18 14:08 usb-Microsoft_Microsoft®_LifeCam_HD-3000-02 -> ../controlC1

/dev/snd/by-path:
total 0
lrwxrwxrwx 1 root root 12 ian 18 14:08 pci-0000:00:14.0-usb-0:10:1.2 -> ../controlC1
lrwxrwxrwx 1 root root 12 ian 18 13:59 pci-0000:00:1f.3 -> ../controlC0


After disconnect:

ls: cannot access '/dev/v41/': No such file or directory
/dev/snd/:
total 0
drwxr-xr-x  2 root root       60 ian 18 14:11 by-path
crw-rw----+ 1 root audio 116,  2 ian 18 13:59 controlC0
crw-rw----+ 1 root audio 116,  9 ian 18 13:59 hwC0D0
crw-rw----+ 1 root audio 116, 10 ian 18 13:59 hwC0D2
crw-rw----+ 1 root audio 116,  4 ian 18 12:00 pcmC0D0c
crw-rw----+ 1 root audio 116,  3 ian 18 14:06 pcmC0D0p
crw-rw----+ 1 root audio 116,  5 ian 18 13:59 pcmC0D2c
crw-rw----+ 1 root audio 116,  6 ian 18 14:00 pcmC0D3p
crw-rw----+ 1 root audio 116,  7 ian 18 14:00 pcmC0D7p
crw-rw----+ 1 root audio 116,  8 ian 18 12:00 pcmC0D8p
crw-rw----+ 1 root audio 116,  1 ian 18 13:59 seq
crw-rw----+ 1 root audio 116, 33 ian 18 13:59 timer

/dev/snd/by-path:
total 0
lrwxrwxrwx 1 root root 12 ian 18 13:59 pci-0000:00:1f.3 -> ../controlC0
Flags: needinfo?(alexandru.simonca)
The path should be /dev/v4l/, not /dev/v41/
Flags: needinfo?(alexandru.simonca)
Same result:

alexandru.simonca@p6301:~$ ls -lR /dev/v4l/ ; ls -lR /dev/snd/
ls: cannot access '/dev/v4l/': No such file or directory
/dev/snd/:
total 0
drwxr-xr-x  2 root root       60 ian 18  2017 by-path
crw-rw----+ 1 root audio 116,  2 ian 18  2017 controlC0
crw-rw----+ 1 root audio 116,  9 ian 18  2017 hwC0D0
crw-rw----+ 1 root audio 116, 10 ian 18  2017 hwC0D2
crw-rw----+ 1 root audio 116,  4 ian 18  2017 pcmC0D0c
crw-rw----+ 1 root audio 116,  3 ian 18 14:28 pcmC0D0p
crw-rw----+ 1 root audio 116,  5 ian 18  2017 pcmC0D2c
crw-rw----+ 1 root audio 116,  6 ian 18  2017 pcmC0D3p
crw-rw----+ 1 root audio 116,  7 ian 18  2017 pcmC0D7p
crw-rw----+ 1 root audio 116,  8 ian 18  2017 pcmC0D8p
crw-rw----+ 1 root audio 116,  1 ian 18  2017 seq
crw-rw----+ 1 root audio 116, 33 ian 18  2017 timer

/dev/snd/by-path:
total 0
lrwxrwxrwx 1 root root 12 ian 18  2017 pci-0000:00:1f.3 -> ../controlC0
Flags: needinfo?(alexandru.simonca)
It's weird. I borrowed an Ubuntu 16.04 x64 laptop and the path /dev/v4l exists.
Could you help provide the whole /dev tree dump? (ls -lR /dev/)
Thanks!
Flags: needinfo?(alexandru.simonca)
ls -R /dev/ would be better. Otherwise it will output many pages.
Rank: 23 → 15
Priority: P2 → P1
Please dump this info with LifeCam HD-3000 connected.
For the desktop computer without built-in camera, there is no /dev/v4l in default. We need to take this into account while detecting connection.
Flags: needinfo?(alexandru.simonca)
Assignee: nobody → mchiang
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8830158 [details]
Bug 1331648 - detect device connection for the case /dev/v4l hasn't existed;

https://reviewboard.mozilla.org/r/107052/#review108380

::: media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc:57
(Diff revision 1)
> -    switch (event->mask) {
> -        case IN_CREATE:
> +    if (event->mask & IN_CREATE) {
> +        if (fd == _fd_v4l || fd == _fd_snd) {
>              DeviceChange();
> -            break;
> +        } else if ((event->mask & IN_ISDIR) && (fd == _fd_dev)) {

Logic is wrong here; the else if (... IN_ISDIR) can't ever be true (unless it's a subset of IN_CREATE, which I doubt)

::: media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc:80
(Diff revision 1)
> -            WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceVideoCapture, _id,
> -                "UNKNOWN EVENT OCCURRED for file \"%s\" on WD #%i\n",
> -                cur_event_filename, cur_event_wd);
> +    if (event->mask & IN_DELETE) {
> +        if (fd == _fd_v4l || fd == _fd_snd) {
> +            DeviceChange();

Can we keep the log message on unknown events?  that will make merging with upstream simpler

::: media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc:90
(Diff revision 1)
> +        } else if (fd == _fd_snd) {
> +            inotify_rm_watch(_fd_snd, _wd_snd);
> +            _wd_snd = -1;
> +        }

else I assume we don't care?  If so, fine, just checking if we want any sort of log or assertion.
Attachment #8830158 - Flags: review?(rjesup) → review-
(Assignee)

Comment 11

2 years ago
mozreview-review-reply
Comment on attachment 8830158 [details]
Bug 1331648 - detect device connection for the case /dev/v4l hasn't existed;

https://reviewboard.mozilla.org/r/107052/#review108380

> Logic is wrong here; the else if (... IN_ISDIR) can't ever be true (unless it's a subset of IN_CREATE, which I doubt)

The case (... IN_ISDIR ) is indeed true in this case.

https://www.kernel.org/pub/linux/kernel/people/rml/inotify/headers/inotify.h
Only the first 11 events are mutually exclusive.

#define IN_ACCESS		0x00000001	/* File was accessed */
#define IN_MODIFY		0x00000002	/* File was modified */
#define IN_ATTRIB		0x00000004	/* Metadata changed */
#define IN_CLOSE_WRITE		0x00000008	/* Writtable file was closed */
#define IN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
#define IN_OPEN			0x00000020	/* File was opened */
#define IN_MOVED_FROM		0x00000040	/* File was moved from X */
#define IN_MOVED_TO		0x00000080	/* File was moved to Y */
#define IN_CREATE		0x00000100	/* Subfile was created */
#define IN_DELETE		0x00000200	/* Subfile was deleted */
#define IN_DELETE_SELF		0x00000400	/* Self was deleted */

/* special flags */
#define IN_ISDIR		0x40000000	/* event occurred against dir */
#define IN_ONESHOT		0x80000000	/* only send event once */

While connecting a camera, a subfolder /dev/v4l is created, we will get a mask 0x40000100

> Can we keep the log message on unknown events?  that will make merging with upstream simpler

Will do

> else I assume we don't care?  If so, fine, just checking if we want any sort of log or assertion.

Will do
Comment hidden (mozreview-request)
Looks like jesup is already reviewing this, so I'm good if he's good.

Comment 15

2 years ago
mozreview-review
Comment on attachment 8830158 [details]
Bug 1331648 - detect device connection for the case /dev/v4l hasn't existed;

https://reviewboard.mozilla.org/r/107052/#review108572
Attachment #8830158 - Flags: review?(rjesup) → review+
Keywords: checkin-needed

Comment 16

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b32816fc8d58
detect device connection for the case /dev/v4l hasn't existed; r=jesup
Keywords: checkin-needed

Comment 17

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b32816fc8d58
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Lifecam 3000 is a common camera.  It's late for an uplift to 53, though this has had a lot of bake time, and this is linux-only (which reduces the risk of fixing it, and also the reward for fixing it).

Given the patch, which isn't trivial and involves fd's, I'm going to wontfix this for 53 at this point.  We could have uplifted it to aurora 53 without problems, but not to late beta.
status-firefox53: affected → wontfix
You need to log in before you can comment on or make changes to this bug.