On Sat, 6 May 2023 at 09:14, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> Format descriptor parsing has grown over time and now mixes parsing of
> frame intervals with various quirk handling. Reorganize it to make the
> code easier to follow, by parsing frame intervals first, and then
> applying fixes and quirks. No functional change is intended.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 40 +++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 9e597bbbfe07..446bd8ff128c 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -405,8 +405,27 @@ static int uvc_parse_format(struct uvc_device *dev,
> get_unaligned_le32(&buffer[17]);
> frame->bFrameIntervalType = buffer[21];
> }
> +
> + /*
> + * Copy the frame intervals.
> + *
> + * Some bogus devices report dwMinFrameInterval equal to
> + * dwMaxFrameInterval and have dwFrameIntervalStep set to
> + * zero. Setting all null intervals to 1 fixes the problem and
> + * some other divisions by zero that could happen.
> + */
> frame->dwFrameInterval = *intervals;
>
> + for (i = 0; i < n; ++i) {
> + interval = get_unaligned_le32(&buffer[26+4*i]);
> + *(*intervals)++ = interval ? interval : 1;
> + }
> +
> + /*
> + * Apply more fixes, quirks and workarounds to handle incorrect
> + * or broken descriptors.
> + */
> +
> /*
> * Several UVC chipsets screw up dwMaxVideoFrameBufferSize
> * completely. Observed behaviours range from setting the
> @@ -420,27 +439,18 @@ static int uvc_parse_format(struct uvc_device *dev,
> frame->dwMaxVideoFrameBufferSize = format->bpp
> * frame->wWidth * frame->wHeight / 8;
>
> - /*
> - * Some bogus devices report dwMinFrameInterval equal to
> - * dwMaxFrameInterval and have dwFrameIntervalStep set to
> - * zero. Setting all null intervals to 1 fixes the problem and
> - * some other divisions by zero that could happen.
> - */
> - for (i = 0; i < n; ++i) {
> - interval = get_unaligned_le32(&buffer[26+4*i]);
> - *(*intervals)++ = interval ? interval : 1;
> - }
> -
> - /*
> - * Make sure that the default frame interval stays between
> - * the boundaries.
> - */
> + /* Clamp the default frame interval to the boundaries. */
> n -= frame->bFrameIntervalType ? 1 : 2;
> frame->dwDefaultFrameInterval =
> clamp(frame->dwDefaultFrameInterval,
> frame->dwFrameInterval[0],
> frame->dwFrameInterval[n]);
>
> + /*
> + * Some devices report frame intervals that are not functional.
> + * If the corresponding quirk is set, restrict operation to the
> + * first interval only.
> + */
> if (dev->quirks & UVC_QUIRK_RESTRICT_FRAME_RATE) {
> frame->bFrameIntervalType = 1;
> frame->dwFrameInterval[0] =
> --
> Regards,
>
> Laurent Pinchart
>
--
Ricardo Ribalda
