Hello Shuah, >> What about G_INPUT and G_TUNER? Consider the following use case, which is >> entirely legal in the V4L2 API: > > Did you mean G_INPUT and G_STD here? I didn't see G_TUNER mentioned > below in the use-case. It can be either ENUM_INPUT or G_TUNER. Both return status information that requires communication with the video decoder chip and/or tuner. It's probably worth mentioning that ENUMINPUT isn't like the other ENUM_ calls in that it doesn't return a static list without talking to the driver - it contains a field (called .status) which actually needs to talk to the hardware in order to populate it. >> 1. Program opens /dev/video0 >> 2. Program calls G_INPUT/G_STD and sees that the appropriate input and >> standard are already set, since all devices have a default input at >> initialization >> 3. Program never calls S_INPUT, S_STD, or S_TUNER >> 4. Program goes into a loop calling ENUM_INPUT, waiting until it returns >> the input as having signal lock >> 5. When signal lock is seen, program calls STREAMON. > > I am missing vb2 streamon change to hold the tuner in this patch set. > Without that change vb2 work isn't complete. Unfortunately I don't > have hybrid hardware that uses a vb2 driver. I don't think you quite understood my concern. The concern is that in the use case above I'm actively using the tuner *before* VIDIOC_STREAMON is called. Hence from a locking standpoint you probably don't want to allow the DVB device to take control of the tuner. >> >> In the above case, you would be actively using the au8522 video decoder but >> not holding the lock, so thr DVB device can be opened and screw everything >> up. Likewise if the DVB device were in use and such a program were run, it >> wouls break. >> > > I think this use-case will be covered with changes to vb2 streamon > to check and hold tuner. I am thinking it might not be necessary to > change g_tuner, g_std, g_input and enum_input at v4l2-core level. > Does that sounds right?? The more I think about it, the less confident I am that you actually can take a fine-grained locking approach without adding additional ioctls to make it explicit. When is the tuner unlocked? Is it when the filehandle is closed? If so, then the the following script would behave in an unexpected manner: #!/bin/sh while [ 1 ]; do v4l2-ctl -n # Some code to parse the output and see if the "status" field for current input shows no signal # If status shows as locked break done v4l2-ctl --stream-mmap=500 --stream-to=/tmp/foo.bin In the above case I'm actively using the tuner but not holding the lock most of the time, so a separate process can grab the DVB device between calls to "v4l2-ctl -n". However if you're keeping the device locked until STREAMOFF, then you'll break all sorts of applications which might just close the filehandle without calling STREAMOFF, and hence you'll have cases where the tuner is left locked in analog mode *forever*, preventing apps from using it in digital mode. Without adding a new ioctl to lock/unlock the analog side of the device, there is no real way to deal with this perfectly legal use case. The downside of that of course is that applications would have to be modified in order for the locking to be used, and the default would have to be to not do locking in order to preserve backward compatibility with existing applications. What other ioctls have we not thought of? I think there is an argument to be made that we're being too aggressive in trying to control the locking based on the ioctls called. It might make sense to simplify the approach to lock on when the device is opened, and unlock when closed. This avoids the complexity of trying to figure out *which* ioctls we need to set the lock on (which likely varies on a per device basis anyway), at the cost of not allowing the device to be used when something has the filehandle opened for the other side of the device (the behavior of which is currently undefined in the spec). I know there are concerns that some apps might leave the FD open even when done using it, but that seems like a less likely case than properly handling fine-grained locking (causing unexpected behavior for the applications that don't expect -EBUSY to be returned after the device has been successfully opened). We can always start with coarse locking on open/close, and do finer grained locking down the road if needed - or simply change the currently undefined behavior in the spec to say that you have to close the device handle before attempting to open the other side of the device. On a related note, you should be testing with MythTV - none of the applications you are currently testing with support both analog *and* digital, so you are not seeing all the race conditions that can occur when you close one side of the device and then *immediately* open the other side. In particular, there is a known race that occurs when closing the DVB device and then opening the V4L device, because the DVB frontend shuts down the tuner asynchronously after the close() call returns to userland. Exiting one application and starting another provides plenty of time for the close() logic to be run, so you're missing all the race conditions. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
