lkml.org 
[lkml]   [2019]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] media/doc: Allow sizeimage to be set by v4l clients
From
Date
Hi Mauro,

Thanks for comments!

On 5/2/19 4:29 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 2 May 2019 15:16:54 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
>> On 5/2/19 2:55 PM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 12 Apr 2019 18:59:15 +0300
>>> Stanimir Varbanov <stanimir.varbanov@linaro.org> 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@linaro.org>
>>>> ---
>>>> 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.
>
> Ok, so let's be explicit here, e. g. something like:
>
> "Clients are allowed to set the sizeimage field for variable length
> compressed data flagged with V4L2_FMT_FLAG_COMPRESSED at
> VIDIOC_ENUMFMT, 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 clear for app developers when they can use this new
> feature.

OK, I will resend with that description.

>
> That still leads us to what happens at libv4l with sizeimage
> for a compressed format that got uncompressed by the library, in
> order to ensure that a change like this won't cause breakages at
> existing userspace apps.

libv4l can decompress formats like MJPEG, right? I mean it isn't to
decompress MPEG/H264 for example.

--
--
regards,
Stan

\
 
 \ /
  Last update: 2019-05-07 18:55    [W:0.053 / U:35.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site