Message ID | 20240517094940.1169-1-nas.chung@chipsnmedia.com (mailing list archive) |
---|---|
State | Superseded |
Headers |
Received: from am.mirrors.kernel.org ([147.75.80.249]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-11588-patchwork=linuxtv.org@vger.kernel.org>) id 1s7uE0-00033q-2S for patchwork@linuxtv.org; Fri, 17 May 2024 09:50:06 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 756651F22A70 for <patchwork@linuxtv.org>; Fri, 17 May 2024 09:50:02 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B1D6F23772; Fri, 17 May 2024 09:49:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chipsnmedia.com header.i=@chipsnmedia.com header.b="aJWfcreG" X-Original-To: linux-media@vger.kernel.org Received: from SLXP216CU001.outbound.protection.outlook.com (mail-koreacentralazon11020002.outbound.protection.outlook.com [52.101.154.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 47DFA1EB48; Fri, 17 May 2024 09:49:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=52.101.154.2 ARC-Seal: i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715939395; cv=fail; b=GRzhE6C/jR2U6Z4VWrxHVXefk9RM1iaEVww45yhfioIkCo30lyD0nD0k313hB2a6zjK2F/iQT/pOmGcFZ3knWoPN4FirPwoB7rglut35roPKQBPOMDgPItqP62VQR/fIu+4w15wQ91SZ2AKGs6SjSu8LENXKSgqjoQRJh+gmjns= ARC-Message-Signature: i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715939395; c=relaxed/simple; bh=8wixs9KktvKVLDwNnu/oWSpJyxfllMrAn8ulQMDCYRg=; h=From:To:Cc:Subject:Date:Message-Id:Content-Type:MIME-Version; b=u0YJ9csOo9L3qO9zFdnRQyAvIQXGTmBWvafAYs3Qw0V+isPQRuZEu8IcLsJ5d375AMFEi9RFxrHNkP3iB67OsuxCz3VbylvV5IPYUsUCFnCAYjkmzks5AtoA5l++GQK6SLV3NsMFGm3XHStf57HB/xXTBursCcd1PCiWqhDBWgE= ARC-Authentication-Results: i=2; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=chipsnmedia.com; spf=pass smtp.mailfrom=chipsnmedia.com; dkim=pass (1024-bit key) header.d=chipsnmedia.com header.i=@chipsnmedia.com header.b=aJWfcreG; arc=fail smtp.client-ip=52.101.154.2 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=chipsnmedia.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chipsnmedia.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kFNPvpuEaK5zCIO5R9m/8qJrDXt466ybg2yjg4GF6ToZwHdbCtLYPtKZypi7rl3akFBMZCd13MdK+t8xb0ljACF0ADwisuxiYPbq5oPIQ++e71MSVVclDUlZvenwlg/HhWFXIhXQaq80c+ElM2WDprUHkSVmXZM+Xy3SesEM/3UbP3in1eFfVcxUHNoGfaOb2JGSyM7nP2qs1+V9JVxnFjDYul2kCg2jzRENTdlwNwQ5VYnDlCRE3enmJ5s5v2Nvmh6MvX+DLF61tYtv0LLTOUoyUZs2wNlOQIdGIeawMqsqsUTkNK0hxeRwBon7w3MvqDd8g+32iwpTTWIbZDtJrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=mT/z9Wm2coDk4CyO+OSPldqlfaO2Mdv/YQQ8LbBLbJ4=; b=HsorrB7WhyCV6T5DEOAWNpLST7J5WqOxXNzLOYTts2TYyPkxsYANSfnkPL8gyRli9dkbwA99iVvTZAbFHWydxJsDkrRTNWyyX8DK7R1dgxrNTCNptFpgm6BfSvmztiH7I8MPxWqkRxROitZDPJcQr4fIsEkpR8NhCxG6t3xiFIcnLvfx7X1o81r4mjpuR3pmQs3Dki3sQSvQZYScbSqezNar41aEhWLWcWWn1Tt1p/mdoxpVzCi97WhzTtzPc7yc2SLcbHzzFXTv+nIoIqoYr6pG+bk4g+wSISO9v5kCoE9728xoRKR+U2kvxihNRuySzU3jAtRGhVbSqtC3uHZdnA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=chipsnmedia.com; dmarc=pass action=none header.from=chipsnmedia.com; dkim=pass header.d=chipsnmedia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chipsnmedia.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mT/z9Wm2coDk4CyO+OSPldqlfaO2Mdv/YQQ8LbBLbJ4=; b=aJWfcreGf/JgfzbnisIXHvgsV+56zf36Wwr2iW+GcwP/0ZevGNvx5IVzWiO/yNkpRSBlUJM9naH6QdfhAFIlUi/y2TwQDzn2zred+aLiyu7DrSwBIvNbHimyD3mgElS2NspZrhu4oH34KP6SWd6YzTo9eOR/5AzIAhezz4YEseA= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=chipsnmedia.com; Received: from SL2P216MB1246.KORP216.PROD.OUTLOOK.COM (2603:1096:101:a::9) by PU4P216MB1999.KORP216.PROD.OUTLOOK.COM (2603:1096:301:12d::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7587.30; Fri, 17 May 2024 09:49:49 +0000 Received: from SL2P216MB1246.KORP216.PROD.OUTLOOK.COM ([fe80::8bac:533d:dbe5:ba28]) by SL2P216MB1246.KORP216.PROD.OUTLOOK.COM ([fe80::8bac:533d:dbe5:ba28%7]) with mapi id 15.20.7587.028; Fri, 17 May 2024 09:49:49 +0000 From: Nas Chung <nas.chung@chipsnmedia.com> To: mchehab@kernel.org, linux-media@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nas Chung <nas.chung@chipsnmedia.com> Subject: [PATCH] media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition Date: Fri, 17 May 2024 18:49:40 +0900 Message-Id: <20240517094940.1169-1-nas.chung@chipsnmedia.com> X-Mailer: git-send-email 2.25.1 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: SL2P216CA0175.KORP216.PROD.OUTLOOK.COM (2603:1096:101:1b::6) To SL2P216MB1246.KORP216.PROD.OUTLOOK.COM (2603:1096:101:a::9) Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SL2P216MB1246:EE_|PU4P216MB1999:EE_ X-MS-Office365-Filtering-Correlation-Id: e84c618b-d87f-494a-fb17-08dc7656b124 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230031|1800799015|52116005|376005|366007|38350700005; X-Microsoft-Antispam-Message-Info: o4UMcvbJ6ZoQq7fIXvlEkJa3zpWz2H4srUsOhHuRZTXAkGxVCqry8w5z2+1+0+aZFKTD/tHGYo+c2F9jT5Urffc1FHOsjwoF4bjcKzIfYcI1KRQ6iO47xocVlVXD//S0UFxvfFJLViWHHYuh4mX+5/cy84t4fxO0ftOk87sios+rUCwExB3WkB0YrRyMy0vCMreVN9KCkc7L4xVx02T6C1t3Ekk6GMI7He5gMWNZHxFaUbaO42nqzuq0IKTE7nVHsuJQxOt37cr8ut2/p75BxrCX6qH2RmytgyJZW/5q45zxFutvblUbYGnpMohNpto7p4GMH70Z8Xse2SGORQaWfwL5uGeOc0bCm3yzmMgN3bflvXRt0nC7HxTd7SlafzlM4dBoNx9ZGFH4E16eRnLKjl9YCkighzxeYpx02v0sq9GtKpfVaSUQQnSJaRZ5d4j5y17nMOiVnm33hD2IEHdM0HLBR4sipy4blNsnydo62xzMYF/WznaiuO5C4jvsbHRaNac0pEk1BY3k8bco80mnR3IWul2bixHM33QHcPxZNE0NRByxOhwr03ypIZu5c/B9eOeVWM7ceWMQZ+EhZklbuQLfSqT7m+dyHTk8XGj4JmBuoFOGbHL4ktuoW3/JfF75kWJLDecajHQ1J/F05ivBbUtC+IstZufaV3VjQog1O3TtuM7cAwHvfUkXhthMNENR9fu2vQ0UGLgqhFM5oNpLa5kYRkGXq+DT0fAThoKUuCfli33NxTTz7Gqm+F5QMJmEgbaRfkIwQcP31fIPkIxbK6hr7QmPfySA/JjhsH2rdfrBj59L11yV0XesWjczv0J1ZDbd2YjcJK+xMLOwHuQNcczapmZfZqG1FqyWnQxZOk9vqnOhyljZjkXk4kh2A3CBqAZdQjYLba3LgrbSxnPGJFmUHU8Vwzek2UjLDOZtdMY5T58uE+6kZbwuRowIKMBkBEfizyx7sCIlM4mqK0/rcehAyV8pNgixOvl4Tp3dKLBkq8jodmhntS4UGLy021bn2VmY03HCnSDqRtGavwFCvLyYLx+NB4Okocja76WcnxLdn4iePWRTQKWG/xv0HDzG8SbijF2NzgwsLFysWkagkeNgnlQ2WEz3XebSYtDHSHPutteBMbjlSnUAfBIUfT8QluftmIkxn2On3qNaNyH1yiHrAPGYxKjPRIemyy+kUo40sfT8sP92azFX80jMMkKd+7JeaUqkLY7sVNnuF5YSmhRHBZmdppZRWFlbH3TDD/Hjvt/ga90vTgfAjnEHp6QiXxb2BxJZiT2Wtb6ozE4PBHDC1oA64c36Ke0gYDrhG4A11Vhz6f/2povECMJIa8UaMZqQSnQek5aCxndQnrhUDA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SL2P216MB1246.KORP216.PROD.OUTLOOK.COM;PTR:;CAT:NONE;SFS:(13230031)(1800799015)(52116005)(376005)(366007)(38350700005);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: DtFQCw73f1f248XoGquBH9f6huJnvpCcgIn1UquwARt2GB2WuMfmzEa+mXOy+S9BCqOEwiKqXU7z6Z13dDvgi5Qxl+eBOFUzK4APeVyQah/0RE/DjngRx+jW0gGbu5PKFs78Fc3vMkMMc/mvepDPPD1LiTY5b/kRsoSR58p05mTurFSFNwFmIaLMW1kXawsSL/ZF3aajYVOBpvAjrqKYeRUIYpuM2p+L1Y6zA6V+ajjdLWptomCcnfUqZW9CxNbW/vJ3qkrey/s2+Gi8bDnJJE5bHvsVCO2+MCgHLs5pDpc8H+CDhTSHlyp3MOPbOLx4VmFmVfy/tWB/XvmNqOWBWUojtPTm9Djhl1WRVB96k6mqp2IWANxVpupm955RtkrjW5MU2nRanjDu8QxYMiN0D2DigACZgSB5v8jXTGe7c8HtauNPDDR17rYZGlApEQifODNvnjG/SZ6rLuOKSWrOZ75g1U9SwGopqUn9omkrdEpRrPiuYLZO/aQTWtONmVtR0asOh4S7SQWbETTenFCWGVaDVjId3HQLOqZ/kbgPRyDwT0W7Xk002R0O2dzNxNzdFIrRcD9CtHOu3u8VKq3GkGmH1/6SogE+9tYCnPDzQgJURW7ARKhGQDF3p6CQHMdpreDVXuv6UFL8sOhSK9YS98OdfAqkruTdnw4+qDX5o+UHEc7IoGHDIUR9rYUIImad7l2vljGqn68CAjDvFUTQNBDEaDJWI9lYRZu+ekYiqGuobszGtNe/Iaix+Vqe6C2pfIExI5tEg+z9KLVk3sH4SlXmRmP915sy2kfZ70d45JW4+msQd/BFtMriP9R8Gk4sD2UnD1O+pgfFCpGO+geRbuinJnm56UzY4yphaLKf26imyRjrlyN2P0s0KOG0Eks79OqafEJn+JcLT8WCltk+kg2o774LqNKLliRx9gE1++T4hLnSJpKdxWbCgiUcoDJuqRub8YqvfVzS3AW9pmTfZeuIPA/FCjBthtsXMWiyIqmPFf0saeU6O2rNNj3GIFWK5ZKXkofUS5T9408GiiO+MjMB0d09z1L+ADFPsMvxqAlMCEFILu+wYnhNvNFKvfFW3jpmN4BH8F9mS2QYNziHXyZDsQCBqzqzlikL3h1MgV3TdSwv/XJ7EOmK6iW1qcEqap2AAdv8U72DBQTPEdEa3s0LDazPyBMjPoy1/6DeFecw9f/940IIOMz8Q5p2AljBYWx2DncnrLrseGkLhL5geDUi3QQtE+EVSzhaMWOdoO7wY7FzQYhV8KUAst9pie+07xQ9levH7sBE1O3SQeemkmebd/9OkIOnI+dCwnk0PiaBBCRzUOZJq/5hJBT1uMYJTK0fTIym0LJKYWrgJxX2VOF8SEVL5fYpUgjSxUhtal6bj8ujCI5rVJWfOzopA1c59GvHMssUMku3tW+4qJZdn5pCplDoFOluhIsvFclCX5B69htSsC6aCu1DGaIquhL/feDVOdsMYfP+iPnfDg9Nuo1dix6V4WIw8ldLyQprbdGAgAuj4aHw9uOGrkcFssJ1DQ5nVReGgIDsj48JLbwDUDq4+cDyRhFhOFy+64jsJFtb5VM9lnqF6f0kbsNuNkhZJ71rGUmegLTEOjU2Zj7oOQ== X-OriginatorOrg: chipsnmedia.com X-MS-Exchange-CrossTenant-Network-Message-Id: e84c618b-d87f-494a-fb17-08dc7656b124 X-MS-Exchange-CrossTenant-AuthSource: SL2P216MB1246.KORP216.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2024 09:49:49.1778 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4d70c8e9-142b-4389-b7f2-fa8a3c68c467 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Z2FKZS46gimDdbElxRQylk9MqOn5BSlnzYT5MmmX0z+pv5KJlL12kOHsWzK4GAISUHHa3dR21vN/V9QMRxeugDhwUfyNsM6oVYSeQ+QXz44= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PU4P216MB1999 X-LSpam-Score: -2.6 (--) X-LSpam-Report: No, score=-2.6 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_MISSING=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,SPF_HELO_NONE=0.001,SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no |
Series |
media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE condition
|
|
Commit Message
Nas Chung
May 17, 2024, 9:49 a.m. UTC
We expect V4L2_TYPE_IS_CAPTURE() macro allow only CAPTURE type.
But, Inverting OUTPUT type can allow undefined v4l2_buf_type.
Check CAPTURE type directly instead of inverting OUTPUT type.
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
include/uapi/linux/videodev2.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Comments
Hey Nas, thanks for the patch, I think making the macro more explicit is generally a good idea, but in this case all !OUTPUT are actually CAPTURE types (besides the one deprecated type) and when I look at the definitions of some of the set commands like S_FMT, I can see that they require a type as parameter. So, could you explain in the commit message, how it can happen that the buf_type is undefined? And if so maybe that case should be fixed instead? I have improved your commit message below, but please explain why this is needed, e.g. which case did you hit where you found an undefined buffer. On 17.05.2024 18:49, Nas Chung wrote: >We expect V4L2_TYPE_IS_CAPTURE() macro allow only CAPTURE type. >But, Inverting OUTPUT type can allow undefined v4l2_buf_type. >Check CAPTURE type directly instead of inverting OUTPUT type. My suggestion for this commit message: """ Explicitly compare the type of the buffer with the available CAPTURE buffer types, to avoid matching a buffer type outside of the valid buffer type set. """ Basically fixing the sentence structure and grammar and focusing more on the reason of your action instead of describing what the code does (which should hopefully be obvious in most cases) I hope that helps :) Regards, Sebastian > >Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >--- > include/uapi/linux/videodev2.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > >diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >index fe6b67e83751..32b10e2b7695 100644 >--- a/include/uapi/linux/videodev2.h >+++ b/include/uapi/linux/videodev2.h >@@ -171,7 +171,13 @@ 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) \ >+ ((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE \ >+ || (type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE \ >+ || (type) == V4L2_BUF_TYPE_VBI_CAPTURE \ >+ || (type) == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE \ >+ || (type) == V4L2_BUF_TYPE_SDR_CAPTURE \ >+ || (type) == V4L2_BUF_TYPE_META_CAPTURE) > > enum v4l2_tuner_type { > V4L2_TUNER_RADIO = 1, >-- >2.25.1 > >
Hi, Sebastian. Thanks for the prompt reply. >-----Original Message----- >From: Sebastian Fricke <sebastian.fricke@collabora.com> >Sent: Friday, May 17, 2024 7:10 PM >To: Nas Chung <nas.chung@chipsnmedia.com> >Cc: mchehab@kernel.org; linux-media@vger.kernel.org; linux- >kernel@vger.kernel.org >Subject: Re: [PATCH] media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE >condition > >Hey Nas, > >thanks for the patch, I think making the macro more explicit is >generally a good idea, but in this case all !OUTPUT are actually CAPTURE >types (besides the one deprecated type) and when I look at the >definitions of some of the set commands like S_FMT, I can see that they >require a type as parameter. >So, could you explain in the commit message, how it can happen that the >buf_type is undefined? And if so maybe that case should be fixed >instead? v4l2-compliance test G_SELECTION ioctl with invalid buffer type. testBasicSelection() { // Check handling of invalid type. sel.type = 0xff; fail_on_test(doioctl(node, VIDIOC_G_SELECTION, &sel) != EINVAL); // Check handling of invalid target. } In v4l2 driver, I'm trying to replace IF clause for buffer type checking to helper macro. But, If I use V4L2_TYPE_IS_CAPTURE in g_selection() ioctl_ops function, It cannot pass the above test case. And, Buffer type is set by host. We cannot ensure host always set valid buffer type. So, I think we can prevent any potential bugs. Or checking valid buffer type in v4l_g_selection() is another option. >I have improved your commit message below, but please explain why this >is needed, e.g. which case did you hit where you found an undefined >buffer. > >On 17.05.2024 18:49, Nas Chung wrote: >>We expect V4L2_TYPE_IS_CAPTURE() macro allow only CAPTURE type. >>But, Inverting OUTPUT type can allow undefined v4l2_buf_type. >>Check CAPTURE type directly instead of inverting OUTPUT type. > >My suggestion for this commit message: > >""" >Explicitly compare the type of the buffer with the available CAPTURE >buffer types, to avoid matching a buffer type outside of the valid >buffer type set. >""" Much better! Thanks. Thanks. Nas. > >Basically fixing the sentence structure and grammar and focusing more on >the reason of your action instead of describing what the code does >(which should hopefully be obvious in most cases) > >I hope that helps :) > >Regards, >Sebastian > >> >>Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >>--- >> include/uapi/linux/videodev2.h | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >>diff --git a/include/uapi/linux/videodev2.h >b/include/uapi/linux/videodev2.h >>index fe6b67e83751..32b10e2b7695 100644 >>--- a/include/uapi/linux/videodev2.h >>+++ b/include/uapi/linux/videodev2.h >>@@ -171,7 +171,13 @@ 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) \ >>+ ((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE \ >>+ || (type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE \ >>+ || (type) == V4L2_BUF_TYPE_VBI_CAPTURE \ >>+ || (type) == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE \ >>+ || (type) == V4L2_BUF_TYPE_SDR_CAPTURE \ >>+ || (type) == V4L2_BUF_TYPE_META_CAPTURE) >> >> enum v4l2_tuner_type { >> V4L2_TUNER_RADIO = 1, >>-- >>2.25.1 >> >>
On Fri, 17 May 2024 18:49:40 +0900, Nas Chung wrote: > We expect V4L2_TYPE_IS_CAPTURE() macro allow only CAPTURE type. > But, Inverting OUTPUT type can allow undefined v4l2_buf_type. > Check CAPTURE type directly instead of inverting OUTPUT type. > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > --- > include/uapi/linux/videodev2.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index fe6b67e83751..32b10e2b7695 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -171,7 +171,13 @@ 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) \ > + ((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE \ > + || (type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE \ > + || (type) == V4L2_BUF_TYPE_VBI_CAPTURE \ > + || (type) == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE \ > + || (type) == V4L2_BUF_TYPE_SDR_CAPTURE \ > + || (type) == V4L2_BUF_TYPE_META_CAPTURE) Maybe adding a V4L2_TYPE_IS_VALID(type) macro would be helpful to define TYPE_IS_CAPTURE as all valid types that are not OUTPUT: #define V4L2_TYPE_IS_VALID(type) \ ((type) >= V4L2_BUF_TYPE_VIDEO_CAPTURE \ && (type) <= V4L2_BUF_TYPE_META_OUTPUT) #define V4L2_TYPE_IS_CAPTURE(type) \ (V4L2_TYPE_IS_VALID(type) && !V4L2_TYPE_IS_OUTPUT(type)) This would avoid keeping the two explicit lists of OUTPUT and CAPTURE types. Michael > > enum v4l2_tuner_type { > V4L2_TUNER_RADIO = 1, > -- > 2.25.1 > > >
Hi, Michael. Thank you for the feedback. >-----Original Message----- >From: Michael Tretter <m.tretter@pengutronix.de> >Sent: Wednesday, May 22, 2024 4:55 PM >To: Nas Chung <nas.chung@chipsnmedia.com> >Cc: mchehab@kernel.org; linux-media@vger.kernel.org; linux- >kernel@vger.kernel.org; Sebastian Fricke <sebastian.fricke@collabora.com> >Subject: Re: [PATCH] media: uapi: v4l: Change V4L2_TYPE_IS_CAPTURE >condition > >On Fri, 17 May 2024 18:49:40 +0900, Nas Chung wrote: >> We expect V4L2_TYPE_IS_CAPTURE() macro allow only CAPTURE type. >> But, Inverting OUTPUT type can allow undefined v4l2_buf_type. >> Check CAPTURE type directly instead of inverting OUTPUT type. >> >> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >> --- >> include/uapi/linux/videodev2.h | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/videodev2.h >b/include/uapi/linux/videodev2.h >> index fe6b67e83751..32b10e2b7695 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -171,7 +171,13 @@ 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) \ >> + ((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE \ >> + || (type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE \ >> + || (type) == V4L2_BUF_TYPE_VBI_CAPTURE \ >> + || (type) == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE \ >> + || (type) == V4L2_BUF_TYPE_SDR_CAPTURE \ >> + || (type) == V4L2_BUF_TYPE_META_CAPTURE) > >Maybe adding a V4L2_TYPE_IS_VALID(type) macro would be helpful to define >TYPE_IS_CAPTURE as all valid types that are not OUTPUT: > > #define V4L2_TYPE_IS_VALID(type) \ > ((type) >= V4L2_BUF_TYPE_VIDEO_CAPTURE \ > && (type) <= V4L2_BUF_TYPE_META_OUTPUT) > > #define V4L2_TYPE_IS_CAPTURE(type) \ > (V4L2_TYPE_IS_VALID(type) && !V4L2_TYPE_IS_OUTPUT(type)) > >This would avoid keeping the two explicit lists of OUTPUT and CAPTURE >types. I agree that it's better to add a V4L2_TYPE_IS_VALID(type) macro. I will address this in V2. Thanks. Nas. > >Michael > >> >> enum v4l2_tuner_type { >> V4L2_TUNER_RADIO = 1, >> -- >> 2.25.1 >> >> >>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index fe6b67e83751..32b10e2b7695 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -171,7 +171,13 @@ 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) \ + ((type) == V4L2_BUF_TYPE_VIDEO_CAPTURE \ + || (type) == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE \ + || (type) == V4L2_BUF_TYPE_VBI_CAPTURE \ + || (type) == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE \ + || (type) == V4L2_BUF_TYPE_SDR_CAPTURE \ + || (type) == V4L2_BUF_TYPE_META_CAPTURE) enum v4l2_tuner_type { V4L2_TUNER_RADIO = 1,