Skip to content

Commit 5093b7f

Browse files
fix: OPTIC-1608: Seeking video frame by frame sometimes produces duplicated or skipped frames visually (#7027)
Co-authored-by: yyassi-heartex <[email protected]> Co-authored-by: bmartel <[email protected]>
1 parent 68b0a2b commit 5093b7f

File tree

7 files changed

+97
-6
lines changed

7 files changed

+97
-6
lines changed
Binary file not shown.

web/libs/editor/src/components/VideoCanvas/VideoCanvas.tsx

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { forwardRef, memo, type MutableRefObject, useCallback, useEffect, useMemo, useRef, useState } from "react";
22
import { Block, Elem } from "../../utils/bem";
3-
import { FF_LSDV_4711, isFF } from "../../utils/feature-flags";
3+
import { FF_LSDV_4711, FF_VIDEO_FRAME_SEEK_PRECISION, isFF } from "../../utils/feature-flags";
44
import { clamp, isDefined } from "../../utils/utilities";
55
import "./VideoCanvas.scss";
66
import { MAX_ZOOM, MIN_ZOOM } from "./VideoConstants";
@@ -55,6 +55,10 @@ export const clampZoom = (value: number) => clamp(value, MIN_ZOOM, MAX_ZOOM);
5555
const zoomRatio = (canvasWidth: number, canvasHeight: number, width: number, height: number) =>
5656
Math.min(1, Math.min(canvasWidth / width, canvasHeight / height));
5757

58+
// Browsers can only handle time to the nearest 2ms, so we need to round to that precision when seeking by frames
59+
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/currentTime
60+
const BROWSER_TIME_PRECISION = 0.002;
61+
5862
export interface VideoRef {
5963
currentFrame: number;
6064
length: number;
@@ -167,7 +171,9 @@ export const VideoCanvas = memo(
167171
if (!contextRef.current) return;
168172

169173
const currentTime = videoRef.current?.currentTime ?? 0;
170-
const frameNumber = Math.round(currentTime * framerate);
174+
const frameNumber = isFF(FF_VIDEO_FRAME_SEEK_PRECISION)
175+
? Math.ceil(currentTime * framerate)
176+
: Math.round(currentTime * framerate);
171177
const frame = clamp(frameNumber, 1, length || 1);
172178
const onChange = props.onFrameChange ?? (() => {});
173179

@@ -415,8 +421,16 @@ export const VideoCanvas = memo(
415421
goToFrame(frame: number) {
416422
const frameClamped = clamp(frame, 1, length);
417423

418-
this.currentTime = frameClamped / framerate;
419-
requestAnimationFrame(() => drawVideo());
424+
// We need to subtract 1 from the frame number because the frame number is 1-based
425+
// and the currentTime is 0-based
426+
const frameZeroBased = frameClamped - 1;
427+
428+
// Calculate exact frame time
429+
const exactTime = frameZeroBased / framerate;
430+
431+
// Round to next closest browser precision frame time
432+
this.currentTime =
433+
Math.round(exactTime / BROWSER_TIME_PRECISION) * BROWSER_TIME_PRECISION + BROWSER_TIME_PRECISION;
420434
},
421435
};
422436

@@ -459,7 +473,9 @@ export const VideoCanvas = memo(
459473
const video = videoRef.current;
460474

461475
loadTimeout = setTimeout(() => {
462-
const length = Math.ceil(video.duration * framerate);
476+
const length = isFF(FF_VIDEO_FRAME_SEEK_PRECISION)
477+
? Math.round(video.duration * framerate)
478+
: Math.ceil(video.duration * framerate);
463479
const [width, height] = [video.videoWidth, video.videoHeight];
464480

465481
const dimensions = {

web/libs/editor/src/tags/object/Video/Video.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import IsReadyMixin from "../../../mixins/IsReadyMixin";
66
import ProcessAttrsMixin from "../../../mixins/ProcessAttrs";
77
import { SyncableMixin } from "../../../mixins/Syncable";
88
import { parseValue } from "../../../utils/data";
9+
import { FF_VIDEO_FRAME_SEEK_PRECISION, isFF } from "../../../utils/feature-flags";
910
import ObjectBase from "../Base";
1011

1112
/**
@@ -196,7 +197,11 @@ const Model = types
196197
setFrame(frame) {
197198
if (self.frame !== frame && self.framerate) {
198199
self.frame = frame;
199-
self.ref.current.currentTime = frame / self.framerate;
200+
if (isFF(FF_VIDEO_FRAME_SEEK_PRECISION)) {
201+
self.ref.current.goToFrame(frame);
202+
} else {
203+
self.ref.current.currentTime = frame / self.framerate;
204+
}
200205
}
201206
},
202207

web/libs/editor/src/utils/feature-flags.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ export const FF_PER_FIELD_COMMENTS = "fflag_feat_all_leap_1430_per_field_comment
208208

209209
export const FF_IMAGE_MEMORY_USAGE = "fflag_feat_front_optic_1479_improve_image_tag_memory_usage_short";
210210

211+
export const FF_VIDEO_FRAME_SEEK_PRECISION = "fflag_fix_front_optic_1608_improve_video_frame_seek_precision_short";
212+
211213
Object.assign(window, {
212214
APP_SETTINGS: {
213215
...(window.APP_SETTINGS ?? {}),
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { LabelStudio, VideoView } from "@humansignal/frontend-test/helpers/LSF/index";
2+
import { simpleVideoConfig } from "../../data/video_segmentation/regions";
3+
import { FF_VIDEO_FRAME_SEEK_PRECISION } from "libs/editor/src/utils/feature-flags";
4+
5+
const simpleVideoData = { video: "/public/files/fps_24_frames_24_video.webm" };
6+
7+
describe("Video Frame Seeking", () => {
8+
beforeEach(() => {
9+
LabelStudio.addFeatureFlagsOnPageLoad({
10+
[FF_VIDEO_FRAME_SEEK_PRECISION]: true,
11+
});
12+
});
13+
14+
it("Should be able to seek to a specific frame and see the correct frame without duplicating or skipping frames", () => {
15+
LabelStudio.params().config(simpleVideoConfig).data(simpleVideoData).withResult([]).init();
16+
17+
LabelStudio.waitForObjectsReady();
18+
19+
VideoView.captureVideoCanvas("video_canvas");
20+
21+
VideoView.clickAtFrame(2);
22+
23+
VideoView.videoCanvasShouldChange("video_canvas", 0);
24+
25+
VideoView.captureVideoCanvas("video_canvas");
26+
27+
VideoView.clickAtFrame(3);
28+
29+
VideoView.videoCanvasShouldChange("video_canvas", 0);
30+
31+
VideoView.captureVideoCanvas("video_canvas");
32+
33+
VideoView.clickAtFrame(4);
34+
35+
VideoView.videoCanvasShouldChange("video_canvas", 0);
36+
});
37+
});

web/libs/frontend-test/src/helpers/LSF/VideoView.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export const VideoView = {
1818
cy.log("Get VideoView's drawing area");
1919
return this.root.get(".konvajs-content");
2020
},
21+
get videoCanvas() {
22+
return this.root.get(".lsf-video-canvas");
23+
},
2124
get timelineContainer() {
2225
return this.root.get(".lsf-video-segmentation__timeline");
2326
},
@@ -134,4 +137,32 @@ export const VideoView = {
134137
canvasShouldNotChange(name: string, treshold = 0.1) {
135138
return this.drawingArea.compareScreenshot(name, "shouldNotChange", { withHidden: [".lsf-video-canvas"], treshold });
136139
},
140+
141+
/**
142+
* Captures a screenshot of the video canvas to compare later
143+
* @param {string} name name of the screenshot
144+
*/
145+
captureVideoCanvas(name: string) {
146+
return this.videoCanvas.captureScreenshot(name, { withHidden: [".konvajs-content"] });
147+
},
148+
149+
/**
150+
* Captures a new screenshot of the video canvas and compares it to already taken one
151+
* Fails if screenshots are identical
152+
* @param name name of the screenshot
153+
* @param treshold to compare image. It's a relation between original number of pixels vs changed number of pixels
154+
*/
155+
videoCanvasShouldChange(name: string, treshold = 0.1) {
156+
return this.videoCanvas.compareScreenshot(name, "shouldChange", { withHidden: [".konvajs-content"], treshold });
157+
},
158+
159+
/**
160+
* Captures a new screenshot of the video canvas and compares it to already taken one
161+
* Fails if screenshots are different
162+
* @param name name of the screenshot
163+
* @param treshold to compare image. It's a relation between original number of pixels vs changed number of pixels
164+
*/
165+
videoCanvasShouldNotChange(name: string, treshold = 0.1) {
166+
return this.videoCanvas.compareScreenshot(name, "shouldNotChange", { withHidden: [".konvajs-content"], treshold });
167+
},
137168
};

0 commit comments

Comments
 (0)