Hi Hans, Am 21.01.2013 09:59, schrieb Hans Verkuil: > On Sun January 20 2013 22:15:51 Frank Schäfer wrote: >> Hi Hans, >> >> I noticed that there's code in the v4l2 core that enables/disables >> ioctls and checks some of the parameters depending on the device type. >> While reading the code an comparing it to the V4L2 API document, some >> more questions came up: >> >> 1) Video devices with VBI functionality: >> The spec says: "To address the problems of finding related video and VBI >> devices VBI capturing and output is also available as device function >> under /dev/video". >> Is that still valid ? > No, that's not valid. It really was never valid: most drivers didn't implement > this, and those that did likely didn't work. During one of the media summits > we decided not to allow this. Allowing VBI functionality in video node has a > number of problems: > > 1) it's confusing: why have a vbi node at all if you can do it with a video > node as well? Yeah, although I think the problem described in the spec document is real. No idea how good applications are in finding the correct VBI device belonging to a specific video device node... Hmm... yeah... why have separate device nodes at all ? We could provide the same functionality with a single device node (e.g. /dev/multimediaX). I assume separation into radio / video / vbi device nodes gears towards typical feature sets of applications. Probably something to think about for V4L3... ;) > In fact, applications always use the vbi node for vbi data. > > 2) it breaks down when you want to read() the data: read() can handle only > one 'format' at a time. So if you want to read both video and vbi at the same > time then you need to nodes. Ouch, yes ! Ok, so the traditional read() concept is probably the _real_ reason for having separate device nodes... > 3) it makes drivers quite complex: separating this functionality in distinct > nodes makes life much easier. It looks like the v4l2 core has been improved a lot and now does most of the work for the driver, so it's probably not that complex anymore. >> What about VBI "configuration" (e.g. >> VIDIOC_G/S/TRY_FMT for VBI formats) ? >> Looking into the v4l2 core code, it seems that the VBI buffer types >> (field "type" in struct v4l2_format) are only accepted, if the device is >> a VBI device. > That's correct. I've added these checks because drivers often didn't test > that themselves. It's also much easier to test in the v4l2 core than > repeating the same test in every driver. > >> 2) VIDIOC_G/S/TRY_FMT and VBI devices: >> The sepc says: "VBI devices must implement both the VIDIOC_G_FMT and >> VIDIOC_S_FMT ioctl, even if VIDIOC_S_FMT ignores all requests and always >> returns default parameters as VIDIOC_G_FMT does. VIDIOC_TRY_FMT is >> optional." What's the reason for this ? Is it still valid ? > This is still valid (in fact, v4l2-compliance requires the presence of > TRY_FMT as well as I don't think there is a good reason not to implement > it). The reason for this is that this simplifies applications: no need to > test for the presence of S_FMT. > >> 3) VIDIOC_S_TUNER: field "type" in struct v4l2_tuner >> Are radio tuners accessable through video devices (and the other way >> around) ? > Not anymore. This used to be possible, although because it was never > properly tested most drivers probably didn't implement that correctly. > > The radio API has always been a bit messy and we have been slowly cleaning > it up. Yeah, I think the most confusing things are input/output/routing (G/S_INPUT, G/S_AUDIO). > >> Has this field to be set by the application ? > No, it is filled in by the core based on the device node used. This follows > the spec which does not require apps to set the type. > >> If yes: driver overwrites >> the value or returns with an error if the type doesn't match the tuner >> at the requested index ? >> I wonder if it would make sense to check the tuner type inside the v4l >> core (like the fmt/buffer type check for G/S_PARM). >> >> 4) VIDIOC_DBG_G_CHIP_IDENT: >> Is it part of CONFIG_VIDEO_ADV_DEBUG just like VIDIOC_DBG_G/S_REGISTER ? > No. It just returns some chip info, it doesn't access the hardware, so there > is no need to put it under ADV_DEBUG. Ok. I just noticed that in most drivers it's inside the #ifdef CONFIG_VIDEO_ADV_DEBUG. It also has been renamed from VIDIOC_G_CHIP_IDENT to VIDIOC_DBG_G_CHIP_IDENT which somehow suggests that it's an advanced debug feature. > >> In determine_valid_ioctls(), it is placed outside the #ifdef >> CONFIG_VIDEO_ADV_DEBUG section. >> The spec says "Identify the chips on a TV card" but isn't it suitable >> for all device types (radio/video/vbi) ? > That's correct. A patch is welcome :-) To be sure that I understood you correctly: VIDIOC_DBG_G_CHIP_IDENT is suitable for all device types ? Then no patch is needed but the spec document has to be fixed. >> determine_valid_ioctls() in >> v4l2-dev.c enables it for all devices. >> >> 5) The buffer ioctls (VIDIOC_REQBUFS, VIDIOC_CREATE_BUFS, >> VIDIOC_PREPARE_BUF, VIDIOC_QUERYBUF, VIDIOC_QBUF, VIDIOC_DQBUF) are not >> applicable to radio devices, right ? > That's correct. > >> In function determine_valid_ioctls() in v4l2-dev.c they are enabled for >> all device types. > A patch fixing this is welcome! Coming soon. > >> 6) VIDIOC_G/S_AUDIO: Shouldn't it be disabled in >> determine_valid_ioctls() for VBI devices ? > No. VBI devices still allow this. Strictly speaking it isn't useful for vbi > devices, but allowing G/S_INPUT but not G/S_AUDIO feels inconsistent to me. > > While it isn't useful, it doesn't hurt either. > >> Btw: function determine_valid_ioctls() in v4l2-dev.c is a good summary >> that explains which ioctls are suitable for which device types >> (radio/video/vbi). >> Converting its content into a table would be a great >> extension/clarifaction of the V4L2 API spec document ! > We played around with the idea of 'profiles' where for each type of device > you have a table listing what can and cannot be done. The problem is time... > > If you are interesting in pursuing this, then I am happy to help with advice > and pointers (v4l2-compliance is also a great source of information). I could create a simple html table with X = device type, Y = ioctl. > >> Thanks for your patience ! > My pleasure! > > Regards, > > Hans One last question: I'm looking for a possibility to disable all ioctls when the device is unplugged. I think this is a common problem/task of all drivers for hotpluggable devices, because the disconnect callbacks can't unregister the device until it get's closed by the application. What's the best way to do this ? v4l2_disable_ioctl() has no effect after video_register_device() is called... Regards, Frank -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
