fix(video): stop stream-copying oversized sources into HLS #166

Merged
cameron merged 6 commits from fix/hls-copy-limits into master 2026-10-13 15:09:38 +00:00
6 changed files with 149 additions and 46 deletions

No files matched your search

+3
View File
@@ -29,6 +29,9 @@ EXCLUDED_DIRS=
# ── Video / HLS ───────────────────────────────────────────────────────── # ── Video / HLS ─────────────────────────────────────────────────────────
HLS_CONCURRENCY=2 HLS_CONCURRENCY=2
HLS_TIMEOUT_SECONDS=900 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 PLAYLIST_CLEANUP_INTERVAL_SECONDS=86400
# ── Telemetry ──────────────────────────────────────────────────────────── # ── Telemetry ────────────────────────────────────────────────────────────
+6 -5
View File
@@ -25,14 +25,14 @@ RUST_LOG=debug cargo run
### Testing ### Testing
```bash ```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 # Run specific test
cargo test test_name DATABASE_URL=":memory:" cargo test test_name
# Run tests with output # Run tests with output
cargo test -- --nocapture DATABASE_URL=":memory:" cargo test -- --nocapture
``` ```
### Database Migrations ### Database Migrations
@@ -664,7 +664,7 @@ Module map:
## Testing Patterns ## 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: When testing database code:
- Use in-memory SQLite: `DATABASE_URL=":memory:"` - Use in-memory SQLite: `DATABASE_URL=":memory:"`
@@ -708,6 +708,7 @@ Optional:
WATCH_QUICK_INTERVAL_SECONDS=60 # Quick scan interval WATCH_QUICK_INTERVAL_SECONDS=60 # Quick scan interval
WATCH_FULL_INTERVAL_SECONDS=3600 # Full scan interval WATCH_FULL_INTERVAL_SECONDS=3600 # Full scan interval
DATE_BACKFILL_MAX_PER_TICK=500 # Cap on canonical-date drain per watcher tick 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) OTLP_OTLS_ENDPOINT=http://... # OTLP collector for span export (any build; unset = no export)
# AI Insights Configuration # AI Insights Configuration
+8
View File
@@ -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 skip them. Delete the `*.unsupported` files to force retries (for example
after upgrading ffmpeg or adding libheif) after upgrading ffmpeg or adding libheif)
- `VIDEO_PATH` is a path where HLS playlists and video parts should be stored - `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 - `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) - `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 - `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; populated retroactively) [default: `2000`]. Errors don't burn the cap;
only successful hashes count. 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
```
+2 -1
View File
@@ -155,7 +155,8 @@ pub fn beat_filtergraph(opts: &SegmentOpts, per_photo: &[f64]) -> String {
fn video_encoder_args(nvenc: bool) -> Vec<String> { fn video_encoder_args(nvenc: bool) -> Vec<String> {
if nvenc { 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", "-c:v",
"h264_nvenc", "h264_nvenc",
+129 -8
View File
@@ -161,6 +161,34 @@ pub struct VideoStreamMeta {
/// `r_frame_rate`, which lies on variable-framerate sources). `None` /// `r_frame_rate`, which lies on variable-framerate sources). `None`
/// when ffprobe couldn't parse either field — caller picks a fallback. /// when ffprobe couldn't parse either field — caller picks a fallback.
pub frame_rate: Option<f32>, 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"`, /// 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 // 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 // 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 // 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("-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) .arg(video_path)
.output() .output()
.await; .await;
@@ -262,15 +291,38 @@ pub async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta {
.and_then(parse_ffprobe_rational) .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!( 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 { VideoStreamMeta {
is_h264, is_h264,
rotation, rotation,
frame_rate, frame_rate,
width,
height,
bit_rate,
} }
} }
@@ -507,12 +559,20 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
let rotation = stream_meta.rotation; let rotation = stream_meta.rotation;
let has_rotation = rotation != 0; 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 // Stream-copy is only safe when the source GOP fits inside a
// single HLS segment. Otherwise ffmpeg has to extend segments // single HLS segment. Otherwise ffmpeg has to extend segments
// past hls_time to land on a keyframe, producing uneven // past hls_time to land on a keyframe, producing uneven
// segments and seeking glitches. // segments and seeking glitches.
const HLS_SEGMENT_SECONDS: f64 = 3.0; 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 { match get_max_gop_seconds(&video_file).await {
Some(g) if g > HLS_SEGMENT_SECONDS => { Some(g) if g > HLS_SEGMENT_SECONDS => {
info!( info!(
@@ -536,7 +596,7 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
false false
}; };
let use_copy = is_h264 && !has_rotation && gop_ok; let use_copy = is_h264 && !has_rotation && size_ok && gop_ok;
if has_rotation { if has_rotation {
info!( info!(
@@ -550,6 +610,12 @@ impl Handler<GeneratePlaylistMessage> for PlaylistGenerator {
} else if use_copy { } else if use_copy {
info!("Video {} is already h264, using stream copy", video_file); info!("Video {} is already h264, using stream copy", video_file);
span.add_event("Using stream copy (h264 detected)", vec![]); 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 { } else if is_h264 {
info!( info!(
"Video {} is h264 but needs transcoding for GOP alignment", "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("-rc").arg("vbr");
cmd.arg("-cq").arg("23"); cmd.arg("-cq").arg("23");
cmd.arg("-pix_fmt").arg("yuv420p"); 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 { } else {
cmd.arg("-c:v").arg("h264"); cmd.arg("-c:v").arg("h264");
cmd.arg("-crf").arg("21"); cmd.arg("-crf").arg("21");
cmd.arg("-preset").arg("veryfast"); 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"); cmd.arg("-c:a").arg("aac");
// Force an IDR frame every hls_time seconds so each HLS // Force an IDR frame every hls_time seconds so each HLS
// segment starts on a keyframe — accurate seeking without // segment starts on a keyframe — accurate seeking without
// players having to decode from a prior segment. // players having to decode from a prior segment.
cmd.arg("-force_key_frames").arg("expr:gte(t,n_forced*3)"); 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 // -f hls is required because the playlist is written to a .tmp
@@ -867,7 +948,47 @@ impl Handler<GeneratePreviewClipMessage> for PreviewClipGenerator {
#[cfg(test)] #[cfg(test)]
mod tests { 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] #[test]
fn parses_common_rational_framerates() { fn parses_common_rational_framerates() {
+1 -32
View File
@@ -1,7 +1,7 @@
use futures::TryFutureExt; use futures::TryFutureExt;
use log::{debug, error, info, warn}; use log::{debug, error, info, warn};
use std::io::Result; use std::io::Result;
use std::process::{Output, Stdio}; use std::process::Stdio;
use std::sync::OnceLock; use std::sync::OnceLock;
use std::time::Instant; use std::time::Instant;
use tokio::process::Command; use tokio::process::Command;
@@ -47,37 +47,6 @@ pub enum GifType {
} }
impl Ffmpeg { 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> { async fn get_video_duration(&self, input_file: &str) -> Result<u32> {
Command::new("ffprobe") Command::new("ffprobe")
.args(["-i", input_file]) .args(["-i", input_file])