Re: [PATCH] media/doc: Allow sizeimage to be set by v4l clients

From: Hans Verkuil
Date: Fri Apr 12 2019 - 06:45:04 EST


On 4/10/19 5:21 PM, Stanimir Varbanov wrote:
> This changes v4l2_pix_format and v4l2_plane_pix_format sizeimage
> field description to allow v4l clients to set bigger image size
> in case of variable length compressed data.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx>
> ---
> Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst | 6 +++++-
> Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 4 +++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst b/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst
> index 5688c816e334..a5cdc4537012 100644
> --- a/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst
> +++ b/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst
> @@ -31,7 +31,11 @@ describing all planes of that format.
>
> * - __u32
> - ``sizeimage``
> - - Maximum size in bytes required for image data in this plane.
> + - Maximum size in bytes required for image data in this plane,
> + set by the driver. When the image consists of variable length
> + compressed data this is the number of bytes required by the
> + codec to support the worst-case compression scenario. Clients
> + are allowed to set the sizeimage field, but drivers may modify it.

How about this:

----------------------
Maximum size in bytes required for image data in this plane,
set by the driver. When the image consists of variable length
compressed data this is the number of bytes required by the
codec to support the worst-case compression scenario.

For uncompressed images the driver will set the value. For
variable length compressed data clients are allowed to set
the sizeimage field, but the driver may ignore it and set the
value itself, or it may modify the provided value based on alignment
requirements or minimum/maximum size requirements. If the client wants
to leave this to the driver, then it should set sizeimage to 0.
-----------------------

I think this is more precise, esp. the last bit that clients can set
it to 0 to have the driver provide the value is important in my view.

Regards,

Hans

> * - __u32
> - ``bytesperline``
> - Distance in bytes between the leftmost pixels in two adjacent
> diff --git a/Documentation/media/uapi/v4l/pixfmt-v4l2.rst b/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
> index 71eebfc6d853..70e08c58febc 100644
> --- a/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
> +++ b/Documentation/media/uapi/v4l/pixfmt-v4l2.rst
> @@ -89,7 +89,9 @@ Single-planar format structure
> - Size in bytes of the buffer to hold a complete image, set by the
> driver. Usually this is ``bytesperline`` times ``height``. When
> the image consists of variable length compressed data this is the
> - maximum number of bytes required to hold an image.
> + number of bytes required by the codec to support the worst-case
> + compression scenario. Clients are allowed to set the sizeimage
> + field, but drivers may modify it.
> * - __u32
> - ``colorspace``
> - Image colorspace, from enum :c:type:`v4l2_colorspace`.
>