Hi Laurent,
On Thu, Mar 17, 2022 at 04:36:59PM +0200, Laurent Pinchart wrote:
> The helper functions that test validity of colorspace-related fields
> use the last value of the corresponding enums. This isn't very
> future-proof, as there's a high chance someone adding a new value may
> forget to update the helpers. Add new "LAST" entries to the enumerations
> to improve this, and keep them private to the kernel.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> include/media/v4l2-common.h | 10 +++++-----
> include/uapi/linux/videodev2.h | 29 +++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 3eb202259e8c..b686124e2ccf 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -563,19 +563,19 @@ static inline void v4l2_buffer_set_timestamp(struct v4l2_buffer *buf,
> static inline bool v4l2_is_colorspace_valid(__u32 colorspace)
> {
> return colorspace > V4L2_COLORSPACE_DEFAULT &&
> - colorspace <= V4L2_COLORSPACE_DCI_P3;
> + colorspace <= V4L2_COLORSPACE_LAST;
> }
>
> static inline bool v4l2_is_xfer_func_valid(__u32 xfer_func)
> {
> return xfer_func > V4L2_XFER_FUNC_DEFAULT &&
> - xfer_func <= V4L2_XFER_FUNC_SMPTE2084;
> + xfer_func <= V4L2_XFER_FUNC_LAST;
> }
>
> static inline bool v4l2_is_ycbcr_enc_valid(__u8 ycbcr_enc)
> {
> return ycbcr_enc > V4L2_YCBCR_ENC_DEFAULT &&
> - ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M;
> + ycbcr_enc <= V4L2_YCBCR_ENC_LAST;
> }
>
> static inline bool v4l2_is_hsv_enc_valid(__u8 hsv_enc)
> @@ -585,8 +585,8 @@ static inline bool v4l2_is_hsv_enc_valid(__u8 hsv_enc)
>
> static inline bool v4l2_is_quant_valid(__u8 quantization)
> {
> - return quantization == V4L2_QUANTIZATION_FULL_RANGE ||
> - quantization == V4L2_QUANTIZATION_LIM_RANGE;
> + return quantization > V4L2_QUANTIZATION_DEFAULT &&
> + quantization <= V4L2_QUANTIZATION_LAST;
> }
>
> #endif /* V4L2_COMMON_H_ */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 16dcd9dd1a50..099da1576db6 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -245,6 +245,14 @@ enum v4l2_colorspace {
>
> /* DCI-P3 colorspace, used by cinema projectors */
> V4L2_COLORSPACE_DCI_P3 = 12,
> +
> +#ifdef __KERNEL__
> + /*
> + * Largest supported colorspace value, used by the framework to check
> + * for invalid values.
> + */
> + V4L2_COLORSPACE_LAST = 12,
I might just add the enum there, it is more obvious it needs updating if
it's right next to the previous one. Or rely on the compiler assigning the
value, and update the code. Up to you.
For both:
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> +#endif
> };
>
> /*
> @@ -283,6 +291,13 @@ enum v4l2_xfer_func {
> V4L2_XFER_FUNC_NONE = 5,
> V4L2_XFER_FUNC_DCI_P3 = 6,
> V4L2_XFER_FUNC_SMPTE2084 = 7,
> +#ifdef __KERNEL__
> + /*
> + * Largest supported transfer function value, used by the framework to
> + * check for invalid values.
> + */
> + V4L2_XFER_FUNC_LAST = 7,
> +#endif
> };
>
> /*
> @@ -343,6 +358,13 @@ enum v4l2_ycbcr_encoding {
>
> /* SMPTE 240M -- Obsolete HDTV */
> V4L2_YCBCR_ENC_SMPTE240M = 8,
> +#ifdef __KERNEL__
> + /*
> + * Largest supported encoding value, used by the framework to check for
> + * invalid values.
> + */
> + V4L2_YCBCR_ENC_LAST = 8,
> +#endif
> };
>
> /*
> @@ -378,6 +400,13 @@ enum v4l2_quantization {
> V4L2_QUANTIZATION_DEFAULT = 0,
> V4L2_QUANTIZATION_FULL_RANGE = 1,
> V4L2_QUANTIZATION_LIM_RANGE = 2,
> +#ifdef __KERNEL__
> + /*
> + * Largest supported quantization value, used by the framework to check
> + * for invalid values.
> + */
> + V4L2_QUANTIZATION_LAST = 2,
> +#endif
> };
>
> /*
--
Regards,
Sakari Ailus
