GNOME Bugzilla – Bug 583640
[v4lsrc/v4l2src] add support for better device detection with libgudev
Last modified: 2009-09-15 14:30:18 UTC
Hi, as some of you may have read in my mail to gst-devel [1] I'm currently looking for a HAL replacement to do device detection in Cheese. I'd really like to remove all the hal stuff and the ioctls we use know in favor of a pure gstreamer approach. At the moment v4l2src property prober blindly scans /dev/ dir looking for video devices. This patch adds support for a better device enumeration using libudev. It also allows to query device-name and flags while the device is still closed so I can do something like (pseudocode) src = gst_element_factory_make ("v4lsrc"); list = probe_devices_with_property_probe (src); for device in list { g_object_set (src, "device", device, NULL); g_object_get (src, "device-name", &device-name, NULL); g_print ("detected device %s (%s)\n", device-name, device); g_object_get (src, "device-name", &flags, NULL); if (flags & V4L2_CAP_CAPTURE) add_device_to_the_list (device) } This cannot be done at the moment since flags property can be queried only if the device is opened (pipeline READY or PLAYING). Libudev support is completely optional (--with-libudev argument) and code falls back to current methods if it's disabled or if it fails.
Created attachment 135232 [details] [review] v4l2src: add optional support for udev device detection
oh, I forgot the note :P 1. http://tinyurl.com/gstv4ldevicedetection
Any news? I'd really like to get some feedback on this and hopefully get it included so I can remove that stuff from cheese in time for 2.28. Also I'd like to get your thoughts about this so I can write the counterpart patch for v4lsrc. IMHO this patch doesn't introduce any regression and improves a lot the current detection code. I have a couple of improvements in mind for the current patch, but first I'd really like to get some opinion from other gstreamer developers.
Where does libudev comes from - I don't have it on any of my systems? The code looks nice. What I wonder is if we would want to have this also for e.g. alsa plugin. Should we have a small helper lib that gst_v4l2_get_udev_property -> gst_udev_get_property in the long run.
I think our current device probing API (GstPropertyProbe and selected properties) is complete rubbish and not fit for the job, and we shouldn't put too much effort into improving it, let alone add new dependencies for that, even if they're optional. I think we should rather try and come up with a new API (GstDeviceProbe interface or somesuch) that doesn't completely suck. I realise that's not going to help you immediately, but then getting this code out of cheese and into gstreamer is not exactly mission critical, so maybe we can afford taking a bit longer to get this right?
(In reply to comment #4) > Where does libudev comes from - I don't have it on any of my systems? It's part of udev since version 182 or so, IIRC. Probably distributors ship it in a separate package though, (I have it in Fedora 11).
(In reply to comment #5) > I think our current device probing API (GstPropertyProbe and selected > properties) is complete rubbish and not fit for the job, and we shouldn't put > too much effort into improving it, let alone add new dependencies for that, > even if they're optional. I think we should rather try and come up with a new > API (GstDeviceProbe interface or somesuch) that doesn't completely suck. I agree, GstPropertyProbe sucks, but that's what we have now. And, IMHO, it's not so good to reject an improvement of the present state in the name of a future, probably better but still not even sketched, interface that will replace it. I wrote this patch because my mail in gstreamer-devel didn't get any reply. So I supposed (and still believe) PropertyProbe rework it's not exactly an high priority task, and there isn't anyone willing/able to spend some time in designing a good interface for device detection. If propertyprobe sucks, its v4l?src implementation sucks even more. This patch just tries to fix the current implementation (with this patch I can easily do device detection, while still being a bit ugly it works pretty fine. As it is now I just cannot use gstreamer for that job). About the new dependency argument, let alone it's optional, please note that libudev is part of udev, it's currently used at least in pulseaudio, and will be used soon (if not yet) in DeviceKit-power, DeviceKit-disks, NetworkManager, gnome-bluetooth, maybe X and probably many more (http://lists.freedesktop.org/archives/devkit-devel/2009-April/000140.html). So it's not an exotic dependency, it's or will be soon part of the core stuff in every linux distribution. > I realise that's not going to help you immediately, but then getting this code > out of cheese and into gstreamer is not exactly mission critical, so maybe we > can afford taking a bit longer to get this right? Sure I have no hurry, unless a bit longer does mean never ;) Anyway, while getting this code out of cheese isn't mission critical I consider a gstreamer fail having to do a low level task like device detection in cheese. @Stefan: I meant udev 128, sorry.
I am not against this patch. Imho its not changing the interface, but improving the internals. I need to upgrade my ubuntu, udev-124 here. I will do that this week and then test the patch.
We also have libgudev now, that's part of the just released udev 143. http://www.kernel.org/pub/linux/utils/kernel/hotplug/gudev/
(In reply to comment #9) > We also have libgudev now, that's part of the just released udev 143. Yep, I was planning to port the patch to libgudev but I was waiting for a decision about this before to hack on it further :P
May I ask a decision about this before Base freeze (Jul 6)? I should have a couple of spare days the next week and I could update the patch and port it to v4lsrc too. If not, at least for Good freeze (Aug 7) would be great but I'm not sure if it's worth to maintain two differents code paths in cheese for v4lsrc and v4l2src so getting included in both Base and Good is highly preferred. If you need more time for decision just let me know... I will include libgudev code in cheese and postpone this thing to 2.29.
Created attachment 137765 [details] [review] v4l2src: optional support for device probing with gudev This new patch just adds support for device probing using gudev. Now that the device is opened in READY state there is no more need for udev based property getters so that part has been stripped out. Now the workflow for device probing would be: probe devices with propertyprobe for (devices) { set "device" set GST_STATE_READY (open the device) get useful properties (e.g. drop non capture devices) set GST_STATE_NULL (close the device) } Note that this patch is *a lot* more trivial than the previous. If this would be applied cheese will have a little regression because there won't be any way to retrieve Product and Vendor id for the devices. Another patch that adds those properties to v4l2src will probably follow.
Created attachment 137766 [details] [review] simple test case A little example I wrote to quickly test my changes.
Created attachment 137774 [details] [review] v4lsrc: optional support for device probing with gudev Same (almost) patch for v4lsrc
It sounds fine to me. OpenSolaris doesn't have udev, but preserving the old behaviour should mean there's no regression in v4l2src there.
(In reply to comment #15) > It sounds fine to me. OpenSolaris doesn't have udev, but preserving the old > behaviour should mean there's no regression in v4l2src there. Yep, there shouldn't be any regression. So, can I commit the patches?
I've seen noone complaining - so go for it
Committed both patches and added a slightly modified test case both in -base and in -good