Hi Laurent, On 18/12/2019 20:48, Laurent Pinchart wrote: > uvc_fixup_video_ctrl() needs the frame size for bandwidth calculation, > which it computes from the frame width, height and bpp. The same > calculation is done earlier and the resulting value is stored in > dwMaxVideoFrameBufferSize. Reuse it instead of recomputing it. I presume you mean earlier during uvc_parse_format()? uvc_parse_format() is called from uvc_parse_control(), while uvc_fixup_video_ctrl() is called during uvc_register_chains(), both call-paths originating from uvc_probe(). uvc_register_chains() does indeed run after uvc_parse_format() within the uvc_probe() thus the ordering is correct. However there is a slight difference in that within uvc_parse_format(), frame->dwMaxVideoFrameBufferSize is set to buffer[17] if the frame-type is UVC_VS_FRAME_FRAME_BASED, or zero otherwise. And then it gets overridden again [0] if it is a non-compressed format, to the calculation which is replaced/desired here. [0] https://elixir.bootlin.com/linux/v5.5-rc7/source/drivers/media/usb/uvc/uvc_driver.c#L627 Assuming there is no (supported) occasion where we could be calling uvc_fixup_video_ctrl() on a compressed (or non-frame based) stream type... Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > drivers/media/usb/uvc/uvc_video.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index 8fa77a81dd7f..a269aeaa9ecc 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -178,7 +178,7 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > * high-speed devices) per second and add the UVC header size > * (assumed to be 12 bytes long). > */ > - bandwidth = frame->wWidth * frame->wHeight / 8 * format->bpp; > + bandwidth = frame->dwMaxVideoFrameBufferSize; > bandwidth *= 10000000 / interval + 1; > bandwidth /= 1000; > if (stream->dev->udev->speed == USB_SPEED_HIGH) -- Regards -- Kieran
