Em 19-09-2010 18:10, Hans Verkuil escreveu: > On Sunday, September 19, 2010 22:20:18 Mauro Carvalho Chehab wrote: >> Em 19-09-2010 16:06, Hans Verkuil escreveu: >>> On Sunday, September 19, 2010 20:29:58 Mauro Carvalho Chehab wrote: >>>> Em 19-09-2010 11:58, Hans Verkuil escreveu: >>>>> On Sunday, September 19, 2010 13:43:43 Mauro Carvalho Chehab wrote: >>>> >>> Multiple stream per device node: if you are talking about allowing e.g. both VBI streaming >>> and video streaming from one device node, then that is indeed allowed by the current spec. >>> Few drivers support this though, and it is a really bad idea. During the Helsinki meeting we >>> agreed to remove this from the spec (point 10a in the mini summit report). >> >> I'm talking about read(), overlay and mmap(). The spec says, at "Multiple Opens" [1]: >> "When a device supports multiple functions like capturing and overlay simultaneously, >> multiple opens allow concurrent use of the device by forked processes or specialized applications." >> >> [1] http://linuxtv.org/downloads/v4l-dvb-apis/ch01.html#id2717880 >> >> So, it is allowed by the spec. What is forbidden is to have some copy logic in kernel to duplicate data. > > There is no streaming involved with overlays, is there? It is all handled in the driver and > userspace just tells the driver where the window is. I may be wrong, I'm much more familiar > with output overlays (OSD). Are overlays actually still working anywhere these days? It depends on what you call streaming. On overlay mode, there's a PCI2PCI transfer stream, from video capture memory into the video adapter memory. It is still a stream, even though it is not "visible" after started. >> Besides that, not all device drivers will work with all applications or provide the complete set of >> functionalities. For example, there are only three drivers (ivtv, cx18 and pvrusb2), as far as I remember, >> that only implements read() method. By using your logic that "only a few drivers allow feature X", maybe >> we should deprecate read() as well ;) > > There's nothing wrong with read. But reading while streaming at the same time from the same source, > that's another matter. You may not like its implementation, but it is currently in use, and there's nothing at spec forbidding it. > And I'm hoping that vb2 will make it possible to implement streaming in > ivtv and cx18. Ok. That's another reason why we should lock poll/read. > <snip> > >>>> The problem with the current implementation of v4l2_fh() is that there's no way for the core >>>> to get per-fh info. >>> >>> You mean how to get from a struct file to a v4l2_fh? That should be done through >>> filp->private_data, but since many driver still put other things there, that is not >>> really usable at the moment without changing all those drivers first. >> >> It should be drivers decision to put whatever they want on a "priv_data". If you want to have >> core data, then you need to use embeed for the core, but keeping priv_data for private driver's >> internal usage. That's the standard way used on Linux. You're doing just the reverse ;) > > I don't follow your reasoning here. What kernel does, in general, is to use a "class inheritance" by embeding one struct into another. This is used mainly on the core structs. For drivers, it provides void *priv data for internal driver-only usage. The way you're wanting to do with v4l2_fh is just the reverse: use priv_data for core usage, and embed struct for the drivers. >>> This actually will work correctly. When a device node is registered in cx88, it is already >>> hooked into the v4l2_device of the core struct. This was needed to handle the i2c subdevs >>> in the right place: the cx88 core struct. So refcounting will also be done in the core struct. >> >> No. Look at the actual code. For example, this is what cx88-mpeg does: >> >> struct cx8802_dev *dev = pci_get_drvdata(pci_dev); >> >> cx88 core is at dev->core. >> >> The same happens with cx88-video, using struct cx8800: >> >> struct cx8800_dev *dev = pci_get_drvdata(pci_dev); >> >> cx88 core is also at dev->core. >> >> This device is implemented using multiple PCI devices, one for each function. Function 0 (video) and Function 2 >> (used for TS devices, like mpeg encoders) can be used independently, but there are some data that are concurrent. >> So, drivers will likely need to use two locks, one for the core and one for the function. > > I was talking about refcounting in cx88 using my patch, not locking. Locking in > cx88 will almost certainly need two locks. Depending on the way the cx88 core lock will be implemented, you may need to unlock it before release. I just arguing that it needs to take more care/review at cx88, in order to avoid having a dead lock there. Cheers, Mauro -- 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
