[Bug 796958] ksvideosrc: device enumeration bug when hotplugging a camera

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796958

--- Comment #2 from LRN <lrn1986 at gmail.com> ---
Oh-kay. That was some years back, but i'll try to figure it out.

Now, the GUID testing at the beginning - i don't know what i was thinking, but
my guess is that a check for KSCATEGORY_CAPTURE or KSCATEGORY_RENDER was
intended to ensure that only I/O media devices get processed. The commit
message mentions that this code *could* support audio, in theory, so i might
have been thinking in that direction. However, when later the device is
actually found, the code always calls new_video_source() to create a video
device, so only video is supported at the moment.

I'm less sure about the ks_enumerate_devices(). Why have you changed that call?
Did it not work in its current form? Enumerating the devices that match the
GUID of the new device serves to narrow down the list (according to what i
wrote back in bug 747757).

It is my understanding that DBT_DEVICEARRIVAL processing here is meant to find
the new device that just arrived. It checks dbcc_name to ensure that it only
gets that specific new device, and not the rest of the devices. 

According to what i wrote years before, we get multiple DBT_DEVICEARRIVAL, for
each (?) supported class, so a capture device would always eventually give us a
message that matches KSCATEGORY_CAPTURE, and then we'll enumerate all capture
devices (kind of suboptimal, but i guess i was trying to achieve most with the
least amount of code...), we find the newly-added device, and, well, add it to
GST.

My guess that in your case it grabs the new audio device (since by that point
it checks only by name, not by type, and the new audio device matches).

A more correct fix would be to only change the ks_enumerate_devices() call,
changing the first argument to &KSCATEGORY_VIDEO. And add a comment mentioning
that for audio support we will have to call it *again*, but with
&KSCATEGORY_AUDIO, and then call a hypothetical new_audio_source() function if
we find a match.
Actually, for the initial GUID check the &KSCATEGORY_RENDER should be removed,
since, again, we only support capture-devices. And, again, a comment mentioning
it (that for playback support we need to check KSCATEGORY_RENDER devices) would
be nice.

That way we'll only look at capture devices, and of those we'll look only at
the ones that are also *video* devices. Something along these lines:

> /* Only capture devices supported at the moment.
>  * Check also for KSCATEGORY_RENDER to support playback devices.
>  */
> if (!IsEqualGUID (&bcdi->dbcc_classguid, &KSCATEGORY_CAPTURE))
>   break;
>
> /* List all video devices that also support dbcc_classguid,
>  * i.e. KSCATEGORY_CAPTURE (see above). For audio recording devices
>  * repeat the same, but with &KSCATEGORY_AUDIO.
>  * For A/V playback devices repeat with KSCATEGORY_RENDER.
>  */
> devices =
>     ks_enumerate_devices (&KSCATEGORY_VIDEO,
>     &bcdi->dbcc_classguid);

Now that i'm thinking about this, it does kind of look like the patch you've
submitted...

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.