Combine Player and VideoPreview components.

This makes sense semantically and also simplifies the component
structure as it avoids leaking the Video reference outside of the
component (at least for now).

frontend/src/App.tsx

@@ -6,12 +6,11 @@ import {
 } from './generated/media_set';
 
 import { useEffect, useCallback, useReducer } from 'react';
-import { State, stateReducer, zoomFactor } from './AppState';
+import { State, stateReducer, zoomFactor, PlayState } from './AppState';
 import { AudioFormat } from './generated/media_set';
-import { VideoPreview } from './VideoPreview';
 import { WaveformCanvas } from './WaveformCanvas';
 import { HudCanvas } from './HudCanvas';
-import { Player, PlayState } from './Player';
+import { Player } from './Player';
 import {
   CanvasWidth,
   CanvasHeight,
@@ -21,7 +20,6 @@ import { ControlBar } from './ControlBar';
 import { SeekBar } from './SeekBar';
 import { firstValueFrom, from, Observable } from 'rxjs';
 import { first, map, bufferCount } from 'rxjs/operators';
-import millisFromDuration from './helpers/millisFromDuration';
 import { canZoomViewportIn, canZoomViewportOut } from './helpers/zoom';
 import toHHMMSS from './helpers/toHHMMSS';
 import framesToDuration from './helpers/framesToDuration';
@@ -280,15 +278,6 @@ function App(): JSX.Element {
 
   return (
     <>
-      <Player
-        playState={playState}
-        audioSrc={state.audioSrc}
-        videoSrc={state.videoSrc}
-        currentTime={state.currentTime}
-        onPositionChanged={(currentTime) =>
-          dispatch({ type: 'positionchanged', currentTime: currentTime })
-        }
-      />
       <div className="App bg-gray-800 h-screen flex flex-col">
         <header className="bg-green-900 h-16 grow-0 flex items-center mb-12 px-[88px]">
           <h1 className="text-3xl font-bold">Clipper</h1>
@@ -403,11 +392,15 @@ function App(): JSX.Element {
             }}
           />
 
-          <VideoPreview
+          <Player
             mediaSet={mediaSet}
-            video={document.createElement('video')}
+            playState={playState}
-            position={position}
+            audioSrc={state.audioSrc}
-            duration={millisFromDuration(mediaSet.videoDuration)}
+            videoSrc={state.videoSrc}
+            currentTime={state.currentTime}
+            onPositionChanged={(currentTime) =>
+              dispatch({ type: 'positionchanged', currentTime: currentTime })
+            }
           />
         </div>
       </div>

frontend/src/AppState.test.ts

@@ -1,7 +1,6 @@
 import { MediaSet } from './generated/media_set';
-import { stateReducer, State } from './AppState';
+import { stateReducer, State, PlayState } from './AppState';
 import { from } from 'rxjs';
-import { PlayState } from './Player';
 import { CanvasWidth, SelectionMode } from './HudCanvasState';
 
 const initialState: State = {

frontend/src/AppState.tsx

@@ -2,7 +2,6 @@ import { MediaSet } from './generated/media_set';
 import { Observable } from 'rxjs';
 import { SelectionChangeEvent } from './HudCanvas';
 import { CanvasRange, SelectionMode, CanvasWidth } from './HudCanvasState';
-import { PlayState } from './Player';
 import { zoomViewportIn, zoomViewportOut } from './helpers/zoom';
 import frameToWaveformCanvasX from './helpers/frameToWaveformCanvasX';
 
@@ -21,6 +20,11 @@ interface Position {
   percent: number;
 }
 
+export enum PlayState {
+  Paused,
+  Playing,
+}
+
 export interface State {
   mediaSet?: MediaSet;
   selection: FrameRange;

frontend/src/ControlBar.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { PlayState } from './Player';
+import { PlayState } from './AppState';
 import {
   CloudDownloadIcon,
   FastForwardIcon,

frontend/src/Player.tsx

@@ -1,6 +1,11 @@
+import { MediaSet, MediaSetServiceClientImpl } from './generated/media_set';
+import { newRPC } from './App';
+import { PlayState } from './AppState';
 import { useEffect, useRef } from 'react';
+import millisFromDuration from './helpers/millisFromDuration';
 
 interface Props {
+  mediaSet: MediaSet;
   playState: PlayState;
   audioSrc: string;
   videoSrc: string;
@@ -9,20 +14,20 @@ interface Props {
   onPositionChanged: (currentTime: number) => void;
 }
 
-export enum PlayState {
-  Paused,
-  Playing,
-}
-
 const triggerCallbackIntervalMillis = 20;
 
+// eslint is complaining about prop validation which doesn't make much sense.
+/* eslint-disable react/prop-types */
 export const Player: React.FC<Props> = ({
+  mediaSet,
   playState,
   audioSrc,
   videoSrc,
   currentTime,
   onPositionChanged,
 }) => {
+  const canvasRef = useRef<HTMLCanvasElement>(null);
+
   const audioRef = useRef(new Audio());
   const videoRef = useRef(document.createElement('video'));
 
@@ -75,5 +80,76 @@ export const Player: React.FC<Props> = ({
     onPositionChanged(currentTime);
   }, [currentTime]);
 
-  return null;
+  // render canvas
+  useEffect(() => {
+    // TODO: not sure if requestAnimationFrame is recommended here.
+    requestAnimationFrame(() => {
+      (async function () {
+        if (!mediaSet) {
+          return;
+        }
+
+        const canvas = canvasRef.current;
+        if (canvas == null) {
+          console.error('no canvas ref available');
+          return;
+        }
+
+        const ctx = canvas.getContext('2d');
+        if (ctx == null) {
+          console.error('no 2d context available');
+          return;
+        }
+
+        // Set aspect ratio.
+        canvas.width =
+          canvas.height * (canvas.clientWidth / canvas.clientHeight);
+
+        // If the required position is 0, display the thumbnail instead of
+        // trying to render the video. The most important use case is before a
+        // click event has happened, when autoplay restrictions will prevent
+        // the video being rendered to canvas.
+        if (videoRef.current.currentTime == 0) {
+          const service = new MediaSetServiceClientImpl(newRPC());
+          const thumbnail = await service.GetVideoThumbnail({
+            id: mediaSet.id,
+          });
+
+          // TODO: avoid fetching the image every re-render:
+          const url = URL.createObjectURL(
+            new Blob([thumbnail.image], { type: 'image/jpeg' })
+          );
+          const img = new Image(thumbnail.width, thumbnail.height);
+
+          img.src = url;
+          img.onload = () => ctx.drawImage(img, 0, 0, 177, 100);
+          return;
+        }
+
+        // otherwise, render the video, which (should) work now.
+        const duration = millisFromDuration(mediaSet.videoDuration);
+        const durSecs = duration / 1000;
+        const ratio = videoRef.current.currentTime / durSecs;
+        const x = (canvas.width - 177) * ratio;
+        ctx.clearRect(0, 0, x, canvas.height);
+        ctx.clearRect(x + 177, 0, canvas.width - 177 - x, canvas.height);
+        ctx.drawImage(videoRef.current, x, 0, 177, 100);
+      })();
+    });
+  }, [mediaSet, videoRef.current.currentTime]);
+
+  // render component
+
+  return (
+    <>
+      <div className={`relative grow-0 h-[100px]`}>
+        <canvas
+          className="absolute block w-full h-full"
+          width="500"
+          height="100"
+          ref={canvasRef}
+        ></canvas>
+      </div>
+    </>
+  );
 };

frontend/src/VideoPreview.tsx

@@ -1,89 +0,0 @@
-import { MediaSet, MediaSetServiceClientImpl } from './generated/media_set';
-import { newRPC, VideoPosition } from './App';
-import { useEffect, useRef } from 'react';
-
-interface Props {
-  mediaSet: MediaSet;
-  position: VideoPosition;
-  duration: number;
-  video: HTMLVideoElement;
-}
-
-export const VideoPreview: React.FC<Props> = ({
-  mediaSet,
-  position,
-  duration,
-  video,
-}: Props) => {
-  const videoCanvasRef = useRef<HTMLCanvasElement>(null);
-
-  // effects
-
-  // render canvas
-  useEffect(() => {
-    // TODO: not sure if requestAnimationFrame is recommended here.
-    requestAnimationFrame(() => {
-      (async function () {
-        const canvas = videoCanvasRef.current;
-        if (canvas == null) {
-          console.error('no canvas ref available');
-          return;
-        }
-
-        const ctx = canvas.getContext('2d');
-        if (ctx == null) {
-          console.error('no 2d context available');
-          return;
-        }
-
-        // Set aspect ratio.
-        canvas.width =
-          canvas.height * (canvas.clientWidth / canvas.clientHeight);
-
-        // If the required position is 0, display the thumbnail instead of
-        // trying to render the video. The most important use case is before a
-        // click event has happened, when autoplay restrictions will prevent
-        // the video being rendered to canvas.
-        if (position.currentTime == 0) {
-          const service = new MediaSetServiceClientImpl(newRPC());
-          const thumbnail = await service.GetVideoThumbnail({
-            id: mediaSet.id,
-          });
-
-          // TODO: avoid fetching the image every re-render:
-          const url = URL.createObjectURL(
-            new Blob([thumbnail.image], { type: 'image/jpeg' })
-          );
-          const img = new Image(thumbnail.width, thumbnail.height);
-
-          img.src = url;
-          img.onload = () => ctx.drawImage(img, 0, 0, 177, 100);
-          return;
-        }
-
-        // otherwise, render the video, which (should) work now.
-        const durSecs = duration / 1000;
-        const ratio = position.currentTime / durSecs;
-        const x = (canvas.width - 177) * ratio;
-        ctx.clearRect(0, 0, x, canvas.height);
-        ctx.clearRect(x + 177, 0, canvas.width - 177 - x, canvas.height);
-        ctx.drawImage(video, x, 0, 177, 100);
-      })();
-    });
-  }, [mediaSet, position.currentTime]);
-
-  // render component
-
-  return (
-    <>
-      <div className={`relative grow-0 h-[100px]`}>
-        <canvas
-          className="absolute block w-full h-full"
-          width="500"
-          height="100"
-          ref={videoCanvasRef}
-        ></canvas>
-      </div>
-    </>
-  );
-};