Hello,
On Thu, Jan 24, 2019 at 10:24:48AM +0100, Hans Verkuil wrote:
> Resurrecting this thread, since this functionality would be really useful :-)
>
> On 11/10/18 12:17 PM, Ezequiel Garcia wrote:
> > On Sat, 2018-11-10 at 12:09 +0100, Hans Verkuil wrote:
> >> On 11/10/2018 12:01 PM, Ezequiel Garcia wrote:
> >>> On Sat, 2018-11-10 at 11:29 +0100, Hans Verkuil wrote:
> >>>> On 11/09/2018 10:52 PM, Ezequiel Garcia wrote:
> >>>>> This tool allows to find a device by driver name,
> >>>>> this is useful for scripts to be written in a generic way.
> >>>>
> >>>> Why not add support for this to v4l2-sysfs-path? And improve it at the same
> >>>> time (swradio devices do not show up when I run v4l2-sysfs-path, I also suspect
> >>>> v4l-touch devices aren't recognized. Ditto for media devices.
> >>>>
> >>>
> >>> I can add the functionality to v4l2-sysfs-path, and we can document
> >>> the rest in some TODO list, as I don't think we need to get everything
> >>> solved right now :-)
> >>
> >> I agree with that.
> >
> > Great!
> >
> > About adding it to v4l2-sysfs-path, is there any scenario where we'll want
> > to use udev-based scanning instead of sysfs-based? Maybe we can rename
> > the tool, and still install a v4l2-sysfs-path alias?
>
> To be honest, I don't know. It appears that sysfs is the most generic approach
> and I have seen cases where udev isn't installed on an embedded system.
>
> So I feel that sysfs is the best approach for now.
The problem with sysfs is that it won't give you the device node path.
You can use it to get the device node name known to the kernel, but if
the device node gets renamed by a udev policy, you won't know. That's
why it's better to use udev if available, and fall back on sysfs
otherwise.
Please see below for an additional comment.
> BTW, I've added support for swradio and v4l-touch devices to v4l2-sysfs-path, but I
> noticed that it didn't recognize media devices, I need to dig into it a bit to
> understand how to find those devices.
>
> >>>>> Usage:
> >>>>>
> >>>>> v4l2-get-device -d uvcvideo -c V4L2_CAP_VIDEO_CAPTURE
> >>>>> /dev/video0
> >>>>> /dev/video2
> >>>>>
> >>>>> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> >>>>> ---
> >>>>> configure.ac | 1 +
> >>>>> utils/Makefile.am | 1 +
> >>>>> utils/v4l2-get-device/.gitignore | 1 +
> >>>>> utils/v4l2-get-device/Makefile.am | 4 +
> >>>>> utils/v4l2-get-device/v4l2-get-device.c | 147 ++++++++++++++++++++++++
> >>>>> v4l-utils.spec.in | 1 +
> >>>>> 6 files changed, 155 insertions(+)
> >>>>> create mode 100644 utils/v4l2-get-device/.gitignore
> >>>>> create mode 100644 utils/v4l2-get-device/Makefile.am
> >>>>> create mode 100644 utils/v4l2-get-device/v4l2-get-device.c
> >>>>>
> >>>>> diff --git a/configure.ac b/configure.ac
> >>>>> index 5cc34c248fbf..918cb59704b9 100644
> >>>>> --- a/configure.ac
> >>>>> +++ b/configure.ac
> >>>>> @@ -31,6 +31,7 @@ AC_CONFIG_FILES([Makefile
> >>>>> utils/v4l2-compliance/Makefile
> >>>>> utils/v4l2-ctl/Makefile
> >>>>> utils/v4l2-dbg/Makefile
> >>>>> + utils/v4l2-get-device/Makefile
> >>>>> utils/v4l2-sysfs-path/Makefile
> >>>>> utils/qv4l2/Makefile
> >>>>> utils/cec-ctl/Makefile
> >>>>> diff --git a/utils/Makefile.am b/utils/Makefile.am
> >>>>> index 2d5070288c13..2b2b27107d13 100644
> >>>>> --- a/utils/Makefile.am
> >>>>> +++ b/utils/Makefile.am
> >>>>> @@ -9,6 +9,7 @@ SUBDIRS = \
> >>>>> v4l2-compliance \
> >>>>> v4l2-ctl \
> >>>>> v4l2-dbg \
> >>>>> + v4l2-get-device \
> >>>>> v4l2-sysfs-path \
> >>>>> cec-ctl \
> >>>>> cec-compliance \
> >>>>> diff --git a/utils/v4l2-get-device/.gitignore b/utils/v4l2-get-device/.gitignore
> >>>>> new file mode 100644
> >>>>> index 000000000000..b222144c9f4e
> >>>>> --- /dev/null
> >>>>> +++ b/utils/v4l2-get-device/.gitignore
> >>>>> @@ -0,0 +1 @@
> >>>>> +v4l2-get-device
> >>>>> diff --git a/utils/v4l2-get-device/Makefile.am b/utils/v4l2-get-device/Makefile.am
> >>>>> new file mode 100644
> >>>>> index 000000000000..2e5a6e0ba32f
> >>>>> --- /dev/null
> >>>>> +++ b/utils/v4l2-get-device/Makefile.am
> >>>>> @@ -0,0 +1,4 @@
> >>>>> +bin_PROGRAMS = v4l2-get-device
> >>>>> +v4l2_get_device_SOURCES = v4l2-get-device.c
> >>>>> +v4l2_get_device_LDADD = ../libmedia_dev/libmedia_dev.la
> >>>>> +v4l2_get_device_LDFLAGS = $(ARGP_LIBS)
> >>>>> diff --git a/utils/v4l2-get-device/v4l2-get-device.c b/utils/v4l2-get-device/v4l2-get-device.c
> >>>>> new file mode 100644
> >>>>> index 000000000000..f9cc323b7057
> >>>>> --- /dev/null
> >>>>> +++ b/utils/v4l2-get-device/v4l2-get-device.c
> >>>>> @@ -0,0 +1,147 @@
> >>>>> +/*
> >>>>> + * Copyright © 2018 Collabora, Ltd.
> >>>>> + *
> >>>>> + * Based on v4l2-sysfs-path by Mauro Carvalho Chehab:
> >>>>> + *
> >>>>> + * Copyright © 2011 Red Hat, Inc.
> >>>>> + *
> >>>>> + * Permission to use, copy, modify, distribute, and sell this software
> >>>>> + * and its documentation for any purpose is hereby granted without
> >>>>> + * fee, provided that the above copyright notice appear in all copies
> >>>>> + * and that both that copyright notice and this permission notice
> >>>>> + * appear in supporting documentation, and that the name of Red Hat
> >>>>> + * not be used in advertising or publicity pertaining to distribution
> >>>>> + * of the software without specific, written prior permission. Red
> >>>>> + * Hat makes no representations about the suitability of this software
> >>>>> + * for any purpose. It is provided "as is" without express or implied
> >>>>> + * warranty.
> >>>>> + *
> >>>>> + * THE AUTHORS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
> >>>>> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
> >>>>> + * NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> >>>>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
> >>>>> + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
> >>>>> + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> >>>>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> >>>>> + *
> >>>>> + */
> >>>>> +
> >>>>> +#include <argp.h>
> >>>>> +#include <config.h>
> >>>>> +#include <fcntl.h>
> >>>>> +#include <stdio.h>
> >>>>> +#include <stdlib.h>
> >>>>> +#include <string.h>
> >>>>> +#include <sys/types.h>
> >>>>> +#include <sys/stat.h>
> >>>>> +#include <sys/ioctl.h>
> >>>>> +#include <sys/unistd.h>
> >>>>> +
> >>>>> +#include <linux/videodev2.h>
> >>>>> +
> >>>>> +#include "../libmedia_dev/get_media_devices.h"
> >>>>> +
> >>>>> +const char *argp_program_version = "v4l2-get-device " V4L_UTILS_VERSION;
> >>>>> +const char *argp_program_bug_address = "Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>";
> >>>>> +
> >>>>> +struct args {
> >>>>> + const char *driver;
> >>>>> + unsigned int device_caps;
> >>>>> +};
> >>>>> +
> >>>>> +static const struct argp_option options[] = {
> >>>>> + {"driver", 'd', "DRIVER", 0, "device driver name", 0},
> >>>>> + {"v4l2-device-caps", 'c', "CAPS", 0, "v4l2 device capabilities", 0},
> >>>>
> >>>> I'd like a bus-info option as well, if possible.
> >>>>
> >>>
> >>> Guess that works.
> >>>
> >>>>> + { 0 }
> >>>>> +};
> >>>>> +
> >>>>> +static unsigned int parse_capabilities(const char *arg)
> >>>>> +{
> >>>>> + char *s, *str = strdup(arg);
> >>>>> + unsigned int caps = 0;
> >>>>> +
> >>>>> + s = strtok (str,",");
> >>>>> + while (s != NULL) {
> >>>>> + if (!strcmp(s, "V4L2_CAP_VIDEO_CAPTURE"))
> >>>>> + caps |= V4L2_CAP_VIDEO_CAPTURE;
> >>>>> + else if (!strcmp(s, "V4L2_CAP_VIDEO_OUTPUT"))
> >>>>> + caps |= V4L2_CAP_VIDEO_OUTPUT;
> >>>>> + else if (!strcmp(s, "V4L2_CAP_VIDEO_OVERLAY"))
> >>>>> + caps |= V4L2_CAP_VIDEO_OVERLAY;
> >>>>> + else if (!strcmp(s, "V4L2_CAP_VBI_CAPTURE"))
> >>>>> + caps |= V4L2_CAP_VBI_CAPTURE;
> >>>>> + else if (!strcmp(s, "V4L2_CAP_VBI_OUTPUT"))
> >>>>> + caps |= V4L2_CAP_VBI_OUTPUT;
> >>>>
> >>>> Let's support all CAPS here. I'd also drop the V4L2_CAP_ prefix (or make it optional)
> >>>> and make the strcmp case-insensitive to ease typing.
> >>>>
> >>>
> >>> OK.
> >>>
> >>>>> + s = strtok (NULL, ",");
> >>>>> + }
> >>>>> + free(str);
> >>>>> + return caps;
> >>>>> +}
> >>>>> +
> >>>>> +static error_t parse_opt(int k, char *arg, struct argp_state *state)
> >>>>> +{
> >>>>> + struct args *args = state->input;
> >>>>> +
> >>>>> + switch (k) {
> >>>>> + case 'd':
> >>>>> + args->driver = arg;
> >>>>> + break;
> >>>>> + case 'c':
> >>>>> + args->device_caps = parse_capabilities(arg);
> >>>>> + break;
> >>>>> + default:
> >>>>> + return ARGP_ERR_UNKNOWN;
> >>>>> + }
> >>>>> + return 0;
> >>>>> +}
> >>>>> +
> >>>>> +static struct argp argp = {
> >>>>> + .options = options,
> >>>>> + .parser = parse_opt,
> >>>>> +};
> >>>>> +
> >>>>> +int main(int argc, char *argv[])
> >>>>> +{
> >>>>> + const char *vid;
> >>>>> + struct args args;
> >>>>> + void *md;
> >>>>> +
> >>>>> + args.driver = NULL;
> >>>>> + args.device_caps = 0;
> >>>>> + argp_parse(&argp, argc, argv, 0, 0, &args);
> >>>>> +
> >>>>> + md = discover_media_devices();
> >>>>
> >>>> I never really liked this. My main problem is that it doesn't know about media devices.
> >>>
> >>> Sorry, not sure what you don't really like?
> >>>
> >>> The discover_media_devices() is maybe not the best API, but it's what v4l2-sysfs-path
> >>> uses, and it seemed to fit what we need as a first step (to find v4l2 devices).
> >>>
> >>> Perhaps we can use this for now and pospone improving the discovery library?
> >>
> >> Yes, that can be postponed (I should have been clear about that, sorry).
> >>
> >>>> In my view it should look for media devices first, query them and get all the device
> >>>> nodes referenced in the topology, and then fall back to the old method to discover
> >>>> any remaining device nodes for drivers that do not create a media device.
I agree with Hans here. It's about time to make new development media
device-centric, and only fall back to legacy methods for legacy drivers.
> >>>> You need media device support anyway since you also want to be able to query the media
> >>>> device for a specific driver and find the device node for a specific entity.
> >>>
> >>> I've been thinking about this (since I started with http://git.ideasonboard.org/media-enum.git),
> >>> but to be honest, I failed to see why I would want to query media devices.
> >>>
> >>> Let's say we want to find a H264 decoder, I don't think the topology is be
> >>> of much use, is it?
> >>
> >> Not for codecs, but this should become a generic utility that you can also use to find
> >> e.g. v4l-subdev device nodes from a complex camera driver.
> >
> > Ah, that makes sense.
> >
> > Thanks for the quick review!
> >
> >>> In any case, I think this is part of a bigger discussion, would you consider merging
> >>> v4l discovery for now?
> >>>
> >>>>> +
> >>>>> + vid = NULL;
> >>>>> + do {
> >>>>> + struct v4l2_capability cap;
> >>>>> + char devnode[64];
> >>>>> + int ret;
> >>>>> + int fd;
> >>>>> +
> >>>>> + vid = get_associated_device(md, vid, MEDIA_V4L_VIDEO,
> >>>>> + NULL, NONE);
> >>>>> + if (!vid)
> >>>>> + break;
> >>>>> + snprintf(devnode, 64, "/dev/%s", vid);
> >>>>> + fd = open(devnode, O_RDWR);
> >>>>> + if (fd < 0)
> >>>>> + continue;
> >>>>> +
> >>>>> + memset(&cap, 0, sizeof cap);
> >>>>> + ret = ioctl(fd, VIDIOC_QUERYCAP, &cap);
> >>>>> + if (ret) {
> >>>>> + close(fd);
> >>>>> + continue;
> >>>>> + }
> >>>>> + close(fd);
> >>>>> +
> >>>>> + if (strncmp(args.driver, (char *)cap.driver, sizeof(cap.driver)))
> >>>>> + continue;
> >>>>> + if (args.device_caps && (args.device_caps & cap.device_caps) != args.device_caps)
> >>>>> + continue;
> >>>>> + fprintf(stdout, "%s\n", devnode);
> >>>>> + } while (vid);
> >>>>> + free_media_devices(md);
> >>>>> + return 0;
> >>>>> +}
> >>>>> diff --git a/v4l-utils.spec.in b/v4l-utils.spec.in
> >>>>> index 67bdca57ae92..ab15286b039b 100644
> >>>>> --- a/v4l-utils.spec.in
> >>>>> +++ b/v4l-utils.spec.in
> >>>>> @@ -159,6 +159,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> >>>>> %{_bindir}/ivtv-ctl
> >>>>> %{_bindir}/v4l2-ctl
> >>>>> %{_bindir}/v4l2-sysfs-path
> >>>>> +%{_bindir}/v4l2-get-device
> >>>>> %{_mandir}/man1/ir-keytable.1*
> >>>>> %{_mandir}/man1/ir-ctl.1*
--
Regards,
Laurent Pinchart
