fix: handle transcode for videos without audio track

apps/api/tests/test_ffmpeg_transcoder.py

@@ -0,0 +1,189 @@
+"""
+Tests for FFmpegTranscoder: command construction and error handling.
+
+Verifies:
+- has_audio=True  → ffmpeg cmd includes -map a:0, var_stream_map has audio tracks
+- has_audio=False → ffmpeg cmd excludes -map a:0, var_stream_map is video-only
+- _run() uses errors='replace' to survive Latin-1/Shift-JIS metadata
+- _run() raises RuntimeError with stderr on non-zero exit
+"""
+import asyncio
+import json
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from packages.transcoder.ffmpeg_transcoder import FFmpegTranscoder
+from packages.transcoder.base import TranscodeJob
+
+
+def _make_job(qualities: list[str] | None = None) -> TranscodeJob:
+    return TranscodeJob(
+        media_id="media-1",
+        version_id="v1",
+        input_s3_key="uploads/video.mp4",
+        output_s3_prefix="hls/media-1/v1",
+        qualities=qualities or ["1080p", "720p", "360p"],
+    )
+
+
+# ─── _run() error handling ────────────────────────────────────────────────────
+
+def test_run_raises_on_nonzero():
+    with patch("subprocess.run") as mock_run:
+        mock_result = MagicMock()
+        mock_result.returncode = 1
+        mock_result.stderr = "ffmpeg error details"
+        mock_run.return_value = mock_result
+
+        with pytest.raises(RuntimeError, match="ffmpeg exited 1: ffmpeg error details"):
+            FFmpegTranscoder._run(["ffmpeg", "-i", "test.mp4"], label="ffmpeg")
+
+
+def test_run_uses_errors_replace():
+    """Verify that text=True + errors='replace' is passed to subprocess.run."""
+    with patch("subprocess.run") as mock_run:
+        mock_result = MagicMock()
+        mock_result.returncode = 0
+        mock_result.stderr = ""
+        mock_run.return_value = mock_result
+
+        FFmpegTranscoder._run(["ffmpeg", "-i", "test.mp4"], timeout=60, label="ffmpeg")
+
+        call_kwargs = mock_run.call_args[1]
+        assert call_kwargs["text"] is True
+        assert call_kwargs["errors"] == "replace"
+        assert call_kwargs["timeout"] == 60
+
+
+# ─── has_audio=True command construction ───────────────────────────────────────
+
+def test_transcode_with_audio_includes_audio_map():
+    """When ffprobe detects audio streams, ffmpeg cmd must include -map a:0."""
+    def mock_run_side_effect(cmd, **_kwargs):
+        # First call: video probe → return metadata
+        if "-select_streams" in cmd and cmd[cmd.index("-select_streams") + 1] == "v:0":
+            mock = MagicMock()
+            mock.returncode = 0
+            mock.stderr = ""
+            mock.stdout = json.dumps({
+                "streams": [{"r_frame_rate": "30/1", "duration": 10.0, "width": 1920, "height": 1080}],
+            })
+            return mock
+        # Second call: audio probe → return audio stream
+        if "-select_streams" in cmd and cmd[cmd.index("-select_streams") + 1] == "a":
+            mock = MagicMock()
+            mock.returncode = 0
+            mock.stderr = ""
+            mock.stdout = json.dumps({
+                "streams": [{"codec_type": "audio"}],
+            })
+            return mock
+        # Third call: main ffmpeg transcode
+        mock = MagicMock()
+        mock.returncode = 0
+        mock.stderr = ""
+        return mock
+
+    with patch("subprocess.run", side_effect=mock_run_side_effect) as mock_run:
+        s3_mock = MagicMock()
+        s3_mock.generate_presigned_url.return_value = "https://s3.example.com/uploads/video.mp4"
+        s3_mock.upload_file = MagicMock()
+
+        # Mock thumbnail generation
+        with patch("builtins.open", MagicMock()), \
+             patch("pathlib.Path.glob", return_value=[]), \
+             patch("pathlib.Path.rglob", return_value=[]), \
+             patch("pathlib.Path.mkdir"), \
+             patch("shutil.rmtree"):
+                transcoder = FFmpegTranscoder(s3_mock, "test-bucket")
+                job = _make_job(["720p"])
+                asyncio.run(transcoder.transcode(job))
+
+        # Find the main ffmpeg command call (the one with -filter_complex)
+        ffmpeg_calls = [c for c in mock_run.call_args_list
+                        if any("filter_complex" in str(a) for a in c[0][0])]
+        assert len(ffmpeg_calls) > 0
+        ffmpeg_cmd = ffmpeg_calls[0][0][0]
+
+        # Assert audio map is present
+        assert "-map" in ffmpeg_cmd
+        # Find all -map args
+        maps = [ffmpeg_cmd[i+1] for i, arg in enumerate(ffmpeg_cmd) if arg == "-map"]
+        assert "a:0" in maps, f"Expected -map a:0 in ffmpeg cmd, got maps: {maps}"
+
+        # Assert var_stream_map includes audio tracks
+        var_stream_idx = ffmpeg_cmd.index("-var_stream_map")
+        stream_map = ffmpeg_cmd[var_stream_idx + 1]
+        assert "a:0" in stream_map, f"Expected audio track in var_stream_map, got: {stream_map}"
+
+
+# ─── has_audio=False command construction ─────────────────────────────────────
+
+def test_transcode_without_audio_excludes_audio_map():
+    """When ffprobe detects no audio streams, ffmpeg cmd must NOT include -map a:0."""
+    def mock_run_side_effect(cmd, **_kwargs):
+        # First call: video probe
+        if "-select_streams" in cmd and cmd[cmd.index("-select_streams") + 1] == "v:0":
+            mock = MagicMock()
+            mock.returncode = 0
+            mock.stderr = ""
+            mock.stdout = json.dumps({
+                "streams": [{"r_frame_rate": "30/1", "duration": 10.0, "width": 1920, "height": 1080}],
+            })
+            return mock
+        # Second call: audio probe → NO audio streams
+        if "-select_streams" in cmd and cmd[cmd.index("-select_streams") + 1] == "a":
+            mock = MagicMock()
+            mock.returncode = 0
+            mock.stderr = ""
+            mock.stdout = json.dumps({"streams": []})
+            return mock
+        # Third call: main ffmpeg transcode
+        mock = MagicMock()
+        mock.returncode = 0
+        mock.stderr = ""
+        return mock
+
+    with patch("subprocess.run", side_effect=mock_run_side_effect) as mock_run:
+        s3_mock = MagicMock()
+        s3_mock.generate_presigned_url.return_value = "https://s3.example.com/uploads/video.mp4"
+        s3_mock.upload_file = MagicMock()
+
+        with patch("builtins.open", MagicMock()), \
+             patch("pathlib.Path.glob", return_value=[]), \
+             patch("pathlib.Path.rglob", return_value=[]), \
+             patch("pathlib.Path.mkdir"), \
+             patch("shutil.rmtree"):
+                transcoder = FFmpegTranscoder(s3_mock, "test-bucket")
+                job = _make_job(["720p"])
+                asyncio.run(transcoder.transcode(job))
+
+        # Find the main ffmpeg command call
+        ffmpeg_calls = [c for c in mock_run.call_args_list
+                        if any("filter_complex" in str(a) for a in c[0][0])]
+        assert len(ffmpeg_calls) > 0
+        ffmpeg_cmd = ffmpeg_calls[0][0][0]
+
+        # Assert audio map is NOT present
+        maps = [ffmpeg_cmd[i+1] for i, arg in enumerate(ffmpeg_cmd) if arg == "-map"]
+        assert "a:0" not in maps, f"Expected no -map a:0 in no-audio transcode, got maps: {maps}"
+
+        # Assert var_stream_map is video-only
+        var_stream_idx = ffmpeg_cmd.index("-var_stream_map")
+        stream_map = ffmpeg_cmd[var_stream_idx + 1]
+        assert ",a:" not in stream_map, f"Expected no audio tracks in var_stream_map, got: {stream_map}"
+
+
+# ─── _run() returns stdout on success ──────────────────────────────────────────
+
+def test_run_returns_stdout():
+    with patch("subprocess.run") as mock_run:
+        mock_result = MagicMock()
+        mock_result.returncode = 0
+        mock_result.stderr = ""
+        mock_result.stdout = "output data"
+        mock_run.return_value = mock_result
+
+        result = FFmpegTranscoder._run(["echo", "hello"], label="test")
+        assert result == "output data"

