On 16/08/2023 19:48, Sakari Ailus wrote: > Hi Hans, Laurent, > > On Wed, Aug 16, 2023 at 05:34:32PM +0300, Laurent Pinchart wrote: >> Hi Hans, >> >> Thank you for the patch. >> >> On Wed, Aug 16, 2023 at 02:47:33PM +0200, Hans Verkuil wrote: >>> Currently if VIDIOC_CREATE_BUFS is called to add new buffers, then the requested >>> number of planes per buffer might be different from the already allocated buffers. >>> >>> However, this does not make a lot of sense and there are no drivers that allow >>> for variable number of planes in the allocated buffers. >>> >>> While it is possible do this today, when such a buffer is queued it will fail >>> in the buf_prepare() callback where the plane sizes are checked if those are >>> large enough. If fewer planes were allocated, then the size for the missing >>> planes are 0 and the check will return -EINVAL. >>> >>> But it is much better to do this check in VIDIOC_CREATE_BUFS. >> >> I don't think this is a good idea. One important use case for >> VIDIOC_CREATE_BUFS is to allocate buffers for a different format than >> the one currently configured for the device, to prepare for a future >> capture (or output) session with a different configuration. This patch >> would prevent that. > > I'd prefer to keep this capability in videobuf2, too. Although... one way > achieve that could be to add a flag (or an integer field) in struct > vb2_queue to tell vb2 core that the driver wants to do the num_planes > checks by itself. Having a flag for this was something I intended to do once we have a driver that actually supports this feature. I'm not aware of any driver that needs this today. But I'll make a v2 adding this flag, it is simple to do and doesn't hurt. > > It'd be also nicer, considering the end result, to configure this when > setting up the queue, rather than based on the first buffer created. This > would involve changing a large number of drivers though. > "to configure this": what is "this"? The new flag? Or the number of planes? And with "setting up the queue" do you mean the queue_setup callback or when the vb2_queue is initialized? Regards, Hans
