After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 583640 - [v4lsrc/v4l2src] add support for better device detection with libgudev
[v4lsrc/v4l2src] add support for better device detection with libgudev
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: 0.10.16
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 583469 593938 594966
 
 
Reported: 2009-05-23 12:56 UTC by Filippo Argiolas
Modified: 2009-09-15 14:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2src: add optional support for udev device detection (10.71 KB, patch)
2009-05-23 12:57 UTC, Filippo Argiolas
none Details | Review
v4l2src: optional support for device probing with gudev (6.28 KB, patch)
2009-07-03 07:04 UTC, Filippo Argiolas
committed Details | Review
simple test case (3.94 KB, patch)
2009-07-03 07:08 UTC, Filippo Argiolas
committed Details | Review
v4lsrc: optional support for device probing with gudev (5.93 KB, patch)
2009-07-03 09:40 UTC, Filippo Argiolas
committed Details | Review

Description Filippo Argiolas 2009-05-23 12:56:36 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.
Comment 1 Filippo Argiolas 2009-05-23 12:57:37 UTC
Created attachment 135232 [details] [review]
v4l2src: add optional support for udev device detection
Comment 2 Filippo Argiolas 2009-05-23 12:59:28 UTC
oh, I forgot the note :P

1. http://tinyurl.com/gstv4ldevicedetection
Comment 3 Filippo Argiolas 2009-05-28 07:03:19 UTC
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.
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-01 10:57:07 UTC
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.
Comment 5 Tim-Philipp Müller 2009-06-01 11:28:48 UTC
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?
Comment 6 Filippo Argiolas 2009-06-01 12:37:50 UTC
(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).
Comment 7 Filippo Argiolas 2009-06-01 13:09:33 UTC
(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.
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2009-06-01 14:47:23 UTC
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.
Comment 9 David Zeuthen (not reading bugmail) 2009-06-20 15:38:14 UTC
We also have libgudev now, that's part of the just released udev 143.

 http://www.kernel.org/pub/linux/utils/kernel/hotplug/gudev/
Comment 10 Filippo Argiolas 2009-06-20 15:48:45 UTC
(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
Comment 11 Filippo Argiolas 2009-06-28 08:39:06 UTC
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.
Comment 12 Filippo Argiolas 2009-07-03 07:04:35 UTC
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.
Comment 13 Filippo Argiolas 2009-07-03 07:08:13 UTC
Created attachment 137766 [details] [review]
simple test case

A little example I wrote to quickly test my changes.
Comment 14 Filippo Argiolas 2009-07-03 09:40:34 UTC
Created attachment 137774 [details] [review]
v4lsrc: optional support for device probing with gudev

Same (almost) patch for v4lsrc
Comment 15 Jan Schmidt 2009-07-08 17:03:10 UTC
It sounds fine to me. OpenSolaris doesn't have udev, but preserving the old behaviour should mean there's no regression in v4l2src there.

Comment 16 Filippo Argiolas 2009-07-10 06:26:00 UTC
(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?
Comment 17 Jan Schmidt 2009-07-12 12:19:22 UTC
I've seen noone complaining - so go for it
Comment 18 Filippo Argiolas 2009-07-13 14:26:08 UTC
Committed both patches and added a slightly modified test case both in -base and in -good