Message ID | 20240328-uvc-fix-relative-ptz-speed-v1-1-17373eb8b2be@securitylive.com (mailing list archive) |
---|---|
State | New |
Delegated to: | Laurent Pinchart |
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-8153-patchwork=linuxtv.org@vger.kernel.org>) id 1rpz0r-0001La-2O for patchwork@linuxtv.org; Thu, 28 Mar 2024 23:18:26 +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 590E31F24919 for <patchwork@linuxtv.org>; Thu, 28 Mar 2024 23:18:23 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7C89F13342A; Thu, 28 Mar 2024 23:18:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Vq/XqNxA" X-Original-To: linux-media@vger.kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 1359A6A332; Thu, 28 Mar 2024 23:18:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711667893; cv=none; b=CtXAzK5JWOgga28AtPNQh9bkhfLDFeXKElI4lYFpX0fHg8WlwGQR9t21+77l6hfoklmRLpmhk+HipG7YTGVKTA2xYhOIruGpKE19E0V1d3RtQyaCN7Rss/ulTDh/V5UKKB+Z7NtY7R31mAJzr/9PbXVJbMd/uDCZ7xYt/jLTccU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711667893; c=relaxed/simple; bh=40uh/wKUId8Vzuahx/WopdPsq9bSrg4TfoGpA2V/BrE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=jZs4cV6WjMw5FixdWdwcTwPw/+Aehn17m4FSAFIMfhD1dwTrdk0Cuw1gvJNHfqSMsqZb+0fpZB+gAEiOtAec49GKk15nlc1aGpYuf9rCL5j8iAif6yCkmruHGv4Yva3G0afMOrV+pAuRO/7y8NdOLwWJq0E8idFLQrdeCdhiZt0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Vq/XqNxA; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPS id ACDF9C433C7; Thu, 28 Mar 2024 23:18:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1711667892; bh=40uh/wKUId8Vzuahx/WopdPsq9bSrg4TfoGpA2V/BrE=; h=From:Date:Subject:References:In-Reply-To:To:Cc:Reply-To:From; b=Vq/XqNxAxcS8T/IL2qp5bWeLxiWk1HyBxSo35IjC0it+KT4YT7VQxp0/1IknM6mfA h6h6u53CotkaZiyy6DPamNFFwmsxxuDOSd8cgEdtA1T/bjE/roCRVqh3fR510zlJAx BtTUKDuAXQpeJIvYAHpFBclm7RJ7s3uDjtbirKh/SKPlwIJAD4w39hTYebPSnZNJ5p pb++CJ9WwN+tfZOjIsxYEa21llVMKuN4GmMJo/bApLPLZibb66JqsQeJnCx4uYswTN dOAwsiYIAurPn43gQHSZA/489NF0q3egv/LoYya3Oz2OQxCqpFfEDeTuK3RkHyt3I/ 79f6gaBTxt+dA== Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 98299CD1288; Thu, 28 Mar 2024 23:18:12 +0000 (UTC) From: John Bauer via B4 Relay <devnull+johnebgood.securitylive.com@kernel.org> Date: Thu, 28 Mar 2024 18:18:06 -0500 Subject: [PATCH 1/2] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix 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 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20240328-uvc-fix-relative-ptz-speed-v1-1-17373eb8b2be@securitylive.com> References: <20240328-uvc-fix-relative-ptz-speed-v1-0-17373eb8b2be@securitylive.com> In-Reply-To: <20240328-uvc-fix-relative-ptz-speed-v1-0-17373eb8b2be@securitylive.com> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Mauro Carvalho Chehab <mchehab@kernel.org> Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linh.tp.vu@gmail.com, ribalda@chromium.org, soyer@irl.hu, John Bauer <johnebgood@securitylive.com> X-Mailer: b4 0.13.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1711667891; l=2495; i=johnebgood@securitylive.com; s=20240325; h=from:subject:message-id; bh=QZxJ8IkQ+0S1QgP+EtdvdygUUbw+bFRKj/oMsKdGp5w=; b=BwFjVHPclp/6knYLtkp/FSdHxIvkJwLnwlzuViZ/kUwMkmbxJjNoJxvLls+p8kkgwkCwMyh2e yeZ3fKPtxEbAiHlrRVJmTQtN+5NU91hNGSDVVCUU59RDHQh7VlaigZT X-Developer-Key: i=johnebgood@securitylive.com; a=ed25519; pk=RN31Fmrxbidp1TwtZGNmQwTDjUWMPnewQJfA/ug2P9E= X-Endpoint-Received: by B4 Relay for johnebgood@securitylive.com/20240325 with auth_id=143 X-Original-From: John Bauer <johnebgood@securitylive.com> Reply-To: johnebgood@securitylive.com X-LSpam-Score: -4.1 (----) X-LSpam-Report: No, score=-4.1 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIMWL_WL_HIGH=-1,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_NONE=-0.0001,SPF_HELO_NONE=0.001,SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no |
Series |
media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix
|
|
Commit Message
John Bauer via B4 Relay
March 28, 2024, 11:18 p.m. UTC
From: John Bauer <johnebgood@securitylive.com> The minimum UVC control value for the relative pan/tilt/zoom speeds cannot be probed as the implementation condenses the pan and tilt direction and speed into two 16 bit values. The minimum cannot be set at probe time because it is probed first and the maximum is not yet known. With this fix if a relative speed control is queried or set the minimum is set and checked based on the additive inverse of the maximum at that time. Signed-off-by: John Bauer <johnebgood@securitylive.com> --- drivers/media/usb/uvc/uvc_ctrl.c | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
Comments
Hi John, kernel test robot noticed the following build warnings: [auto build test WARNING on 23956900041d968f9ad0f30db6dede4daccd7aa9] url: https://github.com/intel-lab-lkp/linux/commits/John-Bauer-via-B4-Relay/media-uvcvideo-UVC-minimum-relative-pan-tilt-zoom-speed-fix/20240329-071938 base: 23956900041d968f9ad0f30db6dede4daccd7aa9 patch link: https://lore.kernel.org/r/20240328-uvc-fix-relative-ptz-speed-v1-1-17373eb8b2be%40securitylive.com patch subject: [PATCH 1/2] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix config: arm-defconfig (https://download.01.org/0day-ci/archive/20240330/202403300745.omqy1CdY-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240330/202403300745.omqy1CdY-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403300745.omqy1CdY-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/media/usb/uvc/uvc_ctrl.c:1944:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] default: ^ drivers/media/usb/uvc/uvc_ctrl.c:1944:3: note: insert 'break;' to avoid fall-through default: ^ break; 1 warning generated. vim +1944 drivers/media/usb/uvc/uvc_ctrl.c 1898 1899 int uvc_ctrl_set(struct uvc_fh *handle, 1900 struct v4l2_ext_control *xctrl) 1901 { 1902 struct uvc_video_chain *chain = handle->chain; 1903 struct uvc_control *ctrl; 1904 struct uvc_control_mapping *mapping; 1905 s32 value; 1906 u32 step; 1907 s32 min; 1908 s32 max; 1909 int ret; 1910 1911 if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0) 1912 return -EACCES; 1913 1914 ctrl = uvc_find_control(chain, xctrl->id, &mapping); 1915 if (ctrl == NULL) 1916 return -EINVAL; 1917 if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR)) 1918 return -EACCES; 1919 1920 /* Clamp out of range values. */ 1921 switch (mapping->v4l2_type) { 1922 case V4L2_CTRL_TYPE_INTEGER: 1923 if (!ctrl->cached) { 1924 ret = uvc_ctrl_populate_cache(chain, ctrl); 1925 if (ret < 0) 1926 return ret; 1927 } 1928 1929 min = mapping->get(mapping, UVC_GET_MIN, 1930 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); 1931 max = mapping->get(mapping, UVC_GET_MAX, 1932 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); 1933 1934 /* 1935 * For the relative speed implementation the minimum 1936 * value cannot be probed so it becomes the additive 1937 * inverse of maximum. 1938 */ 1939 switch (xctrl->id) { 1940 case V4L2_CID_ZOOM_CONTINUOUS: 1941 case V4L2_CID_PAN_SPEED: 1942 case V4L2_CID_TILT_SPEED: 1943 min = max * -1; > 1944 default: 1945 break; 1946 } 1947 1948 step = mapping->get(mapping, UVC_GET_RES, 1949 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); 1950 if (step == 0) 1951 step = 1; 1952 1953 xctrl->value = min + DIV_ROUND_CLOSEST((u32)(xctrl->value - min), 1954 step) * step; 1955 if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED) 1956 xctrl->value = clamp(xctrl->value, min, max); 1957 else 1958 xctrl->value = clamp_t(u32, xctrl->value, min, max); 1959 value = xctrl->value; 1960 break; 1961 1962 case V4L2_CTRL_TYPE_BITMASK: 1963 if (!ctrl->cached) { 1964 ret = uvc_ctrl_populate_cache(chain, ctrl); 1965 if (ret < 0) 1966 return ret; 1967 } 1968 1969 xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping); 1970 value = xctrl->value; 1971 break; 1972 1973 case V4L2_CTRL_TYPE_BOOLEAN: 1974 xctrl->value = clamp(xctrl->value, 0, 1); 1975 value = xctrl->value; 1976 break; 1977 1978 case V4L2_CTRL_TYPE_MENU: 1979 if (xctrl->value < (ffs(mapping->menu_mask) - 1) || 1980 xctrl->value > (fls(mapping->menu_mask) - 1)) 1981 return -ERANGE; 1982 1983 if (!test_bit(xctrl->value, &mapping->menu_mask)) 1984 return -EINVAL; 1985 1986 value = uvc_mapping_get_menu_value(mapping, xctrl->value); 1987 1988 /* 1989 * Valid menu indices are reported by the GET_RES request for 1990 * UVC controls that support it. 1991 */ 1992 if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) { 1993 if (!ctrl->cached) { 1994 ret = uvc_ctrl_populate_cache(chain, ctrl); 1995 if (ret < 0) 1996 return ret; 1997 } 1998 1999 if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & value)) 2000 return -EINVAL; 2001 } 2002 2003 break; 2004 2005 default: 2006 value = xctrl->value; 2007 break; 2008 } 2009 2010 /* 2011 * If the mapping doesn't span the whole UVC control, the current value 2012 * needs to be loaded from the device to perform the read-modify-write 2013 * operation. 2014 */ 2015 if ((ctrl->info.size * 8) != mapping->size) { 2016 ret = __uvc_ctrl_load_cur(chain, ctrl); 2017 if (ret < 0) 2018 return ret; 2019 } 2020 2021 /* Backup the current value in case we need to rollback later. */ 2022 if (!ctrl->dirty) { 2023 memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_BACKUP), 2024 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT), 2025 ctrl->info.size); 2026 } 2027 2028 mapping->set(mapping, value, 2029 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)); 2030 2031 if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS) 2032 ctrl->handle = handle; 2033 2034 ctrl->dirty = 1; 2035 ctrl->modified = 1; 2036 return 0; 2037 } 2038
Hi John Maybe you could add a function like static bool is_relative_ptz_ctr(__u32 id) { switch (xctrl->id) { case V4L2_CID_ZOOM_CONTINUOUS: case V4L2_CID_PAN_SPEED: case V4L2_CID_TILT_SPEED: return true; } return false; } to figure out if a control is relative or not, and avoid code duplication. On Fri, 29 Mar 2024 at 00:18, John Bauer via B4 Relay <devnull+johnebgood.securitylive.com@kernel.org> wrote: > > From: John Bauer <johnebgood@securitylive.com> > > The minimum UVC control value for the relative pan/tilt/zoom speeds > cannot be probed as the implementation condenses the pan and tilt > direction and speed into two 16 bit values. The minimum cannot be > set at probe time because it is probed first and the maximum is not > yet known. With this fix if a relative speed control is queried > or set the minimum is set and checked based on the additive inverse of > the maximum at that time. > > Signed-off-by: John Bauer <johnebgood@securitylive.com> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 37 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index e59a463c2761..b389ab3ee05d 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1322,9 +1322,25 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, > break; > } > > - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) > - v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, > - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); > + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) { > + switch (v4l2_ctrl->id) { > + case V4L2_CID_ZOOM_CONTINUOUS: > + case V4L2_CID_PAN_SPEED: > + case V4L2_CID_TILT_SPEED: > + /* > + * For the relative speed implementation the minimum > + * value cannot be probed so it becomes the additive > + * inverse of maximum. > + */ > + v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX, > + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); > + break; > + default: > + v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, > + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); > + break; > + } > + } > > if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) > v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX, > @@ -1914,6 +1930,21 @@ int uvc_ctrl_set(struct uvc_fh *handle, > uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); > max = mapping->get(mapping, UVC_GET_MAX, > uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); > + > + /* > + * For the relative speed implementation the minimum > + * value cannot be probed so it becomes the additive > + * inverse of maximum. > + */ > + switch (xctrl->id) { > + case V4L2_CID_ZOOM_CONTINUOUS: > + case V4L2_CID_PAN_SPEED: > + case V4L2_CID_TILT_SPEED: > + min = max * -1; > + default: > + break; > + } > + > step = mapping->get(mapping, UVC_GET_RES, > uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); > if (step == 0) > > -- > 2.34.1 > >
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index e59a463c2761..b389ab3ee05d 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1322,9 +1322,25 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, break; } - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) - v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) { + switch (v4l2_ctrl->id) { + case V4L2_CID_ZOOM_CONTINUOUS: + case V4L2_CID_PAN_SPEED: + case V4L2_CID_TILT_SPEED: + /* + * For the relative speed implementation the minimum + * value cannot be probed so it becomes the additive + * inverse of maximum. + */ + v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); + break; + default: + v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); + break; + } + } if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX, @@ -1914,6 +1930,21 @@ int uvc_ctrl_set(struct uvc_fh *handle, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN)); max = mapping->get(mapping, UVC_GET_MAX, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); + + /* + * For the relative speed implementation the minimum + * value cannot be probed so it becomes the additive + * inverse of maximum. + */ + switch (xctrl->id) { + case V4L2_CID_ZOOM_CONTINUOUS: + case V4L2_CID_PAN_SPEED: + case V4L2_CID_TILT_SPEED: + min = max * -1; + default: + break; + } + step = mapping->get(mapping, UVC_GET_RES, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); if (step == 0)