On 16/06/2022 20:36, 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>
> Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> Changes since v1:
>
> - Let the compiler assign a value to the *_LAST enum entries
> ---
> 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..ccc138a9104d 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;
There are only two quantization ranges: full and limited. And I very sincerely
hope there will never be a third!
So I would keep this as-is and drop the V4L2_QUANTIZATION_LAST.
Otherwise I like this. So with that change:
Reviewed-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Regards,
Hans
> }
>
> #endif /* V4L2_COMMON_H_ */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index f6f9a690971e..7b7d852dc74b 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, assigned by the compiler, used
> + * by the framework to check for invalid values.
> + */
> + V4L2_COLORSPACE_LAST,
> +#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, assigned by the compiler,
> + * used by the framework to check for invalid values.
> + */
> + V4L2_XFER_FUNC_LAST,
> +#endif
> };
>
> /*
> @@ -343,6 +358,13 @@ enum v4l2_ycbcr_encoding {
>
> /* SMPTE 240M -- Obsolete HDTV */
> V4L2_YCBCR_ENC_SMPTE240M = 8,
> +#ifdef __KERNEL__
> + /*
> + * Largest supported encoding value, assigned by the compiler, used by
> + * the framework to check for invalid values.
> + */
> + V4L2_YCBCR_ENC_LAST,
> +#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, assigned by the compiler, used
> + * by the framework to check for invalid values.
> + */
> + V4L2_QUANTIZATION_LAST,
> +#endif
> };
>
> /*
