[PATCH v3 1/2] media: uapi: add MEDIA_BUS_FMT_METADATA_FIXED media bus format.

[Hans Verkuil]

On 30/10/2020 15:02, Sakari Ailus wrote:
> Hi Dafna,
> 
> On Fri, Oct 30, 2020 at 02:46:08PM +0100, Dafna Hirschfeld wrote:
>> MEDIA_BUS_FMT_METADATA_FIXED should be used when
>> the same driver handles both sides of the link and
>> the bus format is a fixed metadata format that is
>> not configurable from userspace.
>> The width and height will be set to 0 for this format.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
>> Acked-by: Helen Koike <helen.koike at collabora.com>
>> ---
>> changes since v2:
>> added documentation in subdev-formats.rst
>> changes since v1:
>> 1. replace "This format may have 0 height and width."
>> with "Width and height will be set to 0 for this format."
>> and add it also to the commit log
>> 2. s/meida:/media:/ in the patch subject line
>>
>>  .../media/v4l/subdev-formats.rst              | 27 +++++++++++++++++++
>>  include/uapi/linux/media-bus-format.h         |  8 ++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> index c9b7bb3ca089..7f16cbe46e5c 100644
>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
>> @@ -7899,3 +7899,30 @@ formats.
>>        - 0x5001
>>        - Interleaved raw UYVY and JPEG image format with embedded meta-data
>>  	used by Samsung S3C73MX camera sensors.
>> +
>> +.. _v4l2-mbus-metadata-fmts:
>> +
>> +Metadata Formats
>> +^^^^^^^^^^^^^^^^
>> +
>> +This section lists all metadata formats.
>> +
>> +The following table lists the existing metadata formats.
>> +
>> +.. tabularcolumns:: |p{8.0cm}|p{1.4cm}|p{7.7cm}|
>> +
>> +.. flat-table:: Metadata formats
>> +    :header-rows:  1
>> +    :stub-columns: 0
>> +
>> +    * - Identifier
>> +      - Code
>> +      - Comments
>> +    * .. _MEDIA-BUS-FMT-METADATA-FIXED:
>> +
>> +      - MEDIA_BUS_FMT_METADATA_FIXED
>> +      - 0x7001
>> +      - This format should be used when the same driver handles
>> +	both sides of the link and the bus format is a fixed
>> +	metadata format that is not configurable from userspace.
>> +	Width and height will be set to 0 for this format.
>> diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
>> index 84fa53ffb13f..2ce3d891d344 100644
>> --- a/include/uapi/linux/media-bus-format.h
>> +++ b/include/uapi/linux/media-bus-format.h
>> @@ -156,4 +156,12 @@
>>  /* HSV - next is	0x6002 */
>>  #define MEDIA_BUS_FMT_AHSV8888_1X32		0x6001
>>  
>> +/*
>> + * This format should be used when the same driver handles
>> + * both sides of the link and the bus format is a fixed
>> + * metadata format that is not configurable from userspace.
>> + * Width and height will be set to 0 for this format.
>> + */
> 
> Does this mean that metadata with dimensions should not use
> MEDIA_BUS_FMT_METADATA_FIXED? I guess that's not the intention? For some
> formats the dimensions would be relevant but for others not. I'd thus
> replace "will" by "may". Same for the documentation.

struct v4l2_meta_format as used with VIDIOC_G/S/TRY_FMT doesn't have
a width or height either. Supporting width and height for metadata
doesn't really make sense for me for metadata.

Explicitly specifying the width and height here indicates that the
data is basically an array of width x height of some sort which makes
sense for video devices.

Metadata is more complex data that cannot be represented like that.
If metadata is actually an array, then I'm not sure I would call it
metadata, I would probably see it as video with its own pixelformat
that contains non-video data.

Regards,

	Hans

> 
>> +#define MEDIA_BUS_FMT_METADATA_FIXED		0x7001
>> +
>>  #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
>