Merge pull request 'fix(video): stop stream-copying oversized sources into HLS' (#166) from fix/hls-copy-limits into master

Reviewed-on: #166

.env.example

@@ -29,6 +29,9 @@ EXCLUDED_DIRS=
 # ── Video / HLS ─────────────────────────────────────────────────────────
 HLS_CONCURRENCY=2
 HLS_TIMEOUT_SECONDS=900
+# Sources above this bitrate (bits/s) or 1080p are transcoded instead of
+# stream-copied, so HLS segments stay small enough to start and seek quickly.
+HLS_COPY_MAX_BITRATE=20000000
 PLAYLIST_CLEANUP_INTERVAL_SECONDS=86400
 
 # ── Telemetry ────────────────────────────────────────────────────────────

CLAUDE.md

@@ -25,14 +25,14 @@ RUST_LOG=debug cargo run
 
 ### Testing
 ```bash
-# Run all tests (requires BASE_PATH in .env)
+# Run all tests. DATABASE_URL is the only env var the suite needs; no .env required
-cargo test
+DATABASE_URL=":memory:" cargo test
 
 # Run specific test
-cargo test test_name
+DATABASE_URL=":memory:" cargo test test_name
 
 # Run tests with output
-cargo test -- --nocapture
+DATABASE_URL=":memory:" cargo test -- --nocapture
 ```
 
 ### Database Migrations
@@ -664,7 +664,7 @@ Module map:
 
 ## Testing Patterns
 
-Tests require `BASE_PATH` environment variable. Many integration tests create temporary directories and files.
+Tests need `DATABASE_URL` set; `DATABASE_URL=":memory:"` is enough to run the whole suite without a `.env` (otherwise DB-backed tests such as `reels::tests::reel_capture_preserves_owner_defaults` panic with `DATABASE_URL must be set`). Many integration tests create temporary directories and files.
 
 When testing database code:
 - Use in-memory SQLite: `DATABASE_URL=":memory:"`
@@ -708,6 +708,7 @@ Optional:
 WATCH_QUICK_INTERVAL_SECONDS=60        # Quick scan interval
 WATCH_FULL_INTERVAL_SECONDS=3600       # Full scan interval
 DATE_BACKFILL_MAX_PER_TICK=500         # Cap on canonical-date drain per watcher tick
+HLS_COPY_MAX_BITRATE=20000000          # Max source bits/s to stream-copy into HLS; above it (or >1080p) transcodes
 OTLP_OTLS_ENDPOINT=http://...          # OTLP collector for span export (any build; unset = no export)
 
 # AI Insights Configuration

README.md

@@ -69,6 +69,7 @@ They should be defined where the binary is located or above it in an `.env` file
   skip them. Delete the `*.unsupported` files to force retries (for example
   after upgrading ffmpeg or adding libheif)
 - `VIDEO_PATH` is a path where HLS playlists and video parts should be stored
+- `HLS_COPY_MAX_BITRATE` (optional) is the highest source bitrate, in bits/s, that is stream-copied into HLS as-is. Higher-bitrate sources, and any with a short side over 1080 px, are transcoded down to 1080p [default: `20000000`]
 - `GIFS_DIRECTORY` is a path where generated video GIF thumbnails should be stored
 - `BIND_URL` is the url and port to bind to (typically your own IP address)
 - `LOCAL_BIND_URL` (optional) is a second, loopback listener [default: `localhost:8088`]. Set it to an empty string to skip it, e.g. to run a second instance such as the `dev/e2e` harness alongside one that already owns 8088
@@ -242,3 +243,10 @@ unset.
   populated retroactively) [default: `2000`]. Errors don't burn the cap;
   only successful hashes count.
 
+## Testing
+`DATABASE_URL` is the only environment variable the test suite needs, and an
+in-memory SQLite database works, so no `.env` is required:
+
+```bash
+DATABASE_URL=":memory:" cargo test
+```

src/reels/render.rs

