On Wed, May 8, 2019 at 10:58 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
> Hi Boris,
>
> On 4/12/19 10:57 AM, Boris Brezillon wrote:
> > On Thu, 4 Apr 2019 10:16:56 +0200
> > Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote:
> >
> >
> >> +/**
> >> + * struct v4l2_ext_buffer - extended video buffer info
> >> + * @index: id number of the buffer
> >> + * @type: enum v4l2_buf_type; buffer type. _MPLANE and _OVERLAY formats are
> >> + * invalid
> >> + * @flags: buffer informational flags
> >> + * @field: enum v4l2_field; field order of the image in the buffer
> >> + * @timestamp: frame timestamp
> >> + * @sequence: sequence count of this frame
> >> + * @memory: enum v4l2_memory; the method, in which the actual video data is
> >> + * passed
> >> + * @planes: per-plane buffer information
> >> + * @num_planes: number of plane buffers
> >> + * @request_fd: fd of the request that this buffer should use
> >> + * @reserved: some extra space reserved to add future fields (like timecode).
> >> + * Must be set to 0
> >> + *
> >> + * Contains data exchanged by application and driver using one of the Streaming
> >> + * I/O methods.
> >> + */
> >> +struct v4l2_ext_buffer {
> >> + __u32 index;
> >> + __u32 type;
> >> + __u32 flags;
> >> + __u32 field;
> >> + __u64 timestamp;
> >> + __u32 sequence;
> >> + __u32 memory;
> >> + struct v4l2_ext_plane planes[VIDEO_MAX_PLANES];
> >
> > I had a discussion with Tomasz last week, and he made me realize I was
> > misunderstanding the concept of V4L2 planes. I thought it was encoding
> > pixel-component planes, but it's actually memory planes, and sometimes
> > those one memory planes might contain all component planes placed next
> > to each others (like the V4L2_PIX_FMT_NV12 format).
> >
> > So, the question is, what do we want v4l2_ext_plane to encode (memory
> > planes or pixel component planes)?
> > If we go for the pixel component plane approach (which IMHO would be a
> > good thing), that means we'll have to convert V4L2_PIX_FMT_NV12-like
> > single-memory-plane buffers into v4l2_ext_buffer containing X planes,
> > each pointing to the same memory object but at a different offset.
>
> First of all my apologies for the long delay in replying.
>
> I think v4l2_ext_plane should encode pixel component planes, that way
> it becomes much easier to describe e.g. NV12 formats that use a single
> memory range, but where each component plane has its own bytesperline
> value and where each component plane starts at e.g. a page boundary
> due to hardware restrictions.
>
> This is currently impossible to describe without creating a new pixel
> format.
Agreed with the above.
>
> But it is of course possible that different component planes use
> different memory ranges.
>
> I think that the memory information in v4l2_ext_plane should describe the
> memory for that component plane and any following component planes that
> are part of that memory. The memory information for those following
> component planes should be 0 or some other value that makes it clear
> that it is part of the same memory buffer as the preceding component plane.
That could be an option too, but there are also cases where the
userspace has no idea to know if all the memory buffers are the same,
e.g. when it gets dup()ed DMA-buf FDs for all the color planes.
We should let the userspace set file descriptors for all the color
planes and then the framework figure out if that matches the driver
(or format) expectations.
Best regards,
Tomasz
