On Fri September 21 2012 18:47:54 Sylwester Nawrocki wrote: > On 09/21/2012 06:23 PM, Hans Verkuil wrote: > > On Fri September 21 2012 18:13:20 Sylwester Nawrocki wrote: > >> Hi Hans, > >> > >> On 09/19/2012 04:37 PM, Hans Verkuil wrote: > >>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > >>> > >>> length should be set to num_planes in __fill_v4l2_buffer(). That way the > >>> caller knows how many planes there are in the buffer. > >>> > >>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > >> > >> I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes. > >> Consider a use case where device is streaming with 2-planar pixel format > >> and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single > >> queue there will be buffers with different number of planes. The number of > >> planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF > >> won't work. > > > > That's a very good point and one I need to meditate on. > > > > However, your comment applies to patch 1/6, not to this one. > > This patch is about whether or not the length field of v4l2_buffer should > > be filled in with the actual number of planes used by that buffer or not. > > Yes, right. Sorry, I was editing response to multiple patches from this > series and have mixed things a bit. I agree that it is logical and expected > to update struct v4l2_buffer for user space. OK, great. That's was actually the main reason for working on this as this unexpected behavior bit me when writing mplane streaming support for v4l2-ctl. > I have spent some time on this series, and even prepared a patch for s5p-mfc, > as it relies on num_planes being in struct vb2_buffer. But then a realized > there could be buffers with distinct number of planes an a single queue. I'll get back to you about this, probably on Monday. I need to think about the implications of this. Regards, Hans -- 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