@@ -155,7 +155,8 @@ pub fn beat_filtergraph(opts: &SegmentOpts, per_photo: &[f64]) -> String {
 
 fn video_encoder_args(nvenc: bool) -> Vec<String> {
     if nvenc {
-        // p4 ≈ balanced; cq 23 ≈ libx264 crf 21. Matches the HLS transcode path.
+        // p4 ≈ balanced; cq 23 ≈ libx264 crf 21. Same quality target as the
+        // HLS transcode, which uses the faster p1 preset for throughput.
         [
             "-c:v",
             "h264_nvenc",

src/video/actors.rs

@@ -161,6 +161,34 @@ pub struct VideoStreamMeta {
     /// `r_frame_rate`, which lies on variable-framerate sources). `None`
     /// when ffprobe couldn't parse either field — caller picks a fallback.
     pub frame_rate: Option<f32>,
+    /// Coded frame size. `None` when ffprobe didn't report it.
+    pub width: Option<u32>,
+    pub height: Option<u32>,
+    /// Bits per second — the video stream's own rate when the container
+    /// reports it, else the overall format rate. `None` when unknown.
+    pub bit_rate: Option<u64>,
+}
+
+/// Longest the short side of an HLS rendition may be. Sources above this
+/// are transcoded down; the transcode scale filter enforces the same cap.
+const HLS_MAX_SHORT_SIDE: u32 = 1080;
+
+/// Default ceiling for stream-copying a source into HLS, in bits/s.
+/// Overridable via `HLS_COPY_MAX_BITRATE`.
+const HLS_COPY_MAX_BITRATE_DEFAULT: u64 = 20_000_000;
+
+/// Whether a source is small enough to stream-copy into HLS as-is.
+///
+/// Copy keeps the source's resolution and bitrate, so a 4K/100+ Mbps
+/// mirrorless clip would become ~50 MB 3-second segments that stall
+/// playback start and seeks on anything slower than wired gigabit.
+/// Unknown size or bitrate counts as too big — transcoding is the safe
+/// fallback, same as the GOP probe.
+fn fits_hls_copy_limits(meta: &VideoStreamMeta, max_bit_rate: u64) -> bool {
+    let (Some(w), Some(h), Some(br)) = (meta.width, meta.height, meta.bit_rate) else {
+        return false;
+    };
+    w.min(h) <= HLS_MAX_SHORT_SIDE && br <= max_bit_rate
 }
 
 /// Parse ffprobe's rational frame-rate strings (`"30000/1001"`,
@@ -196,9 +224,10 @@ pub async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta {
         // network. For non-faststart phone clips on an SMB mount that turned a
         // metadata probe into a full-file read (tens of seconds per open). The
         // Display Matrix rotation we need is present at stream level, so this
-        // keeps codec/fps/rotation while reading only the header.
+        // keeps codec/fps/rotation while reading only the header. Size and
+        // bit_rate (stream and format level) are header fields too.
         .arg("-show_entries")
-        .arg("stream=codec_name,r_frame_rate,avg_frame_rate:stream_tags=rotate:stream_side_data_list")
+        .arg("stream=codec_name,r_frame_rate,avg_frame_rate,width,height,bit_rate:stream_tags=rotate:stream_side_data_list:format=bit_rate")
         .arg(video_path)
         .output()
         .await;
@@ -262,15 +291,38 @@ pub async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta {
                 .and_then(parse_ffprobe_rational)
         });
 
+    let dimension = |key: &str| {
+        stream
+            .get(key)
+            .and_then(|v| v.as_u64())
+            .and_then(|v| u32::try_from(v).ok())
+            .filter(|v| *v > 0)
+    };
+    let width = dimension("width");
+    let height = dimension("height");
+
+    // ffprobe reports bit_rate as a decimal string. Prefer the video
+    // stream's own rate; fall back to the container's overall rate.
+    let parse_bit_rate = |v: &serde_json::Value| {
+        v.get("bit_rate")
+            .and_then(|b| b.as_str())
+            .and_then(|s| s.parse::<u64>().ok())
+            .filter(|b| *b > 0)
+    };
+    let bit_rate = parse_bit_rate(stream).or_else(|| parse_bit_rate(&json["format"]));
+
     debug!(
-        "Probed {}: codec_h264={}, rotation={}°, fps={:?}",
+        "Probed {}: codec_h264={}, rotation={}°, fps={:?}, size={:?}x{:?}, bit_rate={:?}",
-        video_path, is_h264, rotation, frame_rate
+        video_path, is_h264, rotation, frame_rate, width, height, bit_rate
     );
 
     VideoStreamMeta {
         is_h264,
         rotation,
         frame_rate,
+        width,
+        height,
+        bit_rate,
     }
 }
 
@@ -507,12 +559,20 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
             let rotation = stream_meta.rotation;
             let has_rotation = rotation != 0;
 
+            // Stream-copy keeps the source's resolution and bitrate, so only
+            // copy sources that are already a sensible streaming size.
+            let max_copy_bit_rate = std::env::var("HLS_COPY_MAX_BITRATE")
+                .ok()
+                .and_then(|v| v.parse::<u64>().ok())
+                .unwrap_or(HLS_COPY_MAX_BITRATE_DEFAULT);
+            let size_ok = fits_hls_copy_limits(&stream_meta, max_copy_bit_rate);
+
             // Stream-copy is only safe when the source GOP fits inside a
             // single HLS segment. Otherwise ffmpeg has to extend segments
             // past hls_time to land on a keyframe, producing uneven
             // segments and seeking glitches.
             const HLS_SEGMENT_SECONDS: f64 = 3.0;
-            let gop_ok = if is_h264 && !has_rotation {
+            let gop_ok = if is_h264 && !has_rotation && size_ok {
                 match get_max_gop_seconds(&video_file).await {
                     Some(g) if g > HLS_SEGMENT_SECONDS => {
                         info!(
@@ -536,7 +596,7 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
                 false
             };
 
-            let use_copy = is_h264 && !has_rotation && gop_ok;
+            let use_copy = is_h264 && !has_rotation && size_ok && gop_ok;
 
             if has_rotation {
                 info!(
@@ -550,6 +610,12 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
             } else if use_copy {
                 info!("Video {} is already h264, using stream copy", video_file);
                 span.add_event("Using stream copy (h264 detected)", vec![]);
+            } else if is_h264 && !size_ok {
+                info!(
+                    "Video {} is h264 but too large to copy ({:?}x{:?} @ {:?} bps), transcoding down",
+                    video_file, stream_meta.width, stream_meta.height, stream_meta.bit_rate
+                );
+                span.add_event("Transcoding to reduce size/bitrate", vec![]);
             } else if is_h264 {
                 info!(
                     "Video {} is h264 but needs transcoding for GOP alignment",
@@ -586,17 +652,32 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
                     cmd.arg("-rc").arg("vbr");
                     cmd.arg("-cq").arg("23");
                     cmd.arg("-pix_fmt").arg("yuv420p");
+                    // Without this NVENC turns -force_key_frames into plain
+                    // I-frames, which the HLS muxer can't split on: segments
+                    // fall back to NVENC's 250-frame GOP (8.34 s at 29.97).
+                    cmd.arg("-forced-idr").arg("1");
                 } else {
                     cmd.arg("-c:v").arg("h264");
                     cmd.arg("-crf").arg("21");
                     cmd.arg("-preset").arg("veryfast");
                 }
-                cmd.arg("-vf").arg("scale='min(1080,iw)':-2,setsar=1:1");
+                // Cap the short side so landscape and portrait both land at
+                // real 1080p. iw/ih are post-autorotation display dims.
+                cmd.arg("-vf").arg(format!(
+                    "scale='if(gt(iw,ih),-2,min({m},iw))':'if(gt(iw,ih),min({m},ih),-2)',setsar=1:1",
+                    m = HLS_MAX_SHORT_SIDE
+                ));
                 cmd.arg("-c:a").arg("aac");
                 // Force an IDR frame every hls_time seconds so each HLS
                 // segment starts on a keyframe — accurate seeking without
                 // players having to decode from a prior segment.
                 cmd.arg("-force_key_frames").arg("expr:gte(t,n_forced*3)");
+                // Keep source frame timestamps. Variable-frame-rate phone
+                // clips and screen recordings report r_frame_rate as their
+                // timebase (90000/1); the mpegts muxer's default CFR mode
+                // then duplicates frames to fill it — a 2.6 s clip came out
+                // as 233k frames / 221 MB. vfr leaves timing untouched.
+                cmd.arg("-fps_mode").arg("vfr");
             }
 
             // -f hls is required because the playlist is written to a .tmp
@@ -867,7 +948,47 @@ impl Handler<GeneratePreviewClipMessage> for PreviewClipGenerator {
 
 #[cfg(test)]
 mod tests {
-    use super::parse_ffprobe_rational;
+    use super::{VideoStreamMeta, fits_hls_copy_limits, parse_ffprobe_rational};
+
+    const LIMIT: u64 = 20_000_000;
+
+    fn meta(width: u32, height: u32, bit_rate: u64) -> VideoStreamMeta {
+        VideoStreamMeta {
+            width: Some(width),
+            height: Some(height),
+            bit_rate: Some(bit_rate),
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn copies_phone_sized_sources() {
+        assert!(fits_hls_copy_limits(&meta(1920, 1080, 16_000_000), LIMIT));
+        // Portrait 1080p: short side is the width.
+        assert!(fits_hls_copy_limits(&meta(1080, 1920, 16_000_000), LIMIT));
+    }
+
+    #[test]
+    fn transcodes_4k_sources() {
+        assert!(!fits_hls_copy_limits(&meta(3840, 2160, 16_000_000), LIMIT));
+        assert!(!fits_hls_copy_limits(&meta(2160, 3840, 16_000_000), LIMIT));
+    }
+
+    #[test]
+    fn transcodes_high_bitrate_1080p() {
+        // Mirrorless 1080p at 100 Mbps fits the size cap but not the bitrate.
+        assert!(!fits_hls_copy_limits(&meta(1920, 1080, 100_000_000), LIMIT));
+    }
+
+    #[test]
+    fn transcodes_when_size_or_bitrate_unknown() {
+        assert!(!fits_hls_copy_limits(&VideoStreamMeta::default(), LIMIT));
+        let no_bit_rate = VideoStreamMeta {
+            bit_rate: None,
+            ..meta(1920, 1080, 0)
+        };
+        assert!(!fits_hls_copy_limits(&no_bit_rate, LIMIT));
+    }
 
     #[test]
     fn parses_common_rational_framerates() {

src/video/ffmpeg.rs

@@ -1,7 +1,7 @@
 use futures::TryFutureExt;
 use log::{debug, error, info, warn};
 use std::io::Result;
-use std::process::{Output, Stdio};
+use std::process::Stdio;
 use std::sync::OnceLock;
 use std::time::Instant;
 use tokio::process::Command;
@@ -47,37 +47,6 @@ pub enum GifType {
 }
 
 impl Ffmpeg {
-    async fn _generate_playlist(&self, input_file: &str, output_file: &str) -> Result<String> {
-        let ffmpeg_result: Result<Output> = Command::new("ffmpeg")
-            .arg("-i")
-            .arg(input_file)
-            .arg("-c:v")
-            .arg("h264")
-            .arg("-crf")
-            .arg("21")
-            .arg("-preset")
-            .arg("veryfast")
-            .arg("-hls_time")
-            .arg("3")
-            .arg("-hls_list_size")
-            .arg("100")
-            .arg("-vf")
-            .arg("scale=1080:-2,setsar=1:1")
-            .arg(output_file)
-            .stdout(Stdio::null())
-            .stderr(Stdio::piped())
-            .output()
-            .inspect_err(|e| error!("Failed to run ffmpeg on child process: {}", e))
-            .map_err(|e| std::io::Error::other(e.to_string()))
-            .await;
-
-        if let Ok(ref res) = ffmpeg_result {
-            debug!("ffmpeg output: {:?}", res);
-        }
-
-        ffmpeg_result.map(|_| output_file.to_string())
-    }
-
     async fn get_video_duration(&self, input_file: &str) -> Result<u32> {
         Command::new("ffprobe")
             .args(["-i", input_file])