Merge branch 'fix/download-bitrate-param' into HEAD

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();