| Message ID | 20230808075538.3043934-6-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
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@linux.intel.com> > --- > 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 ?) > + * @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, }; > + * @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 > */ > -- > 2.39.2 >
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@linux.intel.com> > > --- > > 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 > > */
On Mon, Aug 14, 2023 at 10:23:59AM +0000, Sakari Ailus wrote: > On Thu, Aug 10, 2023 at 05:19:02PM +0200, Jacopo Mondi wrote: > > 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@linux.intel.com> > > > --- > > > 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. ycbcr_enc and hsv_enc are stored in an unnamed union. > > > + * @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 > > > */
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 + * @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 + * @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 */
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@linux.intel.com> --- include/uapi/linux/v4l2-mediabus.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)