Re: [PATCH] Quirk-fix-for-OBSBOT-Tiny-camera-minimum-settings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi John

Thanks for your patch!!!


You probably want to run:

$ scripts/checkpatch.pl --strict -g HEAD

before sending the patch, and fix whatever it says.

On Fri, Mar 22, 2024 at 5:31 PM John Bauer <john@xxxxxx> wrote:
>

You want to add all this comment after the
--

If you use b4 https://b4.docs.kernel.org/en/latest/contributor/overview.html
(which I strongly recommend, as it it super nice :) )
It will add the cover later there automagically, plus other super
amazing features

> Greetings, this is my first patch in about 30 years so I'm probably
> going to do something wrong so apologies in advance. Below is my working
> patch tested on the OBSBOT Tiny and Tiny 4k. I'm not sure if the overrides
> can be done in a different way/place. uvc_ctrl_get_rel_speed doesn't have
> a reference to maximum when it's called.

Yeah, that is pretty inconvenient :( we can probably use something
like container_of(), to get that reference, but it would be pretty
ugly.
Other option is modify get/set so it does get a pointer to
ctrl->uvc_data... but that can be a major rework.

@Laurent, any suggesting on what would be the nicest way to implement this.

>
> I am on the OBSBOT Facebook support page which is the only way to get
> support that I can find. I'm really hoping they can fix this in the
> firmware and to fix other issues for example on the OBSBOT Tiny 4k auto
> exposure can't be set in Linux with the UVC driver even though the
> minimum shows it can be 0. I would like to see them add the ability to
> enable/disable AI tracking with a custom XU control as well.

Suddenly there are at least two people talking about this camera :)

Do you have a link to the support page where you report the bug? I am
curious about their response and if the can share how to use the AI
tracking.

There is a series to support ROI:
https://lore.kernel.org/all/20231201071907.3080126-10-yunkec@xxxxxxxxxx/
, maybe that is enough to make this work?


>
> Ricardo, I ran the output message you suggested and 0 is being returned
> for zoom and 1 for pan/tilt which is being negated in
> uvc_ctrl_get_rel_speed: return -data[first+1];
>
> [  219.164932] usb 1-4: Found UVC 1.00 device OBSBOT Tiny 4K (3564:fef4)
> [  219.222257] zoom: GET_MIN 00 0x0 00
> [  219.222261] zoom: GET_MAX 00 0x0 064
> [  219.223261] speed: GET_MIN 00 0x1 00
> [  219.223264] speed: GET_MAX 00 0xa0 00
> [  219.223313] speed: GET_MIN 00 0x1 00
> [  219.223314] speed: GET_MAX 00 0x78 00
> [  219.368848] usb 1-7.3: Found UVC 1.10 device OBSBOT Tiny (3564:fef0)
> [  219.405003] zoom: GET_MIN 00 0x0 00
> [  219.405006] zoom: GET_MAX 00 0x0 064
> [  219.405361] speed: GET_MIN 00 0x1 00
> [  219.405362] speed: GET_MAX 00 0xa0 00
> [  219.405368] speed: GET_MIN 00 0x1 00
> [  219.405369] speed: GET_MAX 00 0x78 00

Thanks for testing this out, I wonder if *all* the other devices
implementing this controls behave similar. From the spec it is not
clear to me waht get_min should return.

>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 29 +++++++++++++++++++++++++++--
>  drivers/media/usb/uvc/uvc_driver.c | 18 ++++++++++++++++++
>  drivers/media/usb/uvc/uvcvideo.h   |  1 +
>  3 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c2761..509eb7ed652a 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1322,9 +1322,22 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>                 break;
>         }
>
> -       if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)
> +       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));
> +                                       uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> +
> +               if (chain->dev->quirks & UVC_QUIRK_OBSBOT_MIN_SETTINGS) {
> +                       switch (v4l2_ctrl->id) {
> +                       case V4L2_CID_ZOOM_CONTINUOUS:
> +                       case V4L2_CID_PAN_SPEED:
> +                       case V4L2_CID_TILT_SPEED:
> +                               v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX,
> +                                                    uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> +                       default:
> +                               break;
> +                       }
> +               }
> +       }
>
>         if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)
>                 v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX,
> @@ -1914,6 +1927,18 @@ 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));
> +
> +               if(chain->dev->quirks & UVC_QUIRK_OBSBOT_MIN_SETTINGS) {
> +                       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)
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index bbd90123a4e7..8cc2fe144887 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -3120,6 +3120,24 @@ static const struct usb_device_id uvc_ids[] = {
>           .bInterfaceSubClass   = 1,
>           .bInterfaceProtocol   = 0,
>           .driver_info          = UVC_INFO_META(V4L2_META_FMT_D4XX) },
> +       /* OBSBOT Tiny 1080p pan, tilt, zoom min settings quirk */
> +       { .match_flags          = USB_DEVICE_ID_MATCH_DEVICE
> +                               | USB_DEVICE_ID_MATCH_INT_INFO,
> +         .idVendor             = 0x3564,
> +         .idProduct            = 0xfef0,
> +         .bInterfaceClass      = USB_CLASS_VIDEO,
> +         .bInterfaceSubClass   = 1,
> +         .bInterfaceProtocol   = 0,
> +         .driver_info          = UVC_INFO_QUIRK(UVC_QUIRK_OBSBOT_MIN_SETTINGS) },
> +       /* OBSBOT Tiny 4k pan, tilt, zoom min settings quirk */
> +       { .match_flags          = USB_DEVICE_ID_MATCH_DEVICE
> +                               | USB_DEVICE_ID_MATCH_INT_INFO,
> +         .idVendor             = 0x3564,
> +         .idProduct            = 0xfef4,
> +         .bInterfaceClass      = USB_CLASS_VIDEO,
> +         .bInterfaceSubClass   = 1,
> +         .bInterfaceProtocol   = 0,
> +         .driver_info          = UVC_INFO_QUIRK(UVC_QUIRK_OBSBOT_MIN_SETTINGS) },
>         /* Generic USB Video Class */
>         { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
>         { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 6fb0a78b1b00..0e2f083a5c0e 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -73,6 +73,7 @@
>  #define UVC_QUIRK_FORCE_Y8             0x00000800
>  #define UVC_QUIRK_FORCE_BPP            0x00001000
>  #define UVC_QUIRK_WAKE_AUTOSUSPEND     0x00002000
> +#define UVC_QUIRK_OBSBOT_MIN_SETTINGS 0x00004000
>
>  /* Format flags */
>  #define UVC_FMT_FLAG_COMPRESSED                0x00000001
> --
> 2.34.1
>





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux