| Message ID | 20201030134609.30020-1-dafna.hirschfeld@collabora.com |
|---|---|
| State | Accepted |
| Commit | 6ad253cc3436269fc6bcff03d704c672f368da0a |
| Headers | show |
| Series | [v3,1/2] media: uapi: add MEDIA_BUS_FMT_METADATA_FIXED media bus format. | expand |
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@collabora.com> > Acked-by: Helen Koike <helen.koike@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. > +#define MEDIA_BUS_FMT_METADATA_FIXED 0x7001 > + > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */ -- Kind regards, Sakari Ailus
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@collabora.com> >> Acked-by: Helen Koike <helen.koike@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 */ >
Hi Hans, On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: > 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@collabora.com> > >> Acked-by: Helen Koike <helen.koike@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. Let's say the metadata is laid out in a similar way than an image; you have lines of data, followed by some padding at the end, and a line has length and a buffer has a number of lines. Sensor metadata falls into this description. Would you then use struct v4l2_pix_format for it, and use V4L2_BUF_TYPE_VIDEO_CAPTURE for it? That would make a few things easier but this is still metadata, not video data. Albeit I guess it's not important to be so strict about that interface-wise, indeed this is not a bad fit for such metadata. Still some fields such as colourspace and quantisation are not relevant, but that holds also for some pixel formats.
On 04/11/2020 13:32, Sakari Ailus wrote: > Hi Hans, > > On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: >> 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@collabora.com> >>>> Acked-by: Helen Koike <helen.koike@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. > > Let's say the metadata is laid out in a similar way than an image; you have > lines of data, followed by some padding at the end, and a line has length > and a buffer has a number of lines. Sensor metadata falls into this > description. > > Would you then use struct v4l2_pix_format for it, and use > V4L2_BUF_TYPE_VIDEO_CAPTURE for it? Yes. It's still metadata, but it has the same 'format' as video data. We have similar situations such as with v4l-touch devices: the data is formatted like video, but it is actually pressure values from a touch pad. But it is 'video-like' in its behavior. > > That would make a few things easier but this is still metadata, not video > data. Albeit I guess it's not important to be so strict about that > interface-wise, indeed this is not a bad fit for such metadata. Still some > fields such as colourspace and quantisation are not relevant, but that > holds also for some pixel formats. > So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? Regards, Hans
Hi Hans, On Wed, Nov 04, 2020 at 02:46:39PM +0100, Hans Verkuil wrote: > On 04/11/2020 13:32, Sakari Ailus wrote: > > Hi Hans, > > > > On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: > >> 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@collabora.com> > >>>> Acked-by: Helen Koike <helen.koike@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. > > > > Let's say the metadata is laid out in a similar way than an image; you have > > lines of data, followed by some padding at the end, and a line has length > > and a buffer has a number of lines. Sensor metadata falls into this > > description. > > > > Would you then use struct v4l2_pix_format for it, and use > > V4L2_BUF_TYPE_VIDEO_CAPTURE for it? > > Yes. It's still metadata, but it has the same 'format' as video data. > We have similar situations such as with v4l-touch devices: the data > is formatted like video, but it is actually pressure values from a > touch pad. But it is 'video-like' in its behavior. > > > > > That would make a few things easier but this is still metadata, not video > > data. Albeit I guess it's not important to be so strict about that > > interface-wise, indeed this is not a bad fit for such metadata. Still some > > fields such as colourspace and quantisation are not relevant, but that > > holds also for some pixel formats. > > > > So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? One more question. What do you do if a link can carry both metadata (as in V4L2_BUF_TYPE_METADATA_CAPTURE) as well as pixel data, but with a fixed format? I'm not sure we have any such case at the moment but it's not inconceivable.
On 04/11/2020 15:54, Sakari Ailus wrote: > Hi Hans, > > On Wed, Nov 04, 2020 at 02:46:39PM +0100, Hans Verkuil wrote: >> On 04/11/2020 13:32, Sakari Ailus wrote: >>> Hi Hans, >>> >>> On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: >>>> 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@collabora.com> >>>>>> Acked-by: Helen Koike <helen.koike@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. >>> >>> Let's say the metadata is laid out in a similar way than an image; you have >>> lines of data, followed by some padding at the end, and a line has length >>> and a buffer has a number of lines. Sensor metadata falls into this >>> description. >>> >>> Would you then use struct v4l2_pix_format for it, and use >>> V4L2_BUF_TYPE_VIDEO_CAPTURE for it? >> >> Yes. It's still metadata, but it has the same 'format' as video data. >> We have similar situations such as with v4l-touch devices: the data >> is formatted like video, but it is actually pressure values from a >> touch pad. But it is 'video-like' in its behavior. >> >>> >>> That would make a few things easier but this is still metadata, not video >>> data. Albeit I guess it's not important to be so strict about that >>> interface-wise, indeed this is not a bad fit for such metadata. Still some >>> fields such as colourspace and quantisation are not relevant, but that >>> holds also for some pixel formats. >>> >> >> So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? > > One more question. > > What do you do if a link can carry both metadata (as in > V4L2_BUF_TYPE_METADATA_CAPTURE) as well as pixel data, but with a fixed > format? > > I'm not sure we have any such case at the moment but it's not > inconceivable. > This should be reflected in the mediabus format. So METADATA_FIXED if it carries metadata, or a video format if it carries video. Userspace could configure this with VIDIOC_SUBDEV_S_FMT if it is something that userspace can actually change. Regards, Hans
On Thu, Nov 05, 2020 at 01:35:00PM +0100, Hans Verkuil wrote: > On 04/11/2020 15:54, Sakari Ailus wrote: > > Hi Hans, > > > > On Wed, Nov 04, 2020 at 02:46:39PM +0100, Hans Verkuil wrote: > >> On 04/11/2020 13:32, Sakari Ailus wrote: > >>> Hi Hans, > >>> > >>> On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: > >>>> 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@collabora.com> > >>>>>> Acked-by: Helen Koike <helen.koike@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. > >>> > >>> Let's say the metadata is laid out in a similar way than an image; you have > >>> lines of data, followed by some padding at the end, and a line has length > >>> and a buffer has a number of lines. Sensor metadata falls into this > >>> description. > >>> > >>> Would you then use struct v4l2_pix_format for it, and use > >>> V4L2_BUF_TYPE_VIDEO_CAPTURE for it? > >> > >> Yes. It's still metadata, but it has the same 'format' as video data. > >> We have similar situations such as with v4l-touch devices: the data > >> is formatted like video, but it is actually pressure values from a > >> touch pad. But it is 'video-like' in its behavior. > >> > >>> > >>> That would make a few things easier but this is still metadata, not video > >>> data. Albeit I guess it's not important to be so strict about that > >>> interface-wise, indeed this is not a bad fit for such metadata. Still some > >>> fields such as colourspace and quantisation are not relevant, but that > >>> holds also for some pixel formats. > >>> > >> > >> So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? > > > > One more question. > > > > What do you do if a link can carry both metadata (as in > > V4L2_BUF_TYPE_METADATA_CAPTURE) as well as pixel data, but with a fixed > > format? > > > > I'm not sure we have any such case at the moment but it's not > > inconceivable. > > > > This should be reflected in the mediabus format. So METADATA_FIXED if it carries > metadata, or a video format if it carries video. Userspace could configure this > with VIDIOC_SUBDEV_S_FMT if it is something that userspace can actually change. Works for me. Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Hi Am 05.11.20 um 13:45 schrieb Sakari Ailus: > On Thu, Nov 05, 2020 at 01:35:00PM +0100, Hans Verkuil wrote: >> On 04/11/2020 15:54, Sakari Ailus wrote: >>> Hi Hans, >>> >>> On Wed, Nov 04, 2020 at 02:46:39PM +0100, Hans Verkuil wrote: >>>> On 04/11/2020 13:32, Sakari Ailus wrote: >>>>> Hi Hans, >>>>> >>>>> On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: >>>>>> 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@collabora.com> >>>>>>>> Acked-by: Helen Koike <helen.koike@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. >>>>> >>>>> Let's say the metadata is laid out in a similar way than an image; you have >>>>> lines of data, followed by some padding at the end, and a line has length >>>>> and a buffer has a number of lines. Sensor metadata falls into this >>>>> description. >>>>> >>>>> Would you then use struct v4l2_pix_format for it, and use >>>>> V4L2_BUF_TYPE_VIDEO_CAPTURE for it? >>>> >>>> Yes. It's still metadata, but it has the same 'format' as video data. >>>> We have similar situations such as with v4l-touch devices: the data >>>> is formatted like video, but it is actually pressure values from a >>>> touch pad. But it is 'video-like' in its behavior. >>>> >>>>> >>>>> That would make a few things easier but this is still metadata, not video >>>>> data. Albeit I guess it's not important to be so strict about that >>>>> interface-wise, indeed this is not a bad fit for such metadata. Still some >>>>> fields such as colourspace and quantisation are not relevant, but that >>>>> holds also for some pixel formats. >>>>> >>>> >>>> So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? >>> >>> One more question. >>> >>> What do you do if a link can carry both metadata (as in >>> V4L2_BUF_TYPE_METADATA_CAPTURE) as well as pixel data, but with a fixed >>> format? >>> >>> I'm not sure we have any such case at the moment but it's not >>> inconceivable. >>> >> >> This should be reflected in the mediabus format. So METADATA_FIXED if it carries >> metadata, or a video format if it carries video. Userspace could configure this >> with VIDIOC_SUBDEV_S_FMT if it is something that userspace can actually change. > > Works for me. > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Hi, There is another patch sent by jmondi that adds csi-2 embedded data format: https://patchwork.kernel.org/project/linux-media/patch/20201102165258.408049-3-jacopo@jmondi.org/ Maybe it worth thinking already how those two patches should fit together. Currently they both try to acquire 0x7001 code. Thanks, Dafna
On Thu, Nov 05, 2020 at 03:21:45PM +0100, Dafna Hirschfeld wrote: > Hi > > Am 05.11.20 um 13:45 schrieb Sakari Ailus: > > On Thu, Nov 05, 2020 at 01:35:00PM +0100, Hans Verkuil wrote: > > > On 04/11/2020 15:54, Sakari Ailus wrote: > > > > Hi Hans, > > > > > > > > On Wed, Nov 04, 2020 at 02:46:39PM +0100, Hans Verkuil wrote: > > > > > On 04/11/2020 13:32, Sakari Ailus wrote: > > > > > > Hi Hans, > > > > > > > > > > > > On Wed, Nov 04, 2020 at 01:16:03PM +0100, Hans Verkuil wrote: > > > > > > > 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@collabora.com> > > > > > > > > > Acked-by: Helen Koike <helen.koike@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. > > > > > > > > > > > > Let's say the metadata is laid out in a similar way than an image; you have > > > > > > lines of data, followed by some padding at the end, and a line has length > > > > > > and a buffer has a number of lines. Sensor metadata falls into this > > > > > > description. > > > > > > > > > > > > Would you then use struct v4l2_pix_format for it, and use > > > > > > V4L2_BUF_TYPE_VIDEO_CAPTURE for it? > > > > > > > > > > Yes. It's still metadata, but it has the same 'format' as video data. > > > > > We have similar situations such as with v4l-touch devices: the data > > > > > is formatted like video, but it is actually pressure values from a > > > > > touch pad. But it is 'video-like' in its behavior. > > > > > > > > > > > > > > > > > That would make a few things easier but this is still metadata, not video > > > > > > data. Albeit I guess it's not important to be so strict about that > > > > > > interface-wise, indeed this is not a bad fit for such metadata. Still some > > > > > > fields such as colourspace and quantisation are not relevant, but that > > > > > > holds also for some pixel formats. > > > > > > > > > > > > > > > > So are you OK with setting width and height to 0 for MEDIA_BUS_FMT_METADATA_*? > > > > > > > > One more question. > > > > > > > > What do you do if a link can carry both metadata (as in > > > > V4L2_BUF_TYPE_METADATA_CAPTURE) as well as pixel data, but with a fixed > > > > format? > > > > > > > > I'm not sure we have any such case at the moment but it's not > > > > inconceivable. > > > > > > > > > > This should be reflected in the mediabus format. So METADATA_FIXED if it carries > > > metadata, or a video format if it carries video. Userspace could configure this > > > with VIDIOC_SUBDEV_S_FMT if it is something that userspace can actually change. > > > > Works for me. > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Hi, > There is another patch sent by jmondi that adds csi-2 embedded data format: > > https://patchwork.kernel.org/project/linux-media/patch/20201102165258.408049-3-jacopo@jmondi.org/ > > Maybe it worth thinking already how those two patches should fit together. > Currently they both try to acquire 0x7001 code. I'll reply to Jacopo. This will at least require discussion, as not all metadata is created equal. -- Sakari Ailus
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. + */ +#define MEDIA_BUS_FMT_METADATA_FIXED 0x7001 + #endif /* __LINUX_MEDIA_BUS_FORMAT_H */