* [PATCH 0/1] media: uvcvideo: Add quirk for exponential exposure
@ 2021-10-03 2:36 Scott K Logan
2021-10-03 2:37 ` [PATCH 1/1] " Scott K Logan
0 siblings, 1 reply; 6+ messages in thread
From: Scott K Logan @ 2021-10-03 2:36 UTC (permalink / raw)
To: Laurent Pinchart, subscribers-only, linux-media; +Cc: Scott K Logan
Ever since I started using them, I've noticed that the absolute exposure
control for the LifeCam webcam hasn't functioned properly. After some
poking around, I managed to charactarize the behavior. To summarize,
only values which follow an exponential pattern appear to result in the
intended change to the webcam's image.
Ideally this quirky behavior could be handled with an extension unit,
but I'm not sure this behavior can be implemented there. I tested this
patch with the two LifeCam webcams I have on hand.
Scott K Logan (1):
media: uvcvideo: Add quirk for exponential exposure
drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
3 files changed, 60 insertions(+)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] media: uvcvideo: Add quirk for exponential exposure
2021-10-03 2:36 [PATCH 0/1] media: uvcvideo: Add quirk for exponential exposure Scott K Logan
@ 2021-10-03 2:37 ` Scott K Logan
2021-10-17 15:27 ` Yuriy M. Kaminskiy
2021-11-24 0:59 ` [PATCH v2 " Scott K Logan
0 siblings, 2 replies; 6+ messages in thread
From: Scott K Logan @ 2021-10-03 2:37 UTC (permalink / raw)
To: Laurent Pinchart, subscribers-only, linux-media; +Cc: Scott K Logan
At least some of the Microsoft LifeCam series of webcams exhibit a
behavior which requires a 'quirk' to be handled properly. When
configuring the absolute exposure value of the image, there are only a
handful of values which will result in a consistent change to the image
exposure, while all other values appear to result in a maximum
exposure.
The valid values appear to follow an exponential pattern from the
maximum value (10000) down to the minimum, yielding less than 15
possible values depending on the device's reported minimum.
Signed-off-by: Scott K Logan <logans@cottsay.net>
---
drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
3 files changed, 60 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 30bfe9069a1f..2dfc70597858 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2142,6 +2142,40 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
return 0;
}
+/* --------------------------------------------------------------------------
+ * Quirks
+ */
+
+static s32 uvc_ctrl_get_abs_exposure_exponential(
+ struct uvc_control_mapping *mapping, u8 query, const u8 *data)
+{
+ s32 i;
+ s32 value = uvc_get_le_value(mapping, query, data);
+
+ switch (query) {
+ case UVC_GET_CUR:
+ case UVC_GET_MIN:
+ case UVC_GET_MAX:
+ case UVC_GET_DEF:
+ for (i = 0; i < 14; ++i) {
+ if (10000 >> i <= value)
+ break;
+ }
+ return 14 - i;
+ case UVC_GET_RES:
+ return 1;
+ default:
+ return value;
+ }
+}
+
+static void uvc_ctrl_set_abs_exposure_exponential(
+ struct uvc_control_mapping *mapping, s32 value, u8 *data)
+{
+ value = 10000 >> (14 - value);
+ uvc_set_le_value(mapping, value, data);
+}
+
/* --------------------------------------------------------------------------
* Control and mapping handling
*/
@@ -2210,6 +2244,13 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
}
}
+ if ((chain->dev->quirks & UVC_QUIRK_EXPONENTIAL_EXPOSURE) &&
+ ctrl->info.selector == UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL) {
+ uvc_dbg(chain->dev, CONTROL, "Applying exponential exposure quirk\n");
+ map->get = uvc_ctrl_get_abs_exposure_exponential;
+ map->set = uvc_ctrl_set_abs_exposure_exponential;
+ }
+
list_add_tail(&map->list, &ctrl->info.mappings);
uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
uvc_map_get_name(map), ctrl->info.entity,
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 7c007426e082..fa34802dfb33 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2718,6 +2718,24 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax },
+ /* Microsoft Lifecam HD-5000 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x076d,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
+ /* Microsoft Lifecam Studio */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x0772,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
/* Logitech Quickcam Fusion */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 2e5366143b81..b6d5ae0b1c90 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -209,6 +209,7 @@
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
#define UVC_QUIRK_FORCE_BPP 0x00001000
+#define UVC_QUIRK_EXPONENTIAL_EXPOSURE 0x00002000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] media: uvcvideo: Add quirk for exponential exposure
2021-10-03 2:37 ` [PATCH 1/1] " Scott K Logan
@ 2021-10-17 15:27 ` Yuriy M. Kaminskiy
2021-11-24 0:59 ` [PATCH v2 " Scott K Logan
1 sibling, 0 replies; 6+ messages in thread
From: Yuriy M. Kaminskiy @ 2021-10-17 15:27 UTC (permalink / raw)
To: Scott K Logan; +Cc: subscribers-only, linux-media
On 10/03/21 02:37 , Scott K Logan wrote:
> At least some of the Microsoft LifeCam series of webcams exhibit a
> behavior which requires a 'quirk' to be handled properly. When
> configuring the absolute exposure value of the image, there are only a
> handful of values which will result in a consistent change to the image
> exposure, while all other values appear to result in a maximum
> exposure.
> The valid values appear to follow an exponential pattern from the
> maximum value (10000) down to the minimum, yielding less than 15
> possible values depending on the device's reported minimum.
FTR, I have not tested patch (yet), but I checked
idVendor 0x045e Microsoft Corp.
idProduct 0x0810 LifeCam HD-3000
bcdDevice 1.08
iManufacturer 1
iProduct 2
iSerial 0
and it exhibit same behavior (only (1e4>>i) works, other values
interpreted as max), so you may want to add chunk below
> Signed-off-by: Scott K Logan <logans@cottsay.net>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 3 files changed, 60 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 30bfe9069a1f..2dfc70597858 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2142,6 +2142,40 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
> return 0;
> }
>
> +/* --------------------------------------------------------------------------
> + * Quirks
> + */
> +
> +static s32 uvc_ctrl_get_abs_exposure_exponential(
> + struct uvc_control_mapping *mapping, u8 query, const u8 *data)
> +{
> + s32 i;
> + s32 value = uvc_get_le_value(mapping, query, data);
> +
> + switch (query) {
> + case UVC_GET_CUR:
> + case UVC_GET_MIN:
> + case UVC_GET_MAX:
> + case UVC_GET_DEF:
> + for (i = 0; i < 14; ++i) {
> + if (10000 >> i <= value)
> + break;
> + }
> + return 14 - i;
> + case UVC_GET_RES:
> + return 1;
> + default:
> + return value;
> + }
> +}
> +
> +static void uvc_ctrl_set_abs_exposure_exponential(
> + struct uvc_control_mapping *mapping, s32 value, u8 *data)
> +{
> + value = 10000 >> (14 - value);
> + uvc_set_le_value(mapping, value, data);
> +}
> +
> /* --------------------------------------------------------------------------
> * Control and mapping handling
> */
> @@ -2210,6 +2244,13 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> }
> }
>
> + if ((chain->dev->quirks & UVC_QUIRK_EXPONENTIAL_EXPOSURE) &&
> + ctrl->info.selector == UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL) {
> + uvc_dbg(chain->dev, CONTROL, "Applying exponential exposure quirk\n");
> + map->get = uvc_ctrl_get_abs_exposure_exponential;
> + map->set = uvc_ctrl_set_abs_exposure_exponential;
> + }
> +
> list_add_tail(&map->list, &ctrl->info.mappings);
> uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> uvc_map_get_name(map), ctrl->info.entity,
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 7c007426e082..fa34802dfb33 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2718,6 +2718,24 @@ static const struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax },
> + /* Microsoft Lifecam HD-5000 */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x045e,
> + .idProduct = 0x076d,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
+ /* Microsoft Lifecam HD-3000 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x0810,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> + /* Microsoft Lifecam Studio */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x045e,
> + .idProduct = 0x0772,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> /* Logitech Quickcam Fusion */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> | USB_DEVICE_ID_MATCH_INT_INFO,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 2e5366143b81..b6d5ae0b1c90 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -209,6 +209,7 @@
> #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> #define UVC_QUIRK_FORCE_Y8 0x00000800
> #define UVC_QUIRK_FORCE_BPP 0x00001000
> +#define UVC_QUIRK_EXPONENTIAL_EXPOSURE 0x00002000
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
> --
>
> 2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] media: uvcvideo: Add quirk for exponential exposure
2021-10-03 2:37 ` [PATCH 1/1] " Scott K Logan
2021-10-17 15:27 ` Yuriy M. Kaminskiy
@ 2021-11-24 0:59 ` Scott K Logan
2021-11-24 6:30 ` Laurent Pinchart
1 sibling, 1 reply; 6+ messages in thread
From: Scott K Logan @ 2021-11-24 0:59 UTC (permalink / raw)
To: Laurent Pinchart, linux-uvc-devel, linux-media, Yuriy M Kaminskiy
Cc: Scott K Logan
At least some of the Microsoft LifeCam series of webcams exhibit a
behavior which requires a 'quirk' to be handled properly. When
configuring the absolute exposure value of the image, there are only a
handful of values which will result in a consistent change to the image
exposure, while all other values appear to result in a maximum
exposure.
The valid values appear to follow an exponential pattern from the
maximum value (10000) down to the minimum, yielding less than 15
possible values depending on the device's reported minimum.
Signed-off-by: Scott K Logan <logans@cottsay.net>
---
drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvc_driver.c | 27 ++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
3 files changed, 69 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 30bfe9069a1f..2dfc70597858 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2142,6 +2142,40 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
return 0;
}
+/* --------------------------------------------------------------------------
+ * Quirks
+ */
+
+static s32 uvc_ctrl_get_abs_exposure_exponential(
+ struct uvc_control_mapping *mapping, u8 query, const u8 *data)
+{
+ s32 i;
+ s32 value = uvc_get_le_value(mapping, query, data);
+
+ switch (query) {
+ case UVC_GET_CUR:
+ case UVC_GET_MIN:
+ case UVC_GET_MAX:
+ case UVC_GET_DEF:
+ for (i = 0; i < 14; ++i) {
+ if (10000 >> i <= value)
+ break;
+ }
+ return 14 - i;
+ case UVC_GET_RES:
+ return 1;
+ default:
+ return value;
+ }
+}
+
+static void uvc_ctrl_set_abs_exposure_exponential(
+ struct uvc_control_mapping *mapping, s32 value, u8 *data)
+{
+ value = 10000 >> (14 - value);
+ uvc_set_le_value(mapping, value, data);
+}
+
/* --------------------------------------------------------------------------
* Control and mapping handling
*/
@@ -2210,6 +2244,13 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
}
}
+ if ((chain->dev->quirks & UVC_QUIRK_EXPONENTIAL_EXPOSURE) &&
+ ctrl->info.selector == UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL) {
+ uvc_dbg(chain->dev, CONTROL, "Applying exponential exposure quirk\n");
+ map->get = uvc_ctrl_get_abs_exposure_exponential;
+ map->set = uvc_ctrl_set_abs_exposure_exponential;
+ }
+
list_add_tail(&map->list, &ctrl->info.mappings);
uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
uvc_map_get_name(map), ctrl->info.entity,
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 7c007426e082..9edf77ee30e6 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2718,6 +2718,33 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax },
+ /* Microsoft Lifecam HD-5000 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x076d,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
+ /* Microsoft Lifecam Studio */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x0772,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
+ /* Microsoft Lifecam HD-3000 */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x045e,
+ .idProduct = 0x0810,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
/* Logitech Quickcam Fusion */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 2e5366143b81..b6d5ae0b1c90 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -209,6 +209,7 @@
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
#define UVC_QUIRK_FORCE_BPP 0x00001000
+#define UVC_QUIRK_EXPONENTIAL_EXPOSURE 0x00002000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
--
2.33.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] media: uvcvideo: Add quirk for exponential exposure
2021-11-24 0:59 ` [PATCH v2 " Scott K Logan
@ 2021-11-24 6:30 ` Laurent Pinchart
2021-11-24 20:44 ` Scott K Logan
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2021-11-24 6:30 UTC (permalink / raw)
To: Scott K Logan; +Cc: linux-uvc-devel, linux-media, Yuriy M Kaminskiy
Hi Scott,
Thank you for the patch.
On Wed, Nov 24, 2021 at 12:59:05AM +0000, Scott K Logan wrote:
> At least some of the Microsoft LifeCam series of webcams exhibit a
> behavior which requires a 'quirk' to be handled properly. When
> configuring the absolute exposure value of the image, there are only a
> handful of values which will result in a consistent change to the image
> exposure, while all other values appear to result in a maximum
> exposure.
> The valid values appear to follow an exponential pattern from the
> maximum value (10000) down to the minimum, yielding less than 15
> possible values depending on the device's reported minimum.
>
> Signed-off-by: Scott K Logan <logans@cottsay.net>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
> drivers/media/usb/uvc/uvc_driver.c | 27 ++++++++++++++++++++
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 3 files changed, 69 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 30bfe9069a1f..2dfc70597858 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2142,6 +2142,40 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
> return 0;
> }
>
> +/* --------------------------------------------------------------------------
> + * Quirks
> + */
> +
> +static s32 uvc_ctrl_get_abs_exposure_exponential(
> + struct uvc_control_mapping *mapping, u8 query, const u8 *data)
> +{
> + s32 i;
> + s32 value = uvc_get_le_value(mapping, query, data);
> +
> + switch (query) {
> + case UVC_GET_CUR:
> + case UVC_GET_MIN:
> + case UVC_GET_MAX:
> + case UVC_GET_DEF:
> + for (i = 0; i < 14; ++i) {
> + if (10000 >> i <= value)
> + break;
> + }
> + return 14 - i;
> + case UVC_GET_RES:
> + return 1;
> + default:
> + return value;
> + }
> +}
> +
> +static void uvc_ctrl_set_abs_exposure_exponential(
> + struct uvc_control_mapping *mapping, s32 value, u8 *data)
> +{
> + value = 10000 >> (14 - value);
In addition to restricting the values to the ones correctly supported by
the device, this maps a linear scale (1 to 10000) to an exponential
scale (1 to 14). The V4L2 control V4L2_CID_EXPOSURE_ABSOLUTE is supposed
to be linear, and documented as expressed in 100 µs units.
Wouldn't it be better to keep the original scale (1 to 10000) and round
the requested value to the closest supported value ?
Additionally, do we have a guarantee that all the devices that need this
quirk will have the same exposure range (1 to 10000), or could the
maximum value be different ?
> + uvc_set_le_value(mapping, value, data);
> +}
> +
> /* --------------------------------------------------------------------------
> * Control and mapping handling
> */
> @@ -2210,6 +2244,13 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> }
> }
>
> + if ((chain->dev->quirks & UVC_QUIRK_EXPONENTIAL_EXPOSURE) &&
> + ctrl->info.selector == UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL) {
> + uvc_dbg(chain->dev, CONTROL, "Applying exponential exposure quirk\n");
> + map->get = uvc_ctrl_get_abs_exposure_exponential;
> + map->set = uvc_ctrl_set_abs_exposure_exponential;
> + }
> +
> list_add_tail(&map->list, &ctrl->info.mappings);
> uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> uvc_map_get_name(map), ctrl->info.entity,
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 7c007426e082..9edf77ee30e6 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2718,6 +2718,33 @@ static const struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax },
> + /* Microsoft Lifecam HD-5000 */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x045e,
> + .idProduct = 0x076d,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> + /* Microsoft Lifecam Studio */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x045e,
> + .idProduct = 0x0772,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> + /* Microsoft Lifecam HD-3000 */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x045e,
> + .idProduct = 0x0810,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> /* Logitech Quickcam Fusion */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> | USB_DEVICE_ID_MATCH_INT_INFO,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 2e5366143b81..b6d5ae0b1c90 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -209,6 +209,7 @@
> #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> #define UVC_QUIRK_FORCE_Y8 0x00000800
> #define UVC_QUIRK_FORCE_BPP 0x00001000
> +#define UVC_QUIRK_EXPONENTIAL_EXPOSURE 0x00002000
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] media: uvcvideo: Add quirk for exponential exposure
2021-11-24 6:30 ` Laurent Pinchart
@ 2021-11-24 20:44 ` Scott K Logan
0 siblings, 0 replies; 6+ messages in thread
From: Scott K Logan @ 2021-11-24 20:44 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-uvc-devel, linux-media, Yuriy M Kaminskiy
Thanks very much for taking a look, Laurent.
On Wed, 2021-11-24 at 08:30 +0200, Laurent Pinchart wrote:
> Hi Scott,
>
> Thank you for the patch.
>
> On Wed, Nov 24, 2021 at 12:59:05AM +0000, Scott K Logan wrote:
> > At least some of the Microsoft LifeCam series of webcams exhibit a
> > behavior which requires a 'quirk' to be handled properly. When
> > configuring the absolute exposure value of the image, there are only a
> > handful of values which will result in a consistent change to the image
> > exposure, while all other values appear to result in a maximum
> > exposure.
> > The valid values appear to follow an exponential pattern from the
> > maximum value (10000) down to the minimum, yielding less than 15
> > possible values depending on the device's reported minimum.
> >
> > Signed-off-by: Scott K Logan <logans@cottsay.net>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 41 ++++++++++++++++++++++++++++++
> > drivers/media/usb/uvc/uvc_driver.c | 27 ++++++++++++++++++++
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 3 files changed, 69 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 30bfe9069a1f..2dfc70597858 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -2142,6 +2142,40 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
> > return 0;
> > }
> >
> > +/* --------------------------------------------------------------------------
> > + * Quirks
> > + */
> > +
> > +static s32 uvc_ctrl_get_abs_exposure_exponential(
> > + struct uvc_control_mapping *mapping, u8 query, const u8 *data)
> > +{
> > + s32 i;
> > + s32 value = uvc_get_le_value(mapping, query, data);
> > +
> > + switch (query) {
> > + case UVC_GET_CUR:
> > + case UVC_GET_MIN:
> > + case UVC_GET_MAX:
> > + case UVC_GET_DEF:
> > + for (i = 0; i < 14; ++i) {
> > + if (10000 >> i <= value)
> > + break;
> > + }
> > + return 14 - i;
> > + case UVC_GET_RES:
> > + return 1;
> > + default:
> > + return value;
> > + }
> > +}
> > +
> > +static void uvc_ctrl_set_abs_exposure_exponential(
> > + struct uvc_control_mapping *mapping, s32 value, u8 *data)
> > +{
> > + value = 10000 >> (14 - value);
>
> In addition to restricting the values to the ones correctly supported by
> the device, this maps a linear scale (1 to 10000) to an exponential
> scale (1 to 14). The V4L2 control V4L2_CID_EXPOSURE_ABSOLUTE is supposed
> to be linear, and documented as expressed in 100 µs units.
>
> Wouldn't it be better to keep the original scale (1 to 10000) and round
> the requested value to the closest supported value ?
This is a very valid point. I'm not in a position to measure the
exposure accurately, but this morning I took the time to plot the
average pixel intensity across a series of stills at varying exposure
settings. While the data didn't look quite as linear as my laptop's
built-in camera, it was substantially closer than the data without the
scaling. If the scale is supposed to be linear, this patch will
certainly make it more so.
As a gut check, I've been operating my LifeCam camera in meetings at
the manual exposure level of 156 out of 10000. The exponentially scaled
value would be 8, which is roughly half way through the advertised
scale (1-14).
> Additionally, do we have a guarantee that all the devices that need this
> quirk will have the same exposure range (1 to 10000), or could the
> maximum value be different ?
I actually didn't think about that. It appears to be the case for both
of the models I own as well as the one Yuriy has (by the way, thanks
for your reply!).
I could change the patch to query UVC_GET_MAX for each get/set
operation if you think that might be worth it. It would obviously be
better to stash the value for later use, but I'm not sure there's a
convenient spot.
> > + uvc_set_le_value(mapping, value, data);
> > +}
> > +
> > /* --------------------------------------------------------------------------
> > * Control and mapping handling
> > */
> > @@ -2210,6 +2244,13 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> > }
> > }
> >
> > + if ((chain->dev->quirks & UVC_QUIRK_EXPONENTIAL_EXPOSURE) &&
> > + ctrl->info.selector == UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL) {
> > + uvc_dbg(chain->dev, CONTROL, "Applying exponential exposure quirk\n");
> > + map->get = uvc_ctrl_get_abs_exposure_exponential;
> > + map->set = uvc_ctrl_set_abs_exposure_exponential;
> > + }
> > +
> > list_add_tail(&map->list, &ctrl->info.mappings);
> > uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> > uvc_map_get_name(map), ctrl->info.entity,
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 7c007426e082..9edf77ee30e6 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2718,6 +2718,33 @@ static const struct usb_device_id uvc_ids[] = {
> > .bInterfaceSubClass = 1,
> > .bInterfaceProtocol = 0,
> > .driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax },
> > + /* Microsoft Lifecam HD-5000 */
> > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > + .idVendor = 0x045e,
> > + .idProduct = 0x076d,
> > + .bInterfaceClass = USB_CLASS_VIDEO,
> > + .bInterfaceSubClass = 1,
> > + .bInterfaceProtocol = 0,
> > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> > + /* Microsoft Lifecam Studio */
> > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > + .idVendor = 0x045e,
> > + .idProduct = 0x0772,
> > + .bInterfaceClass = USB_CLASS_VIDEO,
> > + .bInterfaceSubClass = 1,
> > + .bInterfaceProtocol = 0,
> > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> > + /* Microsoft Lifecam HD-3000 */
> > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > + .idVendor = 0x045e,
> > + .idProduct = 0x0810,
> > + .bInterfaceClass = USB_CLASS_VIDEO,
> > + .bInterfaceSubClass = 1,
> > + .bInterfaceProtocol = 0,
> > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_EXPONENTIAL_EXPOSURE) },
> > /* Logitech Quickcam Fusion */
> > { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > | USB_DEVICE_ID_MATCH_INT_INFO,
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 2e5366143b81..b6d5ae0b1c90 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -209,6 +209,7 @@
> > #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> > #define UVC_QUIRK_FORCE_Y8 0x00000800
> > #define UVC_QUIRK_FORCE_BPP 0x00001000
> > +#define UVC_QUIRK_EXPONENTIAL_EXPOSURE 0x00002000
> >
> > /* Format flags */
> > #define UVC_FMT_FLAG_COMPRESSED 0x00000001
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-24 20:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-03 2:36 [PATCH 0/1] media: uvcvideo: Add quirk for exponential exposure Scott K Logan
2021-10-03 2:37 ` [PATCH 1/1] " Scott K Logan
2021-10-17 15:27 ` Yuriy M. Kaminskiy
2021-11-24 0:59 ` [PATCH v2 " Scott K Logan
2021-11-24 6:30 ` Laurent Pinchart
2021-11-24 20:44 ` Scott K Logan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.