Hi Ricardo,
On Mon, May 15, 2023 at 03:22:13PM +0200, Ricardo Ribalda wrote:
> On Sat, 6 May 2023 at 09:14, Laurent Pinchart wrote:
> >
> > The intervals pointer is incremented for each interval when parsing the
> > format descriptor. This doesn't cause any issue as such, but gets in the
> > way of constifying some pointers. Modify the parsing code to index the
> > intervals pointer as an array and increment it in one go at end of
> > parsing.
> >
> > Careful readers will notice that the maxIntervalIndex variable is set to
> > 1 instead of n - 2 when bFrameIntervalType has a zero value. This is
> > functionally equivalent, as n is equal to 3 in that case.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>
> > ---
> > drivers/media/usb/uvc/uvc_driver.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 446bd8ff128c..11e4fa007f3f 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -370,6 +370,8 @@ static int uvc_parse_format(struct uvc_device *dev,
> > */
> > while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE &&
> > buffer[2] == ftype) {
> > + unsigned int maxIntervalIndex;
> > +
> > frame = &format->frames[format->nframes];
> > if (ftype != UVC_VS_FRAME_FRAME_BASED)
> > n = buflen > 25 ? buffer[25] : 0;
> > @@ -418,7 +420,7 @@ static int uvc_parse_format(struct uvc_device *dev,
> >
> > for (i = 0; i < n; ++i) {
> > interval = get_unaligned_le32(&buffer[26+4*i]);
> > - *(*intervals)++ = interval ? interval : 1;
> > + (*intervals)[i] = interval ? interval : 1;
> > }
> >
> > /*
> > @@ -440,11 +442,11 @@ static int uvc_parse_format(struct uvc_device *dev,
> > * frame->wWidth * frame->wHeight / 8;
> >
> > /* Clamp the default frame interval to the boundaries. */
> > - n -= frame->bFrameIntervalType ? 1 : 2;
> > + maxIntervalIndex = frame->bFrameIntervalType ? n - 1 : 1;
>
> Maybe it is worth mentioning that bFrameIntervalType == 0 is a
> continuous interval. idex 0 is min and 1 is max.
I'll update the comment to
/*
* Clamp the default frame interval to the boundaries. A zero
* bFrameIntervalType value indicates a continuous frame
* interval range, with dwFrameInterval[0] storing the minimum
* value and dwFrameInterval[1] storing the maximum value.
*/
> > frame->dwDefaultFrameInterval =
> > clamp(frame->dwDefaultFrameInterval,
> > frame->dwFrameInterval[0],
> > - frame->dwFrameInterval[n]);
> > + frame->dwFrameInterval[maxIntervalIndex]);
> >
> > /*
> > * Some devices report frame intervals that are not functional.
> > @@ -463,6 +465,8 @@ static int uvc_parse_format(struct uvc_device *dev,
> > (100000000 / frame->dwDefaultFrameInterval) % 10);
> >
> > format->nframes++;
> > + *intervals += n;
> > +
> > buflen -= buffer[0];
> > buffer += buffer[0];
> > }
--
Regards,
Laurent Pinchart
