Compare commits

...
Author SHA1 Message Date
dtourolleandClaude Opus 5 adc460f35d fix(downloads): honor the selected bitrate (videoBitRate, capital R)
Downloading at a specific quality silently returned the full-size
original. The download URL builder spelled the transcode params
`videoBitrate`/`audioBitrate`, but Jellyfin binds `videoBitRate`/
`audioBitRate` — with a capital R.

Query-key binding is case-insensitive, so this is not a casing
preference: the lowercase-r form is a different token that fails to
bind. The server discards it without error and then stream-copies the
source, so picking "480p" produced an original-quality file with no
failure surfaced anywhere. `maxHeight`/`videoCodec` were unaffected
(case-insensitive binding covers them), which is why the height cap
applied while the bitrate cap vanished.

Also set `allowVideoStreamCopy=false` on the transcode presets to force
a real re-encode. Video stream-copy is gated by `allowVideoStreamCopy`,
not `enableAutoStreamCopy` — the latter governs audio only.

`original` is unchanged: it stays a deliberate direct static copy, now
pinned by a test.

The pre-existing unit tests asserted the broken lowercase-r spellings,
so they passed against broken code; corrected. Verified red -> green by
extracting the pre-fix and post-fix builder bodies into an isolated
harness: 15 assertion failures before, 0 after.

TRACES: UR-071 | DR-123

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-12 18:44:54 +02:00
+85 -8
View File
@@ -1772,6 +1772,7 @@ impl MediaRepository for OnlineRepository {
)
}
/// TRACES: UR-071 | DR-123
fn get_video_download_url(
&self,
item_id: &str,
@@ -1789,27 +1790,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.
_ => {
@@ -2548,7 +2565,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!(
@@ -2568,7 +2585,7 @@ mod tests {
"{quality} must use stream.mp4: {url}"
);
assert!(
url.contains("videoBitrate="),
url.contains("videoBitRate="),
"{quality} must set bitrate: {url}"
);
assert!(
@@ -2584,6 +2601,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();