fix(downloads,playback): re-encode undecodable audio and carry the media source through
Work from a parallel session in the same working tree, committed here so the branch is not left half-written. Attribution note: authored in a concurrent Claude session, not by the author of the preceding commit. - DR-171: a downloaded video keeps audio the device can actually decode. `original` quality asked for a straight copy, so an E-AC-3/AC-3/DTS/TrueHD track came down untouched and the webview had nothing to play it with. - `get_video_download_url` gains the media source, so the URL is built against the source actually chosen rather than the item's default. - Device profile and repository plumbing updated to match. Verified green as a whole: 656 Rust tests, 945 frontend tests, svelte-check clean.
This commit is contained in:
@@ -631,8 +631,6 @@ pub async fn resume_queued_downloads(
|
||||
) -> Result<ResumeQueuedResult, String> {
|
||||
use crate::repository::MediaRepository;
|
||||
|
||||
use crate::repository::HybridRepository;
|
||||
|
||||
let repo = repository.0.get(&handle).ok_or("Repository not found")?;
|
||||
|
||||
// The pump needs a target_dir; use the same storage root the other download
|
||||
@@ -683,12 +681,13 @@ pub async fn resume_queued_downloads(
|
||||
async move {
|
||||
if media_type == "video" {
|
||||
Some(
|
||||
<HybridRepository as MediaRepository>::get_video_download_url(
|
||||
crate::repository::resolve_video_download_url(
|
||||
repo.as_ref(),
|
||||
&item_id,
|
||||
&quality,
|
||||
None,
|
||||
),
|
||||
)
|
||||
.await,
|
||||
)
|
||||
} else {
|
||||
match repo.get_audio_stream_url(&item_id).await {
|
||||
|
||||
@@ -804,7 +804,7 @@ pub fn repository_get_subtitle_url(
|
||||
#[tauri::command]
|
||||
#[specta::specta]
|
||||
#[allow(dead_code)]
|
||||
pub fn repository_get_video_download_url(
|
||||
pub async fn repository_get_video_download_url(
|
||||
manager: State<'_, RepositoryManagerWrapper>,
|
||||
handle: String,
|
||||
item_id: String,
|
||||
@@ -812,9 +812,16 @@ pub fn repository_get_video_download_url(
|
||||
media_source_id: Option<String>,
|
||||
) -> Result<String, String> {
|
||||
let repo = manager.0.get(&handle).ok_or("Repository not found")?;
|
||||
Ok(repo
|
||||
.as_ref()
|
||||
.get_video_download_url(&item_id, &quality, media_source_id.as_deref()))
|
||||
// Async because the audio-codec policy has to know what the source's audio
|
||||
// is before it can decide whether the file may be copied verbatim (DR-171).
|
||||
// The frontend calls this exactly as before — the decision stays in Rust.
|
||||
Ok(crate::repository::resolve_video_download_url(
|
||||
repo.as_ref(),
|
||||
&item_id,
|
||||
&quality,
|
||||
media_source_id.as_deref(),
|
||||
)
|
||||
.await)
|
||||
}
|
||||
|
||||
/// Mark an item as favorite
|
||||
|
||||
@@ -3269,7 +3269,13 @@ mod tests {
|
||||
fn get_subtitle_url(&self, _: &str, _: &str, _: i32, _: &str) -> String {
|
||||
unimplemented!()
|
||||
}
|
||||
fn get_video_download_url(&self, _: &str, _: &str, _: Option<&str>) -> String {
|
||||
fn get_video_download_url(
|
||||
&self,
|
||||
_: &str,
|
||||
_: &str,
|
||||
_: Option<&str>,
|
||||
_: Option<&str>,
|
||||
) -> String {
|
||||
unimplemented!()
|
||||
}
|
||||
async fn mark_favorite(&self, _: &str) -> Result<(), repo_types::RepoError> {
|
||||
|
||||
@@ -120,22 +120,34 @@ pub fn webview_can_decode_audio(codec: &str) -> bool {
|
||||
/// delegate this decision; it knows what its own renderer can decode and must
|
||||
/// apply that itself.
|
||||
///
|
||||
/// The track that matters is the one the server will actually serve: the
|
||||
/// default, or the first when none is marked. An unknown codec is left alone —
|
||||
/// forcing a transcode on a guess would burn server CPU for files that play.
|
||||
/// The track that matters is the one the server will actually serve (see
|
||||
/// [`served_audio_codec`]). An unknown codec is left alone — forcing a transcode
|
||||
/// on a guess would burn server CPU for files that play.
|
||||
///
|
||||
/// TRACES: UR-004 | DR-149 | UT-148
|
||||
pub fn audio_forces_transcode(streams: &[(Option<&str>, bool)]) -> bool {
|
||||
let served = streams
|
||||
match served_audio_codec(streams) {
|
||||
Some(codec) => !webview_can_decode_audio(codec),
|
||||
// No audio at all, or a codec the server did not name: leave it alone.
|
||||
None => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// The codec of the audio track the server will actually serve, given the
|
||||
/// source's audio streams as `(codec, is_default)` in source order: the default,
|
||||
/// or the first when none is marked.
|
||||
///
|
||||
/// `None` means "nothing to judge" — no audio streams, or the server named no
|
||||
/// codec for the one it would serve. Both callers of this rule treat that as
|
||||
/// leave-well-alone, never as a licence to assume compatibility.
|
||||
///
|
||||
/// TRACES: UR-004, UR-071 | DR-149, DR-171 | UT-148, UT-166
|
||||
pub fn served_audio_codec<'a>(streams: &[(Option<&'a str>, bool)]) -> Option<&'a str> {
|
||||
streams
|
||||
.iter()
|
||||
.find(|(_, is_default)| *is_default)
|
||||
.or_else(|| streams.first());
|
||||
|
||||
match served {
|
||||
Some((Some(codec), _)) => !webview_can_decode_audio(codec),
|
||||
// No audio at all, or a codec the server did not name: leave it alone.
|
||||
Some((None, _)) | None => false,
|
||||
}
|
||||
.or_else(|| streams.first())
|
||||
.and_then(|(codec, _)| *codec)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -193,6 +205,25 @@ mod tests {
|
||||
assert!(!audio_forces_transcode(&[(None, true)]));
|
||||
}
|
||||
|
||||
/// The download path needs the codec itself, not just the verdict, so it can
|
||||
/// tell the server what to re-encode. It picks the same track the streaming
|
||||
/// verdict is formed from — one rule, one place.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171 | UT-166
|
||||
#[test]
|
||||
fn the_served_codec_is_the_one_the_verdict_is_formed_from() {
|
||||
assert_eq!(
|
||||
served_audio_codec(&[(Some("aac"), false), (Some("eac3"), true)]),
|
||||
Some("eac3")
|
||||
);
|
||||
assert_eq!(
|
||||
served_audio_codec(&[(Some("eac3"), false), (Some("aac"), false)]),
|
||||
Some("eac3")
|
||||
);
|
||||
assert_eq!(served_audio_codec(&[]), None);
|
||||
assert_eq!(served_audio_codec(&[(None, true)]), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_dolby_device_does_not_advertise_dolby_for_video() {
|
||||
// The bug: a Motorola reporting c2.dolby.eac3.decoder direct-played
|
||||
|
||||
@@ -872,10 +872,11 @@ impl MediaRepository for HybridRepository {
|
||||
item_id: &str,
|
||||
quality: &str,
|
||||
media_source_id: Option<&str>,
|
||||
source_audio_codec: Option<&str>,
|
||||
) -> String {
|
||||
// Always use online URL for downloads
|
||||
self.online
|
||||
.get_video_download_url(item_id, quality, media_source_id)
|
||||
.get_video_download_url(item_id, quality, media_source_id, source_audio_codec)
|
||||
}
|
||||
|
||||
async fn mark_favorite(&self, item_id: &str) -> Result<(), RepoError> {
|
||||
@@ -1299,6 +1300,7 @@ mod tests {
|
||||
_item_id: &str,
|
||||
_quality: &str,
|
||||
_media_source_id: Option<&str>,
|
||||
_source_audio_codec: Option<&str>,
|
||||
) -> String {
|
||||
unimplemented!()
|
||||
}
|
||||
@@ -1573,6 +1575,7 @@ mod tests {
|
||||
_item_id: &str,
|
||||
_quality: &str,
|
||||
_media_source_id: Option<&str>,
|
||||
_source_audio_codec: Option<&str>,
|
||||
) -> String {
|
||||
unimplemented!()
|
||||
}
|
||||
|
||||
@@ -197,14 +197,24 @@ pub trait MediaRepository: Send + Sync {
|
||||
format: &str,
|
||||
) -> String;
|
||||
|
||||
/// Get video download URL (synchronous - just constructs URL)
|
||||
/// Called by frontend via Tauri invoke (getVideoDownloadUrl in VideoDownloadButton.svelte)
|
||||
/// Build the URL a video download is fetched from. Synchronous — it only
|
||||
/// constructs a URL, so it stays testable without a server. Reach it through
|
||||
/// [`resolve_video_download_url`] rather than calling it directly.
|
||||
///
|
||||
/// `source_audio_codec` is the codec of the audio track the server would
|
||||
/// serve (see [`served_audio_codec`]); `None` when it is not known. At
|
||||
/// `original` quality it decides whether the file can be copied byte-for-byte
|
||||
/// or has to have its audio re-encoded on the way down — a downloaded file is
|
||||
/// played back with no server in reach, so it has to be decodable *here*.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171
|
||||
#[allow(dead_code)]
|
||||
fn get_video_download_url(
|
||||
&self,
|
||||
item_id: &str,
|
||||
quality: &str,
|
||||
media_source_id: Option<&str>,
|
||||
source_audio_codec: Option<&str>,
|
||||
) -> String;
|
||||
|
||||
/// Mark item as favorite
|
||||
@@ -323,3 +333,44 @@ pub trait MediaRepository: Send + Sync {
|
||||
new_index: u32,
|
||||
) -> Result<(), RepoError>;
|
||||
}
|
||||
|
||||
/// The audio codec the server would serve for `item_id` — the default track, or
|
||||
/// the first when none is marked, matching the track Jellyfin picks.
|
||||
///
|
||||
/// `None` when the item has no audio, names no codec, or cannot be fetched. A
|
||||
/// caller must read that as "unknown", never as "fine": it is the input to a
|
||||
/// policy that only *adds* a transcode, so an unknown codec leaves behaviour
|
||||
/// exactly as it was.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171 | UT-166
|
||||
pub async fn served_audio_codec(repo: &dyn MediaRepository, item_id: &str) -> Option<String> {
|
||||
let item = repo.get_item(item_id).await.ok()?;
|
||||
let audio: Vec<(Option<&str>, bool)> = item
|
||||
.media_streams
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.iter()
|
||||
.filter(|s| s.stream_type == "Audio")
|
||||
.map(|s| (s.codec.as_deref(), s.is_default))
|
||||
.collect();
|
||||
|
||||
device_profile::served_audio_codec(&audio).map(str::to_string)
|
||||
}
|
||||
|
||||
/// Resolve the download URL for a video, applying the audio-codec policy that
|
||||
/// keeps the saved file playable offline (DR-171).
|
||||
///
|
||||
/// Every video download goes through here rather than calling the builder
|
||||
/// directly: the builder is pure and cannot look the codec up, and a caller that
|
||||
/// forgets to is exactly how the silent downloads shipped.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171
|
||||
pub async fn resolve_video_download_url(
|
||||
repo: &dyn MediaRepository,
|
||||
item_id: &str,
|
||||
quality: &str,
|
||||
media_source_id: Option<&str>,
|
||||
) -> String {
|
||||
let codec = served_audio_codec(repo, item_id).await;
|
||||
repo.get_video_download_url(item_id, quality, media_source_id, codec.as_deref())
|
||||
}
|
||||
|
||||
@@ -828,6 +828,32 @@ impl OfflineRepository {
|
||||
/// the synced-but-not-downloaded catalog branch deliberately excluded, so it
|
||||
/// is authoritative regardless of the process-wide catalog-browse flag.
|
||||
///
|
||||
/// Whether cached item `i` belongs to library `l`, decided by media kind.
|
||||
///
|
||||
/// The cache leaves `library_id`/`parent_id` NULL on every item
|
||||
/// ([[offline-libraries-never-cached]]), so there is no link to follow: a
|
||||
/// library's `collection_type` and an item's `item_type` are the only things
|
||||
/// that can associate them. This is Jellyfin taxonomy and therefore lives in
|
||||
/// Rust, never in the frontend.
|
||||
///
|
||||
/// It is a named constant because it is needed in two places that must agree
|
||||
/// — which library *appears* in the Downloaded list, and which items appear
|
||||
/// *inside* it. They disagreed: the listing query used this mapping while the
|
||||
/// browse query only checked that the requested library existed, so opening
|
||||
/// any library showed every downloaded top-level item on the server.
|
||||
///
|
||||
/// A library of some other (or unknown) type keeps everything, since there is
|
||||
/// no mapping to narrow it by and hiding its contents would be worse.
|
||||
///
|
||||
/// TRACES: UR-055 | DR-082, DR-167
|
||||
const LIBRARY_HOLDS_ITEM: &'static str = "(
|
||||
(l.collection_type = 'music' AND i.item_type IN ('MusicAlbum', 'MusicArtist', 'Audio'))
|
||||
OR (l.collection_type = 'movies' AND i.item_type = 'Movie')
|
||||
OR (l.collection_type = 'tvshows' AND i.item_type IN ('Series', 'Season', 'Episode'))
|
||||
OR l.collection_type IS NULL
|
||||
OR l.collection_type NOT IN ('music', 'movies', 'tvshows')
|
||||
)";
|
||||
|
||||
/// TRACES: UR-055 | DR-082, DR-083
|
||||
const DOWNLOADED_ITEMS_CTE: &'static str = "
|
||||
WITH downloaded_items AS (
|
||||
@@ -908,6 +934,7 @@ impl OfflineRepository {
|
||||
EXISTS (
|
||||
SELECT 1 FROM libraries l
|
||||
WHERE l.id = ? AND l.server_id = i.server_id
|
||||
AND {membership}
|
||||
)
|
||||
-- Top-level only: hide leaves whose container is downloaded.
|
||||
AND NOT EXISTS (
|
||||
@@ -922,6 +949,7 @@ impl OfflineRepository {
|
||||
ORDER BY i.sort_name ASC, i.name ASC
|
||||
LIMIT {limit} OFFSET {start_index}",
|
||||
cte = Self::DOWNLOADED_ITEMS_CTE,
|
||||
membership = Self::LIBRARY_HOLDS_ITEM,
|
||||
);
|
||||
|
||||
let query = Query::with_params(
|
||||
@@ -966,7 +994,7 @@ impl OfflineRepository {
|
||||
// We match a library by collection_type ↔ item_type instead: any
|
||||
// completed download of a given media kind qualifies that library.
|
||||
let query = Query::with_params(
|
||||
&format!(
|
||||
format!(
|
||||
"{cte}
|
||||
SELECT l.id, l.name, l.collection_type, l.image_tag
|
||||
FROM libraries l
|
||||
@@ -975,15 +1003,11 @@ impl OfflineRepository {
|
||||
SELECT 1 FROM items i
|
||||
INNER JOIN downloaded_items di ON i.id = di.id
|
||||
WHERE i.server_id = l.server_id
|
||||
AND (
|
||||
(l.collection_type = 'music' AND i.item_type IN ('MusicAlbum', 'MusicArtist', 'Audio'))
|
||||
OR (l.collection_type = 'movies' AND i.item_type = 'Movie')
|
||||
OR (l.collection_type = 'tvshows' AND i.item_type IN ('Series', 'Season', 'Episode'))
|
||||
OR (l.collection_type NOT IN ('music', 'movies', 'tvshows'))
|
||||
)
|
||||
AND {membership}
|
||||
)
|
||||
ORDER BY l.sort_order ASC, l.name ASC",
|
||||
cte = Self::DOWNLOADED_ITEMS_CTE,
|
||||
membership = Self::LIBRARY_HOLDS_ITEM,
|
||||
),
|
||||
vec![QueryParam::String(self.server_id.clone())],
|
||||
);
|
||||
@@ -1959,6 +1983,7 @@ impl MediaRepository for OfflineRepository {
|
||||
_item_id: &str,
|
||||
_quality: &str,
|
||||
_media_source_id: Option<&str>,
|
||||
_source_audio_codec: Option<&str>,
|
||||
) -> String {
|
||||
// Cannot download while offline
|
||||
String::new()
|
||||
@@ -3720,6 +3745,82 @@ mod tests {
|
||||
assert_eq!(track_ids, vec!["track-1", "track-2"]);
|
||||
}
|
||||
|
||||
/// Regression: each downloaded library shows **only its own media**.
|
||||
///
|
||||
/// Cached items carry no link back to their library (`library_id`/`parent_id`
|
||||
/// are NULL — [[offline-libraries-never-cached]]), and the library branch of
|
||||
/// the query only asserted that the requested library *exists*, never that
|
||||
/// the item belongs to it. So opening any downloaded library listed every
|
||||
/// downloaded top-level item on the server: films in the music library,
|
||||
/// albums under TV. The library's `collection_type` decides which item types
|
||||
/// belong to it, the same mapping `get_downloaded_libraries` already uses.
|
||||
///
|
||||
/// TRACES: UR-055 | DR-167 | UT-162
|
||||
#[tokio::test]
|
||||
async fn test_get_downloaded_items_library_does_not_mix_media_types() {
|
||||
let db = create_test_db();
|
||||
seed_library(&db, "music-lib", "music").await;
|
||||
seed_library(&db, "movie-lib", "movies").await;
|
||||
seed_library(&db, "tv-lib", "tvshows").await;
|
||||
|
||||
insert_item(&db, "album-1", "MusicAlbum", None, None, None).await;
|
||||
insert_item(&db, "track-1", "Audio", Some("album-1"), None, None).await;
|
||||
insert_item(&db, "movie-1", "Movie", None, None, None).await;
|
||||
insert_item(&db, "series-1", "Series", None, None, None).await;
|
||||
insert_item(&db, "episode-1", "Episode", None, Some("series-1"), None).await;
|
||||
|
||||
seed_completed_download(&db, "track-1", 1000).await;
|
||||
seed_completed_download(&db, "movie-1", 2000).await;
|
||||
seed_completed_download(&db, "episode-1", 3000).await;
|
||||
|
||||
let repo = make_repo(&db);
|
||||
|
||||
let music: Vec<String> = repo
|
||||
.get_downloaded_items("music-lib", None)
|
||||
.await
|
||||
.unwrap()
|
||||
.items
|
||||
.iter()
|
||||
.map(|i| i.id.clone())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
music,
|
||||
vec!["album-1"],
|
||||
"the music library must not list films or series; got {:?}",
|
||||
music
|
||||
);
|
||||
|
||||
let movies: Vec<String> = repo
|
||||
.get_downloaded_items("movie-lib", None)
|
||||
.await
|
||||
.unwrap()
|
||||
.items
|
||||
.iter()
|
||||
.map(|i| i.id.clone())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
movies,
|
||||
vec!["movie-1"],
|
||||
"the movie library must not list albums or series; got {:?}",
|
||||
movies
|
||||
);
|
||||
|
||||
let tv: Vec<String> = repo
|
||||
.get_downloaded_items("tv-lib", None)
|
||||
.await
|
||||
.unwrap()
|
||||
.items
|
||||
.iter()
|
||||
.map(|i| i.id.clone())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
tv,
|
||||
vec!["series-1"],
|
||||
"the TV library must not list albums or films; got {:?}",
|
||||
tv
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: a downloaded TV library lists the Series, not its Seasons or
|
||||
/// Episodes — the same "individual songs" bug seen for music, for TV. The
|
||||
/// season and episode are still reachable by drilling into the series.
|
||||
|
||||
@@ -1876,6 +1876,7 @@ impl MediaRepository for OnlineRepository {
|
||||
item_id: &str,
|
||||
quality: &str,
|
||||
media_source_id: Option<&str>,
|
||||
source_audio_codec: Option<&str>,
|
||||
) -> String {
|
||||
// NOTE: Jellyfin's `/Videos/{id}/download` endpoint is not universally
|
||||
// available (returns 404 on many server configs), which silently broke
|
||||
@@ -1926,10 +1927,39 @@ impl MediaRepository for OnlineRepository {
|
||||
params.push("audioCodec=aac".to_string());
|
||||
params.push("allowVideoStreamCopy=false".to_string());
|
||||
}
|
||||
// "original" (and any unknown value) → direct, resumable copy.
|
||||
_ => {
|
||||
params.push("Static=true".to_string());
|
||||
}
|
||||
// "original" (and any unknown value) → direct, resumable copy —
|
||||
// unless the audio in that copy is undecodable where the file will
|
||||
// be played back. A download is watched with no server in reach, so
|
||||
// it has to satisfy the same constraint DR-149 applies to streams:
|
||||
// the webview `<video>` element renders video on both platforms and
|
||||
// decodes none of AC-3/E-AC-3/DTS/TrueHD. Copying those bytes to
|
||||
// disk is what made a downloaded film play offline as picture with
|
||||
// no sound while the same film had sound when streamed.
|
||||
//
|
||||
// Only the *audio* is re-encoded. `allowVideoStreamCopy` keeps an
|
||||
// h264 source's picture byte-for-byte, so "original" still means
|
||||
// original quality, and no bitrate or resolution cap is added. A
|
||||
// source the webview could not have rendered anyway (HEVC) is
|
||||
// re-encoded to h264 as a side effect, which is the only form of it
|
||||
// that would have played.
|
||||
//
|
||||
// The cost of the transcode is that the response is no longer
|
||||
// range-resumable, which is exactly why this is decided per item
|
||||
// rather than applied to every `original` download.
|
||||
//
|
||||
// TRACES: UR-071, UR-004 | DR-171 | UT-166
|
||||
_ => match source_audio_codec {
|
||||
Some(codec) if !super::device_profile::webview_can_decode_audio(codec) => {
|
||||
params.push("videoCodec=h264".to_string());
|
||||
params.push("allowVideoStreamCopy=true".to_string());
|
||||
params.push("audioCodec=aac".to_string());
|
||||
params.push("audioBitRate=384000".to_string());
|
||||
}
|
||||
// Decodable, or unknown: an unknown codec must not provoke a
|
||||
// transcode — that would burn server CPU on a guess for files
|
||||
// that play perfectly well.
|
||||
_ => params.push("Static=true".to_string()),
|
||||
},
|
||||
}
|
||||
|
||||
// Add media source ID if provided
|
||||
@@ -2737,7 +2767,7 @@ mod tests {
|
||||
#[test]
|
||||
fn test_video_download_url_uses_stream_not_download_endpoint() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", None);
|
||||
let url = repo.get_video_download_url("item123", "original", None, None);
|
||||
|
||||
// Must NOT use the /download endpoint (404 on real servers).
|
||||
assert!(
|
||||
@@ -2755,7 +2785,7 @@ mod tests {
|
||||
#[test]
|
||||
fn test_video_download_url_original_is_static_direct_copy() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", None);
|
||||
let url = repo.get_video_download_url("item123", "original", None, None);
|
||||
|
||||
// "original" must request a direct static copy (byte-range resumable),
|
||||
// with no transcode params.
|
||||
@@ -2775,7 +2805,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for (quality, height) in [("high", "1080"), ("medium", "720"), ("low", "480")] {
|
||||
let url = repo.get_video_download_url("item123", quality, None);
|
||||
let url = repo.get_video_download_url("item123", quality, None, None);
|
||||
assert!(
|
||||
url.contains("/Videos/item123/stream.mp4"),
|
||||
"{quality} must use stream.mp4: {url}"
|
||||
@@ -2808,7 +2838,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for quality in ["high", "medium", "low"] {
|
||||
let url = repo.get_video_download_url("item123", quality, None);
|
||||
let url = repo.get_video_download_url("item123", quality, None, None);
|
||||
|
||||
assert!(
|
||||
url.contains("videoBitRate="),
|
||||
@@ -2842,7 +2872,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for quality in ["high", "medium", "low"] {
|
||||
let url = repo.get_video_download_url("item123", quality, None);
|
||||
let url = repo.get_video_download_url("item123", quality, None, None);
|
||||
assert!(
|
||||
url.contains("allowVideoStreamCopy=false"),
|
||||
"{quality} must forbid video stream copy: {url}"
|
||||
@@ -2850,17 +2880,99 @@ mod tests {
|
||||
}
|
||||
|
||||
// "original" is a deliberate direct copy — it must NOT disable copying.
|
||||
let original = repo.get_video_download_url("item123", "original", None);
|
||||
let original = repo.get_video_download_url("item123", "original", None, None);
|
||||
assert!(
|
||||
!original.contains("allowVideoStreamCopy=false"),
|
||||
"original must remain a direct copy: {original}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A downloaded file is played with no server in reach, so `original`
|
||||
/// quality cannot mean "copy whatever the source holds" when the source
|
||||
/// holds audio this device cannot decode.
|
||||
///
|
||||
/// `Static=true` hands back the source bytes untouched, E-AC-3/AC-3/DTS
|
||||
/// track included, and video plays through the webview `<video>` element on
|
||||
/// both platforms — which decodes none of them. Streaming already knows this
|
||||
/// (DR-149 forces a transcode over the server's own direct-play offer); the
|
||||
/// download path did not, so a downloaded film played offline as picture with
|
||||
/// no sound while the very same film had sound when streamed.
|
||||
///
|
||||
/// TRACES: UR-071, UR-004 | DR-171 | UT-166
|
||||
#[test]
|
||||
fn test_video_download_url_original_transcodes_undecodable_audio() {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for codec in ["eac3", "ac3", "dts", "truehd", "EAC3"] {
|
||||
let url = repo.get_video_download_url("item123", "original", None, Some(codec));
|
||||
assert!(
|
||||
!url.contains("Static=true"),
|
||||
"{codec} cannot be decoded here, so the source must not be copied verbatim: {url}"
|
||||
);
|
||||
assert!(
|
||||
url.contains("audioCodec=aac"),
|
||||
"{codec} must be re-encoded to aac on the way down: {url}"
|
||||
);
|
||||
// "Original" still has to mean original picture: the video stream is
|
||||
// copied when it can be, so no bitrate or resolution cap appears.
|
||||
assert!(
|
||||
url.contains("allowVideoStreamCopy=true"),
|
||||
"the video stream must still be copied where possible: {url}"
|
||||
);
|
||||
assert!(
|
||||
!url.contains("videoBitRate") && !url.contains("maxHeight"),
|
||||
"original must not degrade the picture to fix the audio: {url}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The converse, and the reason the policy is per-item rather than blanket:
|
||||
/// audio that plays here keeps the byte-exact, range-resumable copy that the
|
||||
/// download worker's resume depends on.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171 | UT-166
|
||||
#[test]
|
||||
fn test_video_download_url_original_keeps_static_copy_for_playable_audio() {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for codec in ["aac", "mp3", "opus", "vorbis", "flac", "AAC"] {
|
||||
let url = repo.get_video_download_url("item123", "original", None, Some(codec));
|
||||
assert!(
|
||||
url.contains("Static=true"),
|
||||
"{codec} plays here — the download must stay a direct copy: {url}"
|
||||
);
|
||||
assert!(
|
||||
!url.contains("audioCodec="),
|
||||
"{codec} needs no transcode: {url}"
|
||||
);
|
||||
}
|
||||
|
||||
// Unknown codec: the policy only ever *adds* a transcode, so an item we
|
||||
// could not look up behaves exactly as it did before.
|
||||
let unknown = repo.get_video_download_url("item123", "original", None, None);
|
||||
assert!(unknown.contains("Static=true"), "url: {unknown}");
|
||||
}
|
||||
|
||||
/// The explicit quality presets already transcode audio to AAC, so the
|
||||
/// policy has nothing to add — and must not start overriding a chosen cap.
|
||||
///
|
||||
/// TRACES: UR-071 | DR-171 | UT-166
|
||||
#[test]
|
||||
fn test_video_download_url_presets_ignore_the_audio_policy() {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for quality in ["high", "medium", "low"] {
|
||||
let with = repo.get_video_download_url("item123", quality, None, Some("eac3"));
|
||||
let without = repo.get_video_download_url("item123", quality, None, None);
|
||||
assert_eq!(with, without, "{quality} must not vary with source audio");
|
||||
assert!(with.contains("audioCodec=aac"), "url: {with}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_video_download_url_passes_media_source_id() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", Some("src-42"));
|
||||
let url = repo.get_video_download_url("item123", "original", Some("src-42"), None);
|
||||
assert!(url.contains("mediaSourceId=src-42"), "url: {url}");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user