On 5/2/19 2:55 PM, Mauro Carvalho Chehab wrote: > Em Fri, 12 Apr 2019 18:59:15 +0300 > Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> escreveu: > >> 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 | 13 ++++++++++++- >> Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 11 ++++++++++- >> 2 files changed, 22 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..005428a8121e 100644 >> --- a/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst >> +++ b/Documentation/media/uapi/v4l/pixfmt-v4l2-mplane.rst >> @@ -31,7 +31,18 @@ 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. >> + >> + 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. >> * - __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..0f7771151db9 100644 >> --- a/Documentation/media/uapi/v4l/pixfmt-v4l2.rst >> +++ b/Documentation/media/uapi/v4l/pixfmt-v4l2.rst >> @@ -89,7 +89,16 @@ 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. >> + >> + 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. > > It is very confusing to understand what you meant by the above paragraph, > as you inverted the sentence order and forgot a comma. > > I would, instead, write the phrases using the direct order, and break > into two paragraphs, e. g., changing the above to: > > "The driver will set the value for uncompressed images. > > Clients are allowed to set the sizeimage field for variable length > compressed data, 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." > > That makes it a lot easier to read, hopefully preventing mistakes from > app and driver developers when reading about sizeimage. > > Yet, I'm not too comfortable on letting this too generic. I mean, > how an app writer would know what formats are "variable length > compressed data", specially since libv4l may actually change that. It's actually quite clearly defined: compressed formats set the V4L2_FMT_FLAG_COMPRESSED flag in VIDIOC_ENUMFMT. Also bytesperline will be 0 for compressed formats. Regards, Hans > > Thanks, > Mauro >