diff --git a/src-tauri/src/repository/online.rs b/src-tauri/src/repository/online.rs index eecdd86f..9c84174a 100644 --- a/src-tauri/src/repository/online.rs +++ b/src-tauri/src/repository/online.rs @@ -1788,6 +1788,7 @@ impl MediaRepository for OnlineRepository { ) } + /// TRACES: UR-071 | DR-123 fn get_video_download_url( &self, item_id: &str, @@ -1805,27 +1806,43 @@ impl MediaRepository for OnlineRepository { // Map the frontend quality preset to concrete transcode params. For // "original" we request a direct static copy (no transcode) which is // byte-range resumable; other presets ask the server to transcode. + // + // 🔴 It is `videoBitRate`/`audioBitRate` — **capital R**. Jellyfin binds + // query keys case-insensitively, so `maxHeight`/`videoCodec` casing is + // free, but `videoBitrate` (lowercase r) is a *different token*: it + // fails to bind, is silently dropped, and the requested cap vanishes + // with no error. That is why every "480p"/"720p" download came back at + // full original quality. See `Jellyfin.Api` BaseEncodingJobOptions. + // + // `allowVideoStreamCopy=false` forces a real re-encode. Without it the + // server may stream-copy the source when it already satisfies the cap — + // fine in itself, but it also means a mis-typed cap degrades silently. + // Note `enableAutoStreamCopy=false` alone does NOT stop a *video* copy; + // video copy is gated by `allowVideoStreamCopy`. match quality { "high" => { - params.push("videoBitrate=8000000".to_string()); + params.push("videoBitRate=8000000".to_string()); params.push("maxHeight=1080".to_string()); - params.push("audioBitrate=384000".to_string()); + params.push("audioBitRate=384000".to_string()); params.push("videoCodec=h264".to_string()); params.push("audioCodec=aac".to_string()); + params.push("allowVideoStreamCopy=false".to_string()); } "medium" => { - params.push("videoBitrate=4000000".to_string()); + params.push("videoBitRate=4000000".to_string()); params.push("maxHeight=720".to_string()); - params.push("audioBitrate=256000".to_string()); + params.push("audioBitRate=256000".to_string()); params.push("videoCodec=h264".to_string()); params.push("audioCodec=aac".to_string()); + params.push("allowVideoStreamCopy=false".to_string()); } "low" => { - params.push("videoBitrate=1500000".to_string()); + params.push("videoBitRate=1500000".to_string()); params.push("maxHeight=480".to_string()); - params.push("audioBitrate=128000".to_string()); + params.push("audioBitRate=128000".to_string()); params.push("videoCodec=h264".to_string()); params.push("audioCodec=aac".to_string()); + params.push("allowVideoStreamCopy=false".to_string()); } // "original" (and any unknown value) → direct, resumable copy. _ => { @@ -2564,7 +2581,7 @@ mod tests { // with no transcode params. assert!(url.contains("Static=true"), "url: {url}"); assert!( - !url.contains("videoBitrate"), + !url.contains("videoBitRate"), "original must not transcode: {url}" ); assert!( @@ -2584,7 +2601,7 @@ mod tests { "{quality} must use stream.mp4: {url}" ); assert!( - url.contains("videoBitrate="), + url.contains("videoBitRate="), "{quality} must set bitrate: {url}" ); assert!( @@ -2600,6 +2617,66 @@ mod tests { } } + /// The bitrate params are spelled `videoBitRate`/`audioBitRate` — **capital + /// R**. Jellyfin binds query keys case-insensitively, so this is not a + /// casing preference: `videoBitrate` is a *different token* that fails to + /// bind and is silently discarded, taking the user's quality cap with it. + /// Nothing errors — the download just returns the full-size original, which + /// is exactly how this bug went unnoticed. + #[test] + fn test_video_download_url_bitrate_params_use_capital_r_spelling() { + let repo = create_test_repository(); + + for quality in ["high", "medium", "low"] { + let url = repo.get_video_download_url("item123", quality, None); + + assert!( + url.contains("videoBitRate="), + "{quality} must spell it videoBitRate (capital R): {url}" + ); + assert!( + url.contains("audioBitRate="), + "{quality} must spell it audioBitRate (capital R): {url}" + ); + + // The lowercase-r spellings never bind — they must not appear at + // all, or the cap is silently dropped by the server. + assert!( + !url.contains("videoBitrate="), + "{quality} emits the unbindable lowercase-r spelling: {url}" + ); + assert!( + !url.contains("audioBitrate="), + "{quality} emits the unbindable lowercase-r spelling: {url}" + ); + } + } + + /// A correctly-spelled cap is still only *conditionally* honored: the server + /// may stream-copy the source when it already satisfies the cap. Video copy + /// is gated by `allowVideoStreamCopy` (NOT `enableAutoStreamCopy`, which + /// only governs audio), so the transcode presets must disable it to + /// guarantee a real re-encode at the requested bitrate. + #[test] + fn test_video_download_url_transcode_presets_forbid_video_stream_copy() { + let repo = create_test_repository(); + + for quality in ["high", "medium", "low"] { + let url = repo.get_video_download_url("item123", quality, None); + assert!( + url.contains("allowVideoStreamCopy=false"), + "{quality} must forbid video stream copy: {url}" + ); + } + + // "original" is a deliberate direct copy — it must NOT disable copying. + let original = repo.get_video_download_url("item123", "original", None); + assert!( + !original.contains("allowVideoStreamCopy=false"), + "original must remain a direct copy: {original}" + ); + } + #[test] fn test_video_download_url_passes_media_source_id() { let repo = create_test_repository();