Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: sys/uvideo: fill v4l2_capability the right way
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Sat, 28 Dec 2024 22:35:56 +0100

Download raw body.

Thread
  • Marcus Glocker:

    sys/uvideo: fill v4l2_capability the right way

  • On Sat, Dec 28, 2024 at 08:54:03PM GMT, Kirill A. Korinsky wrote:
    
    > On Sat, 28 Dec 2024 20:05:36 +0100,
    > Marcus Glocker <marcus@nazgul.ch> wrote:
    > > 
    > > > Don't you think that this is enough to for the bus?
    > > 
    > > Well, maybe, it's more a question about what one considers a bus
    > > information.  I'm considering "uvideo0" to be a device indentifier, not
    > > necessarily a bus information.
    > >
    > 
    > After some thinking, let put an actual usb bus here, see attached diff.
    > 
    > > > About libwebrtc: it assumes that caps->bus_info is unique, and uses strncmp.
    > > 
    > > OK.  Just to understand, since I don't know how libwebrtc operates;
    > > With what will it compare the caps->bus_info string (lets say it's
    > > "uvideo0") with to make a decision?
    > 
    > It works like this:
    > 
    > 1. An application gets device names and UniqueId which is caps->bus_info by
    >    iterating over /dev/videoX where X between 0 to 63;
    > 
    > 2. User decided which device should be used
    > 
    > 3. It getscCapability map and fd to the desired device. To fine the right
    >    device it iterates over /dev/videoX and comparing caps->bus_info with
    >    expected one, or device name, when it is empty string.
    
    Thanks for explaining.  I think I got it now.  It requires a unique ID
    from bus_info to enumerate the devices.
     
    > Original patch fixes an issue, but here I construct the right bus I think.
    > 
    > Ok?
    
    Given that, I think building the entire USB bus string here is probably
    an overkill for now.  I'm fine if we just pass the enumerated device
    driver name here, like you have proposed in your first patch.
     
    > Index: sys/dev/usb/uvideo.c
    > ===================================================================
    > RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
    > diff -u -p -u -p -r1.231 uvideo.c
    > --- sys/dev/usb/uvideo.c	22 Dec 2024 20:30:04 -0000	1.231
    > +++ sys/dev/usb/uvideo.c	28 Dec 2024 19:44:35 -0000
    > @@ -2928,12 +2928,27 @@ uvideo_debug_file_write_frame(void *arg)
    >  int
    >  uvideo_querycap(void *v, struct v4l2_capability *caps)
    >  {
    > +	int res;
    > +	uint32_t route;
    > +	uint8_t bus, ifaceno;
    >  	struct uvideo_softc *sc = v;
    >  
    >  	bzero(caps, sizeof(*caps));
    > -	strlcpy(caps->driver, DEVNAME(sc), sizeof(caps->driver));
    > +	strlcpy(caps->driver, "uvideo", sizeof(caps->driver));
    >  	strlcpy(caps->card, sc->sc_udev->product, sizeof(caps->card));
    > -	strlcpy(caps->bus_info, "usb", sizeof(caps->bus_info));
    > +	strlcpy(caps->bus_info, DEVNAME(sc), sizeof(caps->bus_info));
    > +
    > +	res = usbd_get_location(sc->sc_udev, sc->sc_vs_cur->ifaceh,
    > +	    &bus, &route, &ifaceno);
    > +
    > +	if (!res)
    > +		res =
    > +		    snprintf(caps->bus_info, sizeof(caps->bus_info),
    > +			"usb%u.%u.%x.%u", bus, route, route, ifaceno)
    > +			>= sizeof(caps->bus_info);
    > +
    > +	if (res && sizeof(caps->bus_info) > 0)
    > +		caps->bus_info[0] = '\0';
    >  
    >  	caps->version = 1;
    >  	caps->device_caps = V4L2_CAP_VIDEO_CAPTURE
    > 
    > 
    > -- 
    > wbr, Kirill
    > 
    
    
    
  • Marcus Glocker:

    sys/uvideo: fill v4l2_capability the right way