apps/web/app/share/[token]/page.tsx

@@ -52,11 +52,23 @@ interface ShareValidateResponse {
   error?: string
 }
 
+interface CommentAuthor {
+  id: string
+  name: string
+  avatar_url?: string | null
+}
+
+interface GuestAuthor {
+  id: string
+  name: string
+  email?: string
+}
+
 interface GuestComment {
   id: string
   body: string
-  guest_name: string
-  guest_email: string
+  author?: CommentAuthor | null
+  guest_author?: GuestAuthor | null
   created_at: string
   timecode_start?: number | null
 }
@@ -228,16 +240,31 @@ function GuestCommentList({ token, refreshKey }: GuestCommentListProps) {
 
   return (
     <div className="flex-1 overflow-y-auto px-4 py-3 space-y-2.5">
-      {comments.map((comment) => (
+      {comments.map((comment) => {
+        const displayName = comment.guest_author?.name || comment.author?.name || 'Unknown'
+        const avatarUrl = comment.author?.avatar_url ?? null
+        const [imgError, setImgError] = React.useState(false)
+        return (
         <div
           key={comment.id}
           className="rounded-lg bg-white/[0.03] border border-white/5 px-3 py-2.5"
         >
           <div className="flex items-center gap-2 mb-1.5">
             <div className="flex h-6 w-6 shrink-0 items-center justify-center rounded-full bg-purple-500/20 text-2xs font-medium text-purple-400">
-              {comment.guest_name.charAt(0).toUpperCase()}
+              {avatarUrl && !imgError ? (
+                /* eslint-disable-next-line @next/next/no-img-element */
+                <img
+                  src={avatarUrl}
+                  alt=""
+                  className="h-full w-full rounded-full object-cover"
+                  referrerPolicy="no-referrer"
+                  onError={() => setImgError(true)}
+                />
+              ) : (
+                displayName.charAt(0).toUpperCase()
+              )}
             </div>
-            <span className="text-xs font-medium text-zinc-200">{comment.guest_name}</span>
+            <span className="text-xs font-medium text-zinc-200">{displayName}</span>
             {comment.timecode_start != null && (
               <span className="text-2xs text-zinc-500 font-mono bg-white/5 px-1.5 py-0.5 rounded">
                 {Math.floor(comment.timecode_start / 60)}:
@@ -250,7 +277,8 @@ function GuestCommentList({ token, refreshKey }: GuestCommentListProps) {
           </div>
           <p className="text-sm text-zinc-300 leading-relaxed">{comment.body}</p>
         </div>
-      ))}
+        )
+      })}
     </div>
   )
 }
@@ -466,18 +494,99 @@ interface ShareMediaViewerProps {
 }
 
 function ShareMediaViewer({ asset, token, streamUrl, streamLoading }: ShareMediaViewerProps) {
+  const videoRef = React.useRef<HTMLVideoElement>(null)
+  const audioRef = React.useRef<HTMLAudioElement>(null)
+  const [fatalError, setFatalError] = React.useState<string | null>(null)
+
+  React.useEffect(() => {
+    if (!streamUrl || streamLoading) return
+
+    setFatalError(null)
+    const mediaEl = asset.asset_type === 'video' ? videoRef.current : audioRef.current
+    if (!mediaEl) return
+
+    const isHls = streamUrl.includes('.m3u8')
+
+    // Resolve relative stream URLs against API_URL (backend returns /stream/hls/master.m3u8?token=...)
+    const resolvedUrl = streamUrl.startsWith('/')
+      ? `${API_URL}${streamUrl}`
+      : streamUrl
+
+    let hls: any = null
+    let cancelled = false
+
+    function setupHls() {
+      import('hls.js').then(({ default: Hls }) => {
+        if (cancelled) return
+
+        if (isHls && Hls.isSupported()) {
+          hls = new Hls()
+          hls.loadSource(resolvedUrl)
+          hls.attachMedia(mediaEl!)
+
+          hls.on(Hls.Events.ERROR, (_event: any, data: any) => {
+            if (data.fatal) {
+              setFatalError(
+                data.type === Hls.ErrorTypes.NETWORK_ERROR
+                  ? 'Network error loading video'
+                  : data.type === Hls.ErrorTypes.MEDIA_ERROR
+                    ? 'Media decode error'
+                    : `Playback error: ${data.details || data.type}`
+              )
+              if (hls) {
+                hls.destroy()
+                hls = null
+              }
+            }
+          })
+        } else if (mediaEl!.canPlayType && mediaEl!.canPlayType('application/vnd.apple.mpegurl')) {
+          // Safari native HLS
+          mediaEl!.src = resolvedUrl
+        } else {
+          // Direct URL fallback (mp4, mp3, etc.)
+          mediaEl!.src = resolvedUrl
+        }
+      })
+    }
+
+    if (isHls) {
+      setupHls()
+    } else if (mediaEl) {
+      mediaEl.src = resolvedUrl
+    }
+
+    function handleMediaError() {
+      setFatalError('Media playback failed')
+    }
+    mediaEl.addEventListener('error', handleMediaError)
+
+    return () => {
+      cancelled = true
+      mediaEl.removeEventListener('error', handleMediaError)
+      if (hls) {
+        hls.destroy()
+      }
+    }
+  }, [streamUrl, streamLoading, asset.asset_type])
+
   return (
     <div className="flex-1 flex items-center justify-center bg-black min-h-0 overflow-hidden">
       {asset.asset_type === 'video' && (
         <div className="w-full h-full flex items-center justify-center">
           {streamLoading ? (
             <Loader2 className="h-8 w-8 animate-spin text-zinc-500" />
+          ) : fatalError ? (
+            <div className="flex flex-col items-center gap-2">
+              <AlertTriangle className="h-10 w-10 text-red-500" />
+              <p className="text-sm text-red-400">{fatalError}</p>
+            </div>
           ) : streamUrl ? (
             <video
-              src={streamUrl}
+              ref={videoRef}
               controls
               className="max-h-full max-w-full"
               preload="metadata"
+              playsInline
             >
               Your browser does not support video playback.
             </video>
@@ -494,6 +603,11 @@ function ShareMediaViewer({ asset, token, streamUrl, streamLoading }: ShareMedia
         <div className="w-full max-w-2xl px-8">
           {streamLoading ? (
             <Loader2 className="h-6 w-6 animate-spin text-zinc-500 mx-auto" />
+          ) : fatalError ? (
+            <div className="flex flex-col items-center gap-2">
+              <AlertTriangle className="h-10 w-10 text-red-500" />
+              <p className="text-sm text-red-400">{fatalError}</p>
+            </div>
           ) : streamUrl ? (
             <div className="space-y-6">
               <div className="flex flex-col items-center gap-3">
@@ -502,7 +616,7 @@ function ShareMediaViewer({ asset, token, streamUrl, streamLoading }: ShareMedia
                 </div>
                 <p className="text-sm font-medium text-zinc-300">{asset.name}</p>
               </div>
-              <audio src={streamUrl} controls className="w-full">
+              <audio ref={audioRef} controls className="w-full">
                 Your browser does not support audio playback.
               </audio>
             </div>