Hi Jacopo,
Thanks for the review.
On Thu, Aug 10, 2023 at 05:19:02PM +0200, Jacopo Mondi wrote:
> Hi Sakari
>
> On Tue, Aug 08, 2023 at 10:55:33AM +0300, Sakari Ailus wrote:
> > Now that metadata mbus formats have been added, it is necessary to define
> > which fields in struct v4l2_mbus_format are applicable to them (not many).
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > include/uapi/linux/v4l2-mediabus.h | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
> > index 6b07b73473b5..3cadb3b58b85 100644
> > --- a/include/uapi/linux/v4l2-mediabus.h
> > +++ b/include/uapi/linux/v4l2-mediabus.h
> > @@ -19,12 +19,18 @@
> > * @width: image width
> > * @height: image height
> > * @code: data format code (from enum v4l2_mbus_pixelcode)
> > - * @field: used interlacing type (from enum v4l2_field)
> > - * @colorspace: colorspace of the data (from enum v4l2_colorspace)
> > - * @ycbcr_enc: YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
> > - * @hsv_enc: HSV encoding of the data (from enum v4l2_hsv_encoding)
> > - * @quantization: quantization of the data (from enum v4l2_quantization)
> > - * @xfer_func: transfer function of the data (from enum v4l2_xfer_func)
> > + * @field: used interlacing type (from enum v4l2_field), not applicable
> > + * to metadata mbus codes
>
> "not applicable" is a bit geeric. Should this be set to
> V4L2_FIELD_NONE (for metadata, and progressive image formats maybe ?)
I actually intended to have the same wording here than for the other fields
but missed changing this.
0 corresponds to V4L2_FIELD_ANY.
>
> > + * @colorspace: colorspace of the data (from enum v4l2_colorspace), zero on
> > + * metadata mbus codes
> > + * @ycbcr_enc: YCbCr encoding of the data (from enum v4l2_ycbcr_encoding), zero
> > + * on metadata mbus codes
> > + * @hsv_enc: HSV encoding of the data (from enum v4l2_hsv_encoding), zero on
> > + * metadata mbus codes
>
> Can this be zero ?
>
> enum v4l2_hsv_encoding {
>
> /* Hue mapped to 0 - 179 */
> V4L2_HSV_ENC_180 = 128,
>
> /* Hue mapped to 0-255 */
> V4L2_HSV_ENC_256 = 129,
> };
Good question. Neither value is meaningful for metadata.
The values appear to be such in the enum to avoid colliding with ycbcr
encoding values (another field above).
Generally, what doesn't matter will be zero. These fields have been added
at some point and a lot of drivers do not set them, even for pixel data.
I wonder what Hans and Laurent think.
>
> > + * @quantization: quantization of the data (from enum v4l2_quantization), zero
> > + * on metadata mbus codes
> > + * @xfer_func: transfer function of the data (from enum v4l2_xfer_func), zero
> > + * on metadata mbus codes
> > * @flags: flags (V4L2_MBUS_FRAMEFMT_*)
> > * @reserved: reserved bytes that can be later used
> > */
--
Kind regards,
Sakari Ailus
