Merge pull request #31 from eehkay/fix/compare-r2-playback

fix: play r2 direct uploads in the compare versions view

app/(dashboard)/projects/[projectId]/videos/[videoId]/compare/compare-versions-page-client.tsx

@@ -29,6 +29,7 @@ import {
   DropdownMenuTrigger,
 } from '@/components/ui/dropdown-menu';
 import { resolvePublicBunnyCdnHostname } from '@/lib/bunny-cdn';
+import { isPlayableVideoUrl, resolveR2PlaybackUrl } from '@/lib/video-upload-validation';
 import { cn } from '@/lib/utils';
 
 interface Version {
@@ -91,6 +92,11 @@ function formatTime(seconds: number): string {
   return `${mins}:${secs.toString().padStart(2, '0')}`;
 }
 
+// Panels drifting past this from the source player read as out-of-sync playback.
+const MAX_PANEL_DRIFT_SECONDS = 0.35;
+// Minimum gap between two corrective seeks of the same panel.
+const RESYNC_COOLDOWN_MS = 4000;
+
 const isSafeUrl = (url: string) => {
   try {
     const parsed = new URL(url);
@@ -142,6 +148,8 @@ export default function CompareVersionsPageClient({
   const currentTimeRef = useRef(0);
   const durationRef = useRef(0);
   const lastCommitRef = useRef(0);
+  const lastSyncRef = useRef(0);
+  const resyncCooldownRef = useRef(new WeakMap<YT.Player | PlayerAdapter, number>());
 
   // Direct DOM refs for progress bar / playhead / timecode — updated in the RAF loop
   const progressBarRef = useRef<HTMLDivElement>(null);
@@ -241,7 +249,11 @@ export default function CompareVersionsPageClient({
           try {
             const t = sourcePlayer.getCurrentTime();
             const d = sourcePlayer.getDuration();
-            const playing = sourcePlayer.getPlayerState() === window.YT?.PlayerState?.PLAYING;
+            // PLAYING is 1 in the YouTube API; the numeric fallback keeps
+            // state detection working when the YT script never loads
+            // (bunny/r2-only comparisons, ad blockers).
+            const playing =
+              sourcePlayer.getPlayerState() === (window.YT?.PlayerState?.PLAYING ?? 1);
 
             // Update refs immediately — zero React overhead
             if (t !== undefined) currentTimeRef.current = t;
@@ -257,6 +269,28 @@ export default function CompareVersionsPageClient({
               }
             }
 
+            // Re-sync followers that drift from the source player — providers
+            // buffer at different speeds and drift past ~350ms reads as
+            // out-of-sync playback. The per-player cooldown keeps a follower
+            // that simply cannot keep up (slow network, HLS rebuffering) from
+            // being seeked every second, which would stutter rather than correct.
+            if (playing && t !== undefined && timestamp - lastSyncRef.current >= 1000) {
+              lastSyncRef.current = timestamp;
+              const cooldowns = resyncCooldownRef.current;
+              for (let i = 1; i < players.length; i += 1) {
+                const follower = players[i];
+                if (timestamp - (cooldowns.get(follower) ?? 0) < RESYNC_COOLDOWN_MS) continue;
+                try {
+                  if (Math.abs(follower.getCurrentTime() - t) > MAX_PANEL_DRIFT_SECONDS) {
+                    cooldowns.set(follower, timestamp);
+                    follower.seekTo(t, true);
+                  }
+                } catch {
+                  // Player not ready
+                }
+              }
+            }
+
             // Throttle React state commits to ~4 updates/sec
             if (timestamp - lastCommitRef.current >= 250) {
               lastCommitRef.current = timestamp;
@@ -296,7 +330,7 @@ export default function CompareVersionsPageClient({
     try {
       const firstPlayer = players[0];
       const state = firstPlayer.getPlayerState();
-      const playing = state === window.YT?.PlayerState?.PLAYING;
+      const playing = state === (window.YT?.PlayerState?.PLAYING ?? 1);
 
       if (playing) {
         players.forEach((p) => {
@@ -728,6 +762,13 @@ export default function CompareVersionsPageClient({
                     onRegister={registerPlayer}
                     onUnregister={unregisterPlayer}
                   />
+                ) : version.providerId === 'r2' ? (
+                  <R2Panel
+                    key={versionId}
+                    version={version}
+                    onRegister={registerPlayer}
+                    onUnregister={unregisterPlayer}
+                  />
                 ) : isSafeUrl(version.originalUrl) ? (
                   <iframe
                     src={version.originalUrl}
@@ -977,6 +1018,168 @@ function YouTubePanel({
   return <div ref={containerRef} className="w-full h-full pointer-events-none" />;
 }
 
+// Isolated R2 direct-upload panel: a plain <video> over the app's upload
+// route, mapped to the shared adapter interface. Kept separate from
+// BunnyPanel, whose HLS processing-retry logic does not apply to R2 files.
+function R2Panel({
+  version,
+  onRegister,
+  onUnregister,
+}: {
+  version: Version;
+  onRegister: (versionId: string, player: YT.Player | PlayerAdapter) => void;
+  onUnregister: (versionId: string) => void;
+}) {
+  const panelRef = useRef<HTMLDivElement>(null);
+  const videoRef = useRef<HTMLVideoElement>(null);
+  const [portraitFrameWidth, setPortraitFrameWidth] = useState<number>(0);
+  const [isPortraitSource, setIsPortraitSource] = useState(false);
+
+  useEffect(() => {
+    const panelEl = panelRef.current;
+    if (!panelEl || typeof ResizeObserver === 'undefined') return;
+
+    const updateFrameWidth = () => {
+      const panelWidth = panelEl.clientWidth;
+      const panelHeight = panelEl.clientHeight;
+      if (panelWidth <= 0 || panelHeight <= 0) return;
+      setPortraitFrameWidth(Math.min(panelWidth, panelHeight * (9 / 16)));
+    };
+
+    updateFrameWidth();
+    const observer = new ResizeObserver(updateFrameWidth);
+    observer.observe(panelEl);
+    return () => observer.disconnect();
+  }, []);
+
+  useEffect(() => {
+    const videoEl = videoRef.current;
+    if (!videoEl) return;
+
+    // Same guard the rest of the page applies before putting a URL in the DOM:
+    // proxy paths must be a well-formed upload route, anything else http(s).
+    const playbackUrl = resolveR2PlaybackUrl(version);
+    if (!isPlayableVideoUrl(playbackUrl)) {
+      console.error('Unsafe R2 playback URL, panel not registered:', playbackUrl);
+      return;
+    }
+
+    let cachedTime = 0;
+    let cachedDuration = 0;
+    let isPlaying = false;
+
+    const onLoadedMetadata = () => {
+      if (Number.isFinite(videoEl.duration) && videoEl.duration > 0) {
+        cachedDuration = videoEl.duration;
+      }
+      if (videoEl.videoWidth > 0 && videoEl.videoHeight > 0) {
+        setIsPortraitSource(videoEl.videoHeight > videoEl.videoWidth);
+      }
+    };
+    const onTimeUpdate = () => {
+      cachedTime = videoEl.currentTime || 0;
+    };
+    const onPlay = () => {
+      isPlaying = true;
+    };
+    const onPause = () => {
+      isPlaying = false;
+    };
+    const onEnded = () => {
+      isPlaying = false;
+    };
+
+    const adapter: PlayerAdapter = {
+      playVideo: () => {
+        videoEl.play().catch((err) => console.error('Error playing R2 panel video:', err));
+      },
+      pauseVideo: () => videoEl.pause(),
+      seekTo: (time: number) => {
+        cachedTime = time;
+        videoEl.currentTime = time;
+      },
+      mute: () => {
+        videoEl.muted = true;
+      },
+      unMute: () => {
+        videoEl.muted = false;
+      },
+      isMuted: () => videoEl.muted,
+      getCurrentTime: () => videoEl.currentTime || cachedTime,
+      getDuration: () => {
+        if (Number.isFinite(videoEl.duration) && videoEl.duration > 0) {
+          cachedDuration = videoEl.duration;
+        }
+        return cachedDuration;
+      },
+      getPlayerState: () =>
+        isPlaying ? (window.YT?.PlayerState?.PLAYING ?? 1) : (window.YT?.PlayerState?.PAUSED ?? 2),
+      setPlaybackRate: (rate: number) => {
+        videoEl.playbackRate = rate;
+      },
+      destroy: () => {
+        videoEl.removeEventListener('loadedmetadata', onLoadedMetadata);
+        videoEl.removeEventListener('timeupdate', onTimeUpdate);
+        videoEl.removeEventListener('play', onPlay);
+        videoEl.removeEventListener('pause', onPause);
+        videoEl.removeEventListener('ended', onEnded);
+        videoEl.removeAttribute('src');
+        videoEl.load();
+      },
+    };
+
+    videoEl.addEventListener('loadedmetadata', onLoadedMetadata);
+    videoEl.addEventListener('timeupdate', onTimeUpdate);
+    videoEl.addEventListener('play', onPlay);
+    videoEl.addEventListener('pause', onPause);
+    videoEl.addEventListener('ended', onEnded);
+
+    videoEl.src = playbackUrl;
+    videoEl.load();
+
+    onRegister(version.id, adapter);
+
+    return () => {
+      onUnregister(version.id);
+      adapter.destroy();
+    };
+  }, [version, onRegister, onUnregister]);
+
+  return (
+    <div
+      ref={panelRef}
+      className="relative w-full h-full group flex items-center justify-center bg-black"
+    >
+      <div
+        className={cn(
+          'relative flex items-center justify-center bg-black',
+          isPortraitSource ? 'h-full overflow-hidden' : 'w-full h-full'
+        )}
+        style={
+          isPortraitSource && portraitFrameWidth > 0
+            ? { width: `${portraitFrameWidth}px` }
+            : undefined
+        }
+      >
+        <video
+          ref={videoRef}
+          className="w-full h-full object-contain pointer-events-none border-0 bg-black"
+          style={{
+            pointerEvents: 'none',
+            width: '100%',
+            height: '100%',
+            objectFit: 'contain',
+            objectPosition: 'center',
+            backgroundColor: 'black',
+          }}
+          preload="metadata"
+          playsInline
+        />
+      </div>
+    </div>
+  );
+}
+
 // Isolated Bunny Stream player component per panel mapped to the shared adapter interface
 function BunnyPanel({
   version,

components/video-page-content.tsx

@@ -14,6 +14,7 @@ import { VideoPageError } from '@/components/video-page/video-page-error';
 import { GuestNameGate } from '@/components/video-page/guest-name-gate';
 import { useCommentMedia } from '@/components/video-page/hooks/use-comment-media';
 import { validateAnnotationStrokes } from '@/lib/validation';
+import { resolveR2PlaybackUrl } from '@/lib/video-upload-validation';
 import { useVersionActions } from '@/components/video-page/hooks/use-version-actions';
 import { useWatchProgress } from '@/components/video-page/hooks/use-watch-progress';
 import { useVideoPlayer } from '@/components/video-page/hooks/use-video-player';
@@ -288,18 +289,7 @@ export function VideoPageContent({
       return `https://${bunnyCdnHostname}/${activeVersion.videoId}/playlist.m3u8`;
     }
     if (activeVersion.providerId === 'r2') {
-      if (activeVersion.originalUrl.startsWith('/api/upload/video/')) {
+      return resolveR2PlaybackUrl(activeVersion);
-        return activeVersion.originalUrl;
-      }
-      if (activeVersion.originalUrl.startsWith('videos/')) {
-        const filename = activeVersion.originalUrl.slice('videos/'.length);
-        return `/api/upload/video/${filename}`;
-      }
-      if (activeVersion.videoId.startsWith('videos/')) {
-        const filename = activeVersion.videoId.slice('videos/'.length);
-        return `/api/upload/video/${filename}`;
-      }
-      return activeVersion.originalUrl;
     }
     try {
       const url = new URL(activeVersion.originalUrl);

lib/video-upload-validation.ts

@@ -57,6 +57,7 @@ export function isAllowedVideoFile(fileName: string, mime: string | undefined):
 }
 
 export const VIDEO_OBJECT_KEY_PREFIX = 'videos/';
+export const VIDEO_PROXY_PREFIX = '/api/upload/video/';
 
 const SAFE_VIDEO_BASENAME =
   /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i;
@@ -66,17 +67,50 @@ export function buildVideoObjectKey(filename: string): string {
 }
 
 export function videoProxyPathFromFilename(filename: string): string {
-  return `/api/upload/video/${filename}`;
+  return `${VIDEO_PROXY_PREFIX}${filename}`;
 }
 
 export function videoProxyPathToObjectKey(proxyPath: string): string | null {
-  const prefix = '/api/upload/video/';
+  if (!proxyPath.startsWith(VIDEO_PROXY_PREFIX)) return null;
-  if (!proxyPath.startsWith(prefix)) return null;
+  const filename = proxyPath.slice(VIDEO_PROXY_PREFIX.length);
-  const filename = proxyPath.slice(prefix.length);
   if (!SAFE_VIDEO_BASENAME.test(filename)) return null;
   return buildVideoObjectKey(filename);
 }
 
+/**
+ * Playback URL for a direct-upload (`r2`) version: media always streams through
+ * the app's own upload route. Shared by the video page and the compare view so
+ * the two cannot drift.
+ */
+export function resolveR2PlaybackUrl(version: { videoId: string; originalUrl: string }): string {
+  if (version.originalUrl.startsWith(VIDEO_PROXY_PREFIX)) {
+    return version.originalUrl;
+  }
+  if (version.originalUrl.startsWith(VIDEO_OBJECT_KEY_PREFIX)) {
+    return videoProxyPathFromFilename(version.originalUrl.slice(VIDEO_OBJECT_KEY_PREFIX.length));
+  }
+  if (version.videoId.startsWith(VIDEO_OBJECT_KEY_PREFIX)) {
+    return videoProxyPathFromFilename(version.videoId.slice(VIDEO_OBJECT_KEY_PREFIX.length));
+  }
+  return version.originalUrl;
+}
+
+/**
+ * Guards what ends up in a `<video src>`: proxy paths must be a well-formed
+ * upload route, anything else must be plain http(s).
+ */
+export function isPlayableVideoUrl(url: string): boolean {
+  if (url.startsWith(VIDEO_PROXY_PREFIX)) {
+    return videoProxyPathToObjectKey(url) !== null;
+  }
+  try {
+    const parsed = new URL(url);
+    return parsed.protocol === 'http:' || parsed.protocol === 'https:';
+  } catch {
+    return false;
+  }
+}
+
 export function objectKeyToVideoProxyPath(objectKey: string): string | null {
   if (!objectKey.startsWith(VIDEO_OBJECT_KEY_PREFIX)) return null;
   const filename = objectKey.slice(VIDEO_OBJECT_KEY_PREFIX.length);