On Mon, 15 Aug 2011, Guennadi Liakhovetski wrote: > On Mon, 15 Aug 2011, Hans Verkuil wrote: > > > On Monday, August 15, 2011 13:28:23 Guennadi Liakhovetski wrote: > > > Hi Hans > > > > > > On Mon, 8 Aug 2011, Hans Verkuil wrote: [snip] > > > > but I've changed my mind: I think > > > > this should use a struct v4l2_format after all. > > While switching back, I have to change the struct vb2_ops::queue_setup() > operation to take a struct v4l2_create_buffers pointer. An earlier version > of this patch just added one more parameter to .queue_setup(), which is > easier - changes to videobuf2-core.c are smaller, but it is then > redundant. We could use the create pointer for both input and output. The > video plane configuration in frame format is the same as what is > calculated in .queue_setup(), IIUC. So, we could just let the driver fill > that one in. This would require then the videobuf2-core.c to parse struct > v4l2_format to decide which union member we need, depending on the buffer > type. Do we want this or shall drivers duplicate plane sizes in separate > .queue_setup() parameters? Let me explain my question a bit. The current .queue_setup() method is int (*queue_setup)(struct vb2_queue *q, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); To support multiple-size buffers we also have to pass a pointer to struct v4l2_create_buffers to this function now. We can either do it like this: int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]); and let all drivers fill in respective fields in *create, e.g., either do create->format.fmt.pix_mp.plane_fmt[i].sizeimage = ...; create->format.fmt.pix_mp.num_planes = ...; and also duplicate it in method parameters *num_planes = create->format.fmt.pix_mp.num_planes; sizes[i] = create->format.fmt.pix_mp.plane_fmt[i].sizeimage; or with create->format.fmt.pix.sizeimage = ...; for single-plane. Alternatively we make the prototype int (*queue_setup)(struct vb2_queue *q, struct v4l2_create_buffers *create, unsigned int *num_buffers, void *alloc_ctxs[]); then drivers only fill in *create, and the videobuf2-core will have to check create->format.type to decide, which of create->format.fmt.* is relevant and extract plane sizes from there. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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