Fix and simplify the review/cut step (#1253)

See commits

Author: LukasKalbertodt

src/steps/review/preview.tsx

@@ -1,13 +1,12 @@
 import {
-  forwardRef, useState, useRef, useEffect, useImperativeHandle, SyntheticEvent,
+  forwardRef, useState, useRef, useImperativeHandle, SyntheticEvent,
 } from "react";
 import { useTranslation } from "react-i18next";
-import { match, notNullish, useColorScheme } from "@opencast/appkit";
+import { notNullish, useColorScheme } from "@opencast/appkit";
 
 import { useStudioState } from "../../studio-state";
 import CutOutIcon from "./cut-out-icon.svg";
 import { VideoBox } from "../../ui/VideoBox";
-import { ALMOST_ZERO } from ".";
 import { SHORTCUTS, useShortcut } from "../../shortcuts";
 
 
@@ -37,12 +36,6 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>((
   const videoRefs = [useRef<HTMLVideoElement>(null), useRef<HTMLVideoElement>(null)];
   const allVideos = videoRefs.slice(0, recordings.length);
 
-  const desktopIndex = recordings.length === 2
-    ? (recordings[0].deviceType === "desktop" ? 0 : 1)
-    : null;
-
-  // The index of the last video ref that received an event (0 or 1).
-  const lastOrigin = useRef<0 | 1>();
 
   // When updating the currenTime, i.e. the play position, we want to throttle
   // this somehow. Just always setting `currentTime` is not ideal: consider
@@ -72,16 +65,16 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>((
 
   useImperativeHandle(ref, () => ({
     get currentTime() {
-      return notNullish(videoRefs[lastOrigin.current ?? 0].current?.currentTime);
+      return notNullish(videoRefs[0].current?.currentTime);
     },
     set currentTime(newTime) {
       setTime(newTime);
     },
     get duration() {
-      return notNullish(videoRefs[lastOrigin.current ?? 0].current?.duration);
+      return notNullish(videoRefs[0].current?.duration);
     },
     get isPlaying() {
-      const v = videoRefs[lastOrigin.current ?? 0].current;
+      const v = videoRefs[0].current;
       return v != null && v.currentTime > 0 && !v.paused && !v.ended;
     },
     get isReadyToPlay() {
@@ -98,79 +91,19 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>((
     },
   }));
 
-  // Some browsers don't calculate the duration for the recorded videos
-  // preventing us from seeking in the video. We force it below
-  // in the event handlers of the video elements, but we want to hold off
-  // on some effects until that calculation is done.
-  type DurationCalcState = "done" | "started";
-  const durationCalculationProgress = [
-    useRef<DurationCalcState>(),
-    useRef<DurationCalcState>(),
-  ];
-  const [durationsCalculated, setDurationsCalculated] = useState<boolean>(false);
-
   // Some logic to decide whether we currently are in a part of the video that
   // will be removed. The state will be updated in `onTimeUpdate` below and is
   // only here to trigger a rerender: the condition for rendering the overlay is
   // below.
   const isInCutRegion = (time: number) =>
     (start !== null && time < start) || (end !== null && time > end);
-  const currentTime = videoRefs[lastOrigin.current ?? 0].current?.currentTime || 0;
+  const currentTime = videoRefs[0].current?.currentTime || 0;
   const overlayVisible = isInCutRegion(currentTime);
   const [, setOverlayVisible] = useState(overlayVisible);
 
-  useEffect(() => {
-    if (durationsCalculated) {
-      onReady();
-    }
-  }, [onReady, durationsCalculated]);
-
-  // Setup backup synchronization between both video elements
-  useEffect(() => {
-    if (!durationsCalculated) {
-      return;
-    }
-
-    if (desktopIndex != null) {
-      // If we have two recordings, both will have audio. But the user doesn't
-      // want to hear audio twice, so we mute one video element. Particularly,
-      // we mute the desktop video, as there the audio/video synchronization is
-      // not as critical.
-      notNullish(videoRefs[desktopIndex].current).volume = 0;
-
-      const va = notNullish(videoRefs[0].current);
-      const vb = notNullish(videoRefs[1].current);
-
-      // We regularly check if both video elements diverge too much from one
-      // another.
-      let frameCounter = 0;
-      let fixRequest: number;
-      const fixTime = () => {
-        // Only run every 60 frames.
-        if (frameCounter % 60 === 0) {
-          // We want the difference to be below 150ms. Usually, even without
-          // this backup solution, it should be below 50ms at all time. That's
-          // what testing showed.
-          const diff = Math.abs(va.currentTime - vb.currentTime);
-          if (diff > 0.15 && lastOrigin.current != null) {
-            const origin = videoRefs[lastOrigin.current].current;
-            const target = videoRefs[lastOrigin.current === 0 ? 1 : 0].current;
-            notNullish(target).currentTime = notNullish(origin).currentTime;
-          }
-        }
-
-        frameCounter++;
-        fixRequest = window.requestAnimationFrame(fixTime);
-      };
-      fixRequest = window.requestAnimationFrame(fixTime);
-
-      return () => window.cancelAnimationFrame(fixRequest);
-    }
-  });
-
 
   const jumpInTime = (diff: number) =>
-    setTime(notNullish(videoRefs[lastOrigin.current ?? 0].current?.currentTime) + diff);
+    setTime(notNullish(videoRefs[0].current?.currentTime) + diff);
 
   // TODO: This is obviously not always correct. Finding out the FPS of the
   // recording is surprisingly tricky. And actually, browsers seem to record
@@ -210,49 +143,22 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>((
           ref={videoRefs[index]}
           key={index}
           src={recording.url}
-          onLoadedData={event => {
-            // Force the browser to calculate the duration of the stream
-            // by seeking way past its end. *fingers crossed*
-            // We reset this later in an effect. (See above.)
-            // Also without setting the current time once initially,
-            // some browsers show a black video element instead of the first frame.
-            event.currentTarget.currentTime = Number.MAX_VALUE;
-            durationCalculationProgress[index].current = "started";
-          }}
+          onLoadedData={() => onReady()}
           onSeeked={() => {
-            if (durationsCalculated) {
-              const isOtherSeeking = videoRefs[index == 0 ? 1 : 0].current?.seeking;
-              const queued = queuedSeek.current;
-              if (!isOtherSeeking && queued != null) {
-                allVideos.forEach(r => {
-                  if (r.current) {
-                    r.current.currentTime = queued;
-                  }
-                });
-                queuedSeek.current = null;
-              }
+            const isOtherSeeking = videoRefs[index == 0 ? 1 : 0].current?.seeking;
+            const queued = queuedSeek.current;
+            if (!isOtherSeeking && queued != null) {
+              allVideos.forEach(r => {
+                if (r.current) {
+                  r.current.currentTime = queued;
+                }
+              });
+              queuedSeek.current = null;
             }
           }}
           onTimeUpdate={event => {
-            if (durationsCalculated) {
-              setOverlayVisible(isInCutRegion(event.currentTarget.currentTime));
-              onTimeUpdate(event);
-            } else {
-              match(notNullish(durationCalculationProgress[index].current), {
-                "started": () => {
-                  event.currentTarget.currentTime = ALMOST_ZERO;
-                  durationCalculationProgress[index].current = "done";
-                },
-                "done": () => {
-                  const finishedCalculations = durationCalculationProgress
-                    .filter(p => p.current === "done")
-                    .length;
-                  if (finishedCalculations === recordings.length) {
-                    setDurationsCalculated(true);
-                  }
-                },
-              });
-            }
+            setOverlayVisible(isInCutRegion(event.currentTarget.currentTime));
+            onTimeUpdate(event);
           }}
 
           // For iOS: without the autoplay attribute, the `loadeddata` event is