The series
https://patchwork.linuxtv.org/project/linux-media/list/?series=12485
was all merged but one patch that Laurent found a bug in the index used.
When I tried the fixed patch I found an integer overflow in the
timestamp calculations. This bug can be triggered with slow framerates.
Signed-off-by: Ricardo Ribalda <[email protected]>
---
Ricardo Ribalda (2):
media: uvcvideo: Fix hw timestamp handling for slow FPS
media: uvcvideo: Fix integer overflow calculating timestamp
drivers/media/usb/uvc/uvc_video.c | 33 ++++++++++++++++++++++++++++-----
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 29 insertions(+), 5 deletions(-)
---
base-commit: ef1e48f725d30cb18d3f2d40c48f50f483080cf7
change-id: 20240610-hwtimestamp-followup-2489c5668b21
Best regards,
--
Ricardo Ribalda <[email protected]>
In UVC 1.5 we get a single clock value per frame. With the current
buffer size of 32, FPS slowers than 32 might roll-over twice.
The current code cannot handle two roll-over and provide invalid
timestamps.
Revome all the samples from the circular buffer that are more than two
rollovers old, so the algorithm always provides good timestamps.
Note that we are removing values that are more than one second old,
which means that there is enough distance between the two points that
we use for the interpolation to provide good values.
Tested-by: HungNien Chen <[email protected]>
Reviewed-by: Sergey Senozhatsky <[email protected]>
Reviewed-by: Tomasz Figa <[email protected]>
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/uvc/uvc_video.c | 23 +++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 76a375fe47dc..869876afdcce 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -471,8 +471,30 @@ static void uvc_video_clock_add_sample(struct uvc_clock *clock,
{
unsigned long flags;
+ /*
+ * If we write new data on the position where we had the last
+ * overflow, remove the overflow pointer. There is no SOF overflow
+ * in the whole circular buffer.
+ */
+ if (clock->head == clock->last_sof_overflow)
+ clock->last_sof_overflow = -1;
+
spin_lock_irqsave(&clock->lock, flags);
+ /* Handle SOF overflows. */
+ if (clock->count > 0 && clock->last_sof > sample->dev_sof) {
+ /*
+ * Remove data from the circular buffer that is older than the
+ * last SOF overflow. We only support one SOF overflow per
+ * circular buffer.
+ */
+ if (clock->last_sof_overflow != -1)
+ clock->count = (clock->head - clock->last_sof_overflow
+ + clock->size) % clock->size;
+ clock->last_sof_overflow = clock->head;
+ }
+
+ /* Add sample. */
memcpy(&clock->samples[clock->head], sample, sizeof(*sample));
clock->head = (clock->head + 1) % clock->size;
clock->count = min(clock->count + 1, clock->size);
@@ -605,6 +627,7 @@ static void uvc_video_clock_reset(struct uvc_clock *clock)
clock->head = 0;
clock->count = 0;
clock->last_sof = -1;
+ clock->last_sof_overflow = -1;
clock->sof_offset = -1;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index e5b12717016f..f21207debd54 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -501,6 +501,7 @@ struct uvc_streaming {
unsigned int head;
unsigned int count;
unsigned int size;
+ unsigned int last_sof_overflow;
u16 last_sof;
u16 sof_offset;
--
2.45.2.505.gda0bf45e8d-goog
The function uvc_video_clock_update() supports a single SOF overflow. Or
in other words, the maximum difference between the first ant the last
timestamp can be 4096 ticks or 4.096 seconds.
This results in a maximum value for y2 of: 0x12FBECA00, that overflows
32bits.
y2 = (u32)ktime_to_ns(ktime_sub(last->host_time, first->host_time)) + y1;
Extend the size of y2 to u64 to support all its values.
Without this patch:
# yavta -s 1920x1080 -f YUYV -t 1/5 -c /dev/video0
Device /dev/v4l/by-id/usb-Shine-Optics_Integrated_Camera_0001-video-index0 opened.
Device `Integrated Camera: Integrated C' on `usb-0000:00:14.0-6' (driver 'uvcvideo') supports video, capture, without mplanes.
Video format set: YUYV (56595559) 1920x1080 (stride 3840) field none buffer size 4147200
Video format: YUYV (56595559) 1920x1080 (stride 3840) field none buffer size 4147200
Current frame rate: 1/5
Setting frame rate to: 1/5
Frame rate set: 1/5
8 buffers requested.
length: 4147200 offset: 0 timestamp type/source: mono/SoE
Buffer 0/0 mapped at address 0x7947ea94c000.
length: 4147200 offset: 4149248 timestamp type/source: mono/SoE
Buffer 1/0 mapped at address 0x7947ea557000.
length: 4147200 offset: 8298496 timestamp type/source: mono/SoE
Buffer 2/0 mapped at address 0x7947ea162000.
length: 4147200 offset: 12447744 timestamp type/source: mono/SoE
Buffer 3/0 mapped at address 0x7947e9d6d000.
length: 4147200 offset: 16596992 timestamp type/source: mono/SoE
Buffer 4/0 mapped at address 0x7947e9978000.
length: 4147200 offset: 20746240 timestamp type/source: mono/SoE
Buffer 5/0 mapped at address 0x7947e9583000.
length: 4147200 offset: 24895488 timestamp type/source: mono/SoE
Buffer 6/0 mapped at address 0x7947e918e000.
length: 4147200 offset: 29044736 timestamp type/source: mono/SoE
Buffer 7/0 mapped at address 0x7947e8d99000.
0 (0) [-] none 0 4147200 B 507.554210 508.874282 242.836 fps ts mono/SoE
1 (1) [-] none 2 4147200 B 508.886298 509.074289 0.751 fps ts mono/SoE
2 (2) [-] none 3 4147200 B 509.076362 509.274307 5.261 fps ts mono/SoE
3 (3) [-] none 4 4147200 B 509.276371 509.474336 5.000 fps ts mono/SoE
4 (4) [-] none 5 4147200 B 509.476394 509.674394 4.999 fps ts mono/SoE
5 (5) [-] none 6 4147200 B 509.676506 509.874345 4.997 fps ts mono/SoE
6 (6) [-] none 7 4147200 B 509.876430 510.074370 5.002 fps ts mono/SoE
7 (7) [-] none 8 4147200 B 510.076434 510.274365 5.000 fps ts mono/SoE
8 (0) [-] none 9 4147200 B 510.276421 510.474333 5.000 fps ts mono/SoE
9 (1) [-] none 10 4147200 B 510.476391 510.674429 5.001 fps ts mono/SoE
10 (2) [-] none 11 4147200 B 510.676434 510.874283 4.999 fps ts mono/SoE
11 (3) [-] none 12 4147200 B 510.886264 511.074349 4.766 fps ts mono/SoE
12 (4) [-] none 13 4147200 B 511.070577 511.274304 5.426 fps ts mono/SoE
13 (5) [-] none 14 4147200 B 511.286249 511.474301 4.637 fps ts mono/SoE
14 (6) [-] none 15 4147200 B 511.470542 511.674251 5.426 fps ts mono/SoE
15 (7) [-] none 16 4147200 B 511.672651 511.874337 4.948 fps ts mono/SoE
16 (0) [-] none 17 4147200 B 511.873988 512.074462 4.967 fps ts mono/SoE
17 (1) [-] none 18 4147200 B 512.075982 512.278296 4.951 fps ts mono/SoE
18 (2) [-] none 19 4147200 B 512.282631 512.482423 4.839 fps ts mono/SoE
19 (3) [-] none 20 4147200 B 518.986637 512.686333 0.149 fps ts mono/SoE
20 (4) [-] none 21 4147200 B 518.342709 512.886386 -1.553 fps ts mono/SoE
21 (5) [-] none 22 4147200 B 517.909812 513.090360 -2.310 fps ts mono/SoE
22 (6) [-] none 23 4147200 B 517.590775 513.294454 -3.134 fps ts mono/SoE
23 (7) [-] none 24 4147200 B 513.298465 513.494335 -0.233 fps ts mono/SoE
24 (0) [-] none 25 4147200 B 513.510273 513.698375 4.721 fps ts mono/SoE
25 (1) [-] none 26 4147200 B 513.698904 513.902327 5.301 fps ts mono/SoE
26 (2) [-] none 27 4147200 B 513.895971 514.102348 5.074 fps ts mono/SoE
27 (3) [-] none 28 4147200 B 514.099091 514.306337 4.923 fps ts mono/SoE
28 (4) [-] none 29 4147200 B 514.310348 514.510567 4.734 fps ts mono/SoE
29 (5) [-] none 30 4147200 B 514.509295 514.710367 5.026 fps ts mono/SoE
30 (6) [-] none 31 4147200 B 521.532513 514.914398 0.142 fps ts mono/SoE
31 (7) [-] none 32 4147200 B 520.885277 515.118385 -1.545 fps ts mono/SoE
32 (0) [-] none 33 4147200 B 520.411140 515.318336 -2.109 fps ts mono/SoE
33 (1) [-] none 34 4147200 B 515.325425 515.522278 -0.197 fps ts mono/SoE
34 (2) [-] none 35 4147200 B 515.538276 515.726423 4.698 fps ts mono/SoE
35 (3) [-] none 36 4147200 B 515.720767 515.930373 5.480 fps ts mono/SoE
Cc: [email protected]
Fixes: 66847ef013cc ("[media] uvcvideo: Add UVC timestamps support")
Signed-off-by: Ricardo Ribalda <[email protected]>
---
drivers/media/usb/uvc/uvc_video.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 869876afdcce..82a69f46718d 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -750,11 +750,11 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
unsigned long flags;
u64 timestamp;
u32 delta_stc;
- u32 y1, y2;
+ u32 y1;
u32 x1, x2;
u32 mean;
u32 sof;
- u64 y;
+ u64 y, y2;
if (!uvc_hw_timestamps_param)
return;
@@ -805,7 +805,7 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
sof = y;
uvc_dbg(stream->dev, CLOCK,
- "%s: PTS %u y %llu.%06llu SOF %u.%06llu (x1 %u x2 %u y1 %u y2 %u SOF offset %u)\n",
+ "%s: PTS %u y %llu.%06llu SOF %u.%06llu (x1 %u x2 %u y1 %u y2 %llu SOF offset %u)\n",
stream->dev->name, buf->pts,
y >> 16, div_u64((y & 0xffff) * 1000000, 65536),
sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
@@ -820,7 +820,7 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
goto done;
y1 = NSEC_PER_SEC;
- y2 = (u32)ktime_to_ns(ktime_sub(last->host_time, first->host_time)) + y1;
+ y2 = ktime_to_ns(ktime_sub(last->host_time, first->host_time)) + y1;
/*
* Interpolated and host SOF timestamps can wrap around at slightly
@@ -841,7 +841,7 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
timestamp = ktime_to_ns(first->host_time) + y - y1;
uvc_dbg(stream->dev, CLOCK,
- "%s: SOF %u.%06llu y %llu ts %llu buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n",
+ "%s: SOF %u.%06llu y %llu ts %llu buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %llu)\n",
stream->dev->name,
sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
y, timestamp, vbuf->vb2_buf.timestamp,
--
2.45.2.505.gda0bf45e8d-goog