[v2,4/7] media: uvcvideo: Reorganize format descriptor parsing

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@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 40 +++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 15 deletions(-)

On Sat, 6 May 2023 at 09:14, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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@ideasonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  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
>