| Message ID | 20191218204804.24479-1-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: uvc: Don't recompute frame size unnecessarily | expand |
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@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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)
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)
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/usb/uvc/uvc_video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)