From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1733EC433EF for ; Wed, 29 Jun 2022 13:50:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231289AbiF2Nux (ORCPT ); Wed, 29 Jun 2022 09:50:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51766 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229525AbiF2Nuw (ORCPT ); Wed, 29 Jun 2022 09:50:52 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7D18E18E34 for ; Wed, 29 Jun 2022 06:50:51 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1573B61E87 for ; Wed, 29 Jun 2022 13:50:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45CA6C34114; Wed, 29 Jun 2022 13:50:49 +0000 (UTC) Message-ID: <1d76d6bf-7809-edca-0d1a-9ab84e0c15ea@xs4all.nl> Date: Wed, 29 Jun 2022 15:50:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH 4/7] media: v4l2: Make colorspace validity checks more future-proof Content-Language: en-US To: Laurent Pinchart , linux-media@vger.kernel.org Cc: Sakari Ailus , Jacopo Mondi , Xavier Roumegue , linux-imx@nxp.com, kernel@pengutronix.de References: <20220616183656.19089-1-laurent.pinchart@ideasonboard.com> <20220616183656.19089-5-laurent.pinchart@ideasonboard.com> From: Hans Verkuil In-Reply-To: <20220616183656.19089-5-laurent.pinchart@ideasonboard.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org 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 > Acked-by: Sakari Ailus > --- > 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 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 > }; > > /*