diff mbox series

media: uvcvideo: Support borderline implementation of hw timestamping

Message ID 20210817161202.49560-1-ribalda@chromium.org
State New
Headers show
Series media: uvcvideo: Support borderline implementation of hw timestamping | expand

Commit Message

Ricardo Ribalda Aug. 17, 2021, 4:12 p.m. UTC
Some cameras do not set pts and src if there is no camera data in the
buffer. They do this without clearing the PTS and SCR bits of the
header. This is probably done due to a strict/borderline interpretation
of the standard:

"STC must be captured when the first video data of a video frame is put
on the USB bus."

Eg:
buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000
buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000
buffer: 0xa7755c00 len 000668 header:0x8c stc 73779dba sof 070c pts 7376d37a

Support those cameras by only decoding the clock if there is camera data
in the buffer.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Laurent Pinchart Aug. 19, 2021, 10:54 p.m. UTC | #1
Hi Ricardo,

Thank you for the patch.

On Tue, Aug 17, 2021 at 06:12:02PM +0200, Ricardo Ribalda wrote:
> Some cameras do not set pts and src if there is no camera data in the

> buffer. They do this without clearing the PTS and SCR bits of the

> header. This is probably done due to a strict/borderline interpretation

> of the standard:

> 

> "STC must be captured when the first video data of a video frame is put

> on the USB bus."

> 

> Eg:

> buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000

> buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000

> buffer: 0xa7755c00 len 000668 header:0x8c stc 73779dba sof 070c pts 7376d37a

> 

> Support those cameras by only decoding the clock if there is camera data

> in the buffer.


Which cameras do you know to be affected ?

> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

> ---

>  drivers/media/usb/uvc/uvc_video.c | 12 ++++++++++++

>  1 file changed, 12 insertions(+)

> 

> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c

> index e16464606b14..6d0e474671a2 100644

> --- a/drivers/media/usb/uvc/uvc_video.c

> +++ b/drivers/media/usb/uvc/uvc_video.c

> @@ -490,6 +490,18 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,

>  	if (len < header_size)

>  		return;

>  

> +	/*

> +	 * Some cameras when there is no camera data in a buffer, do not

> +	 * handle properly the pts and scr. This can be due to an borderline

> +	 * interpretation of the standard: "STC must be captured when the

> +	 * first video data of a video frame is put on the USB bus."

> +	 * Due to their internal design, their firmware cannot clear the

> +	 * UVC_STREAM_PTS and UVC_STREAM_SCR bits. Which forces us to use the

> +	 * length of the buffer to decide if pts and scr are valid or not.

> +	 */

> +	if (len == header_size)

> +		return;

> +


Won't this prevent using timestamp data provided by cameras that behave
properly ?

>  	/* Extract the timestamps:

>  	 *

>  	 * - store the frame PTS in the buffer structure


-- 
Regards,

Laurent Pinchart
diff mbox series

Patch

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index e16464606b14..6d0e474671a2 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -490,6 +490,18 @@  uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	if (len < header_size)
 		return;
 
+	/*
+	 * Some cameras when there is no camera data in a buffer, do not
+	 * handle properly the pts and scr. This can be due to an borderline
+	 * interpretation of the standard: "STC must be captured when the
+	 * first video data of a video frame is put on the USB bus."
+	 * Due to their internal design, their firmware cannot clear the
+	 * UVC_STREAM_PTS and UVC_STREAM_SCR bits. Which forces us to use the
+	 * length of the buffer to decide if pts and scr are valid or not.
+	 */
+	if (len == header_size)
+		return;
+
 	/* Extract the timestamps:
 	 *
 	 * - store the frame PTS in the buffer structure