Re: [uvc] Frames smaller than dwMaxVideoFrameSize

[Sergey Zakharchenko]

Hello again list,

I wrote:
> We're seeing problems using cameras based on Geo GC6500 in Linux
> kernels 4.19+ in raw mode (as opposed to H.264 mode, to avoid IP
> issues).

> Putting frame size reporting/calculations aside for now

Now, on to frame size calculations. There's a small gadget in the code
seemingly existing since the dawn of time:

https://github.com/torvalds/linux/blob/95f5cbff90b9e4324839a5c28ee3153a3c9921a5/drivers/media/usb/uvc/uvc_video.c#L119

if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) ||
(ctrl->dwMaxVideoFrameSize == 0 && stream->dev->uvc_version < 0x0110))
ctrl->dwMaxVideoFrameSize = frame->dwMaxVideoFrameBufferSize;

It seems to be overwriting a closer-to-the-truth value of 3110412 in
dwMaxVideoFrameSize with frame->dwMaxVideoFrameBufferSize which
happens to be 4147200 and is used from then on, including the
uvc_video_validate_buffer() check. FWIW uvc_version is 0x0110. The
value is still not quite 3110400, but even if it were so (e.g. to
please uvc_video_validate_buffer()), it would have been overwritten
anyway. The condition block logic seems worth double-checking (sure
it's || not &&?).

My original question on whether a value below the maximum is valid
remains (hope the answer is obvious).

Best regards,

--
DoubleF

[uvc] Frames smaller than dwMaxVideoFrameSize

[Sergey Zakharchenko]

Hello list,

We're seeing problems using cameras based on Geo GC6500 in Linux
kernels 4.19+ in raw mode (as opposed to H.264 mode, to avoid IP
issues). We have traced that to commit
95f5cbff90b9e4324839a5c28ee3153a3c9921a5 "media: uvcvideo: Also
validate buffers in BULK mode". The uvc_video_validate_buffer()
equality check fails (e.g. for 1920x1080 NV12 dwMaxVideoFrameSize is
4147200, bytesused is 3110400) and frames aren't shipped to userspace.

Putting frame size reporting/calculations aside for now, I'm curious
about the rationale of the whole uvc_video_validate_buffer() check for
size equality in general. The UVC 1.1 spec, and the variable name, say
"maximum", and I haven't found a specific statement that frames must
be exactly that size. The spec further implies that frames can indeed
legally be smaller by saying "The sender is required to toggle the
Frame ID at least every dwMaxVideoFrameSize", note how it says "at
least". The check doesn't seem legitimate, at least at a first glance.
Could you provide some background? If you wanted an heuristic to
detect broken/incomplete frames, there might be other ways to do that.

The raw video stream is otherwise (e.g. kernels before 4.19, or after
but with the equality check removed) processed just fine with e.g.
gstreamer FWIW, because the frames aren't, indeed, broken.

Interested parties CCd.

Best regards,

-- 
DoubleF

P.S. In case you're wondering, 3110400 * 8 bits= 1920*1080 * 8 bits
[8-bit Y values] + (1920*1080)/(2*2) * 16 bits [16-bit chromatic pair
values (pairs of 8-bit values) subsampled at 2x2]. Judging by NV12
format description, this looks the way it should be.