[RESEND v3 0/3] Change V4L2_TYPE_IS_CAPTURE condition

[RESEND v3 0/3] Change V4L2_TYPE_IS_CAPTURE condition

[Nas Chung]

Hi, All.

I'm resending v3 patch series submitted on
https://lore.kernel.org/all/20240604051120.9441-1-nas.chung@chipsnmedia.com/

I have included a cover letter to provide additional context and
facilitate the review process.


v3 RESEND
- Add a cover letter

v3
- Adding a comment after "V4L2_BUF_TYPE_META_OUTPUT = 14,"
- Fixing a build warning in the venus/vdec.c code
- Removing V4L2_BUF_TYPE_VIDEO_OVERLAY in V4L2_TYPE_IS_OUTPUT()

v2
- Improve commit message
- Add V4L2_TYPE_IS_VALID(type) macro

Nas Chung (3):
  media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition
  media: qcom: venus: Fix uninitialized variable warning
  media: uapi: v4l: Fix V4L2_TYPE_IS_OUTPUT condition

 drivers/media/platform/qcom/venus/vdec.c |  2 +-
 include/uapi/linux/videodev2.h           | 11 +++++++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.25.1

[RESEND v3 1/3] media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition

[Nas Chung]

Explicitly compare a buffer type only with valid buffer types,
to avoid matching a buffer type outside of the valid buffer type set.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>
---
 include/uapi/linux/videodev2.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 4e91362da6da..8293b2197ea3 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -153,10 +153,17 @@ enum v4l2_buf_type {
 	V4L2_BUF_TYPE_SDR_OUTPUT           = 12,
 	V4L2_BUF_TYPE_META_CAPTURE         = 13,
 	V4L2_BUF_TYPE_META_OUTPUT	   = 14,
+	/*  V4L2_TYPE_IS_VALID and V4L2_TYPE_IS_OUTPUT must
+	 *  be updated if a new type is added.
+	 */
 	/* Deprecated, do not use */
 	V4L2_BUF_TYPE_PRIVATE              = 0x80,
 };
 
+#define V4L2_TYPE_IS_VALID(type)		\
+	((type) >= V4L2_BUF_TYPE_VIDEO_CAPTURE	\
+	 && (type) <= V4L2_BUF_TYPE_META_OUTPUT)
+
 #define V4L2_TYPE_IS_MULTIPLANAR(type)			\
 	((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE	\
 	 || (type) == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
@@ -171,7 +178,8 @@ enum v4l2_buf_type {
 	 || (type) == V4L2_BUF_TYPE_SDR_OUTPUT			\
 	 || (type) == V4L2_BUF_TYPE_META_OUTPUT)
 
-#define V4L2_TYPE_IS_CAPTURE(type) (!V4L2_TYPE_IS_OUTPUT(type))
+#define V4L2_TYPE_IS_CAPTURE(type)	\
+	(V4L2_TYPE_IS_VALID(type) && !V4L2_TYPE_IS_OUTPUT(type))
 
 enum v4l2_tuner_type {
 	V4L2_TUNER_RADIO	     = 1,
-- 
2.25.1

[RESEND v3 2/3] media: qcom: venus: Fix uninitialized variable warning

[Nas Chung]

Avoid uninitialized variable when both V4L2_TYPE_IS_OUTPUT() and
V4L2_TYPE_IS_CAPTURE() return false.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 drivers/media/platform/qcom/venus/vdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index d12089370d91..b72b579018e3 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -161,7 +161,7 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
 
 		if (V4L2_TYPE_IS_OUTPUT(type)) {
 			valid = venus_helper_check_codec(inst, fmt[i].pixfmt);
-		} else if (V4L2_TYPE_IS_CAPTURE(type)) {
+		} else {
 			valid = venus_helper_check_format(inst, fmt[i].pixfmt);
 
 			if (fmt[i].pixfmt == V4L2_PIX_FMT_QC10C &&
-- 
2.25.1

[RESEND v3 3/3] media: uapi: v4l: Fix V4L2_TYPE_IS_OUTPUT condition

[Nas Chung]

V4L2_TYPE_IS_OUTPUT() returns true for V4L2_BUF_TYPE_VIDEO_OVERLAY
which definitely belongs to CAPTURE.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 include/uapi/linux/videodev2.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 8293b2197ea3..5e85cf09278a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -171,7 +171,6 @@ enum v4l2_buf_type {
 #define V4L2_TYPE_IS_OUTPUT(type)				\
 	((type) == V4L2_BUF_TYPE_VIDEO_OUTPUT			\
 	 || (type) == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE		\
-	 || (type) == V4L2_BUF_TYPE_VIDEO_OVERLAY		\
 	 || (type) == V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY	\
 	 || (type) == V4L2_BUF_TYPE_VBI_OUTPUT			\
 	 || (type) == V4L2_BUF_TYPE_SLICED_VBI_OUTPUT		\
-- 
2.25.1

RE: [RESEND v3 0/3] Change V4L2_TYPE_IS_CAPTURE condition

[Nas Chung]

Hi, All.

Just a gentle ping on this patch series.

I've addressed all the suggested modifications in the cover letter.

Please let me know if I missed anything.

Thanks.
Nas.

>-----Original Message-----
>From: Nas Chung <nas.chung@chipsnmedia.com>
>Sent: Thursday, July 25, 2024 3:11 PM
>To: mchehab@kernel.org; hverkuil@xs4all.nl; bryan.odonoghue@linaro.org;
>linux-media@vger.kernel.org
>Cc: linux-kernel@vger.kernel.org; Nas Chung <nas.chung@chipsnmedia.com>
>Subject: [RESEND v3 0/3] Change V4L2_TYPE_IS_CAPTURE condition
>
>Hi, All.
>
>I'm resending v3 patch series submitted on
>https://lore.kernel.org/all/20240604051120.9441-1-
>nas.chung@chipsnmedia.com/
>
>I have included a cover letter to provide additional context and
>facilitate the review process.
>
>
>v3 RESEND
>- Add a cover letter
>
>v3
>- Adding a comment after "V4L2_BUF_TYPE_META_OUTPUT = 14,"
>- Fixing a build warning in the venus/vdec.c code
>- Removing V4L2_BUF_TYPE_VIDEO_OVERLAY in V4L2_TYPE_IS_OUTPUT()
>
>v2
>- Improve commit message
>- Add V4L2_TYPE_IS_VALID(type) macro
>
>Nas Chung (3):
>  media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition
>  media: qcom: venus: Fix uninitialized variable warning
>  media: uapi: v4l: Fix V4L2_TYPE_IS_OUTPUT condition
>
> drivers/media/platform/qcom/venus/vdec.c |  2 +-
> include/uapi/linux/videodev2.h           | 11 +++++++++--
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
>--
>2.25.1

Re: [RESEND v3 2/3] media: qcom: venus: Fix uninitialized variable warning

[Bryan O'Donoghue]

On 25/07/2024 07:10, Nas Chung wrote:
> Avoid uninitialized variable when both V4L2_TYPE_IS_OUTPUT() and
> V4L2_TYPE_IS_CAPTURE() return false.
> 
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> ---
>   drivers/media/platform/qcom/venus/vdec.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index d12089370d91..b72b579018e3 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -161,7 +161,7 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
>   
>   		if (V4L2_TYPE_IS_OUTPUT(type)) {
>   			valid = venus_helper_check_codec(inst, fmt[i].pixfmt);
> -		} else if (V4L2_TYPE_IS_CAPTURE(type)) {
> +		} else {
>   			valid = venus_helper_check_format(inst, fmt[i].pixfmt);
>   
>   			if (fmt[i].pixfmt == V4L2_PIX_FMT_QC10C &&

Can you give an example of the error - compile time I assume, this fixes ?

---
bod

RE: [RESEND v3 2/3] media: qcom: venus: Fix uninitialized variable warning

[Nas Chung]

Hi, Bryan O'Donoghue.

I received feedback from Hans with detailed information about this error.
https://lore.kernel.org/linux-kernel/a962e94f-a669-40e1-8b28-9ef077cf69be@xs4all.nl/

And, you may face the following error message using the smatch and the sparse test.
"find_format_by_index() error: uninitialized symbol 'valid'."

"make ARCH=arm64 CROSS_COMPILE=aarch64-none-linux-gnu- -j8 M=drivers/media/platform/qcom/venus C=2 CHECK=smatch"

Or, linux-kernel build with "-Wmaybe-uninitialized" gcc option.
"make ARCH=arm64 CROSS_COMPILE=aarch64-none-linux-gnu- -j8 M=drivers/media/platform/qcom/venus KCFLAGS="-Wmaybe-uninitialized"

>-----Original Message-----
>From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>Sent: Thursday, August 29, 2024 7:31 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; linux-media@vger.kernel.org
>Cc: linux-kernel@vger.kernel.org
>Subject: Re: [RESEND v3 2/3] media: qcom: venus: Fix uninitialized
>variable warning
>
>On 25/07/2024 07:10, Nas Chung wrote:
>> Avoid uninitialized variable when both V4L2_TYPE_IS_OUTPUT() and
>> V4L2_TYPE_IS_CAPTURE() return false.
>>
>> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
>> ---
>>   drivers/media/platform/qcom/venus/vdec.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
>b/drivers/media/platform/qcom/venus/vdec.c
>> index d12089370d91..b72b579018e3 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -161,7 +161,7 @@ find_format_by_index(struct venus_inst *inst,
>unsigned int index, u32 type)
>>
>>   		if (V4L2_TYPE_IS_OUTPUT(type)) {
>>   			valid = venus_helper_check_codec(inst, fmt[i].pixfmt);
>> -		} else if (V4L2_TYPE_IS_CAPTURE(type)) {
>> +		} else {
>>   			valid = venus_helper_check_format(inst, fmt[i].pixfmt);
>>
>>   			if (fmt[i].pixfmt == V4L2_PIX_FMT_QC10C &&
>
>Can you give an example of the error - compile time I assume, this fixes ?
>
>---
>bod