fix(player): strip the burn-in the server puts back into its own transcode URL
The negotiation asks for no subtitle stream (DR-176), but when PlaybackInfo answers with a TranscodingUrl we played that URL verbatim — and the server built it from its own subtitle verdict. Jellyfin's StreamInfo.ToUrl appends SubtitleStreamIndex and SubtitleMethod whenever it picked a track, so the burn-in we had just declined came straight back through the URL, turning a remux into a full frame-by-frame re-encode. Live TV never declined it at all: open_live_stream sent no index, so the server applied the channel's default track, and broadcast subtitles are DVB bitmaps that NormalizeSubtitleEmbed converts to burn-in on sight. without_server_chosen_subtitle() drops SubtitleStreamIndex, SubtitleMethod, SubtitleCodec and alwaysBurnInSubtitleWhenTranscoding from any URL the server built — matched case-insensitively, as Jellyfin binds query keys — and re-appends the -1 sentinel, because an absent index is not "none", it is "you choose". Applied at both adoption points, plus the sentinel in the live-stream negotiation body and its fallback URL.
This commit is contained in:
@@ -122,6 +122,55 @@ pub fn playback_subtitle_stream_index() -> i32 {
|
|||||||
NO_SUBTITLE_STREAM
|
NO_SUBTITLE_STREAM
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Query keys through which a stream URL can carry a subtitle decision.
|
||||||
|
///
|
||||||
|
/// Jellyfin binds query keys case-insensitively, so the match has to be too —
|
||||||
|
/// the server itself mixes casing (`SubtitleStreamIndex` but
|
||||||
|
/// `alwaysBurnInSubtitleWhenTranscoding`).
|
||||||
|
const SUBTITLE_QUERY_KEYS: &[&str] = &[
|
||||||
|
"subtitlestreamindex",
|
||||||
|
"subtitlemethod",
|
||||||
|
"subtitlecodec",
|
||||||
|
"alwaysburninsubtitlewhentranscoding",
|
||||||
|
];
|
||||||
|
|
||||||
|
/// Rewrite a stream URL so it asks for no subtitle, whoever built it.
|
||||||
|
///
|
||||||
|
/// [`playback_subtitle_stream_index`] only governs the URLs *this app* builds.
|
||||||
|
/// When `PlaybackInfo` answers with a `TranscodingUrl`, the URL was built by the
|
||||||
|
/// server from its own subtitle verdict, and we play it verbatim — so a server
|
||||||
|
/// that picked a track anyway (a live channel opened without an index, a source
|
||||||
|
/// whose default is image-based) hands us `SubtitleMethod=Encode`, and the
|
||||||
|
/// burn-in the negotiation just declined comes back through the URL. Burn-in is
|
||||||
|
/// a *video* cost: it rules out remuxing and forces a full re-encode.
|
||||||
|
///
|
||||||
|
/// Stripping the keys is not enough on its own — an absent index is not "none",
|
||||||
|
/// it is "you choose" — so the sentinel is always appended.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
pub fn without_server_chosen_subtitle(url: &str) -> String {
|
||||||
|
let (path, query) = match url.split_once('?') {
|
||||||
|
Some((path, query)) => (path, query),
|
||||||
|
None => (url, ""),
|
||||||
|
};
|
||||||
|
|
||||||
|
let mut kept: Vec<&str> = query
|
||||||
|
.split('&')
|
||||||
|
.filter(|param| !param.is_empty())
|
||||||
|
.filter(|param| {
|
||||||
|
let key = param.split_once('=').map_or(*param, |(key, _)| key);
|
||||||
|
!SUBTITLE_QUERY_KEYS
|
||||||
|
.iter()
|
||||||
|
.any(|subtitle_key| key.eq_ignore_ascii_case(subtitle_key))
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
let sentinel = format!("SubtitleStreamIndex={}", NO_SUBTITLE_STREAM);
|
||||||
|
kept.push(&sentinel);
|
||||||
|
|
||||||
|
format!("{}?{}", path, kept.join("&"))
|
||||||
|
}
|
||||||
|
|
||||||
/// Whether a subtitle in this format can reach the app as a sidecar it draws
|
/// Whether a subtitle in this format can reach the app as a sidecar it draws
|
||||||
/// itself — the same verdict as [`subtitle_forces_burn_in`], from the reader's
|
/// itself — the same verdict as [`subtitle_forces_burn_in`], from the reader's
|
||||||
/// side, and the one a subtitle picker needs.
|
/// side, and the one a subtitle picker needs.
|
||||||
@@ -249,6 +298,81 @@ mod tests {
|
|||||||
assert_eq!(playback_subtitle_stream_index(), -1);
|
assert_eq!(playback_subtitle_stream_index(), -1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A transcode URL the *server* built carries the server's own subtitle
|
||||||
|
/// verdict. Adopting it verbatim re-introduces the burn-in
|
||||||
|
/// [`playback_subtitle_stream_index`] exists to prevent — the negotiation
|
||||||
|
/// asks for no subtitle, and then we play a URL that asks for one anyway.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
#[test]
|
||||||
|
fn a_server_built_transcode_url_has_its_burn_in_stripped() {
|
||||||
|
// Shape taken from Jellyfin's `StreamInfo.ToUrl`: it appends
|
||||||
|
// `SubtitleStreamIndex` and `SubtitleMethod` whenever it picked a track.
|
||||||
|
let served = "/videos/abc/master.m3u8?DeviceId=jt&MediaSourceId=src1\
|
||||||
|
&VideoCodec=h264&SubtitleMethod=Encode&SubtitleStreamIndex=2\
|
||||||
|
&PlaySessionId=xyz";
|
||||||
|
|
||||||
|
let url = without_server_chosen_subtitle(served);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
url.contains("SubtitleStreamIndex=-1"),
|
||||||
|
"the adopted URL must ask for no subtitle: {url}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!url.contains("SubtitleStreamIndex=2"),
|
||||||
|
"the server's chosen track must not survive: {url}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!url.contains("SubtitleMethod"),
|
||||||
|
"burn-in must not be requested: {url}"
|
||||||
|
);
|
||||||
|
// Everything else identifies the job and must survive untouched.
|
||||||
|
for kept in [
|
||||||
|
"DeviceId=jt",
|
||||||
|
"MediaSourceId=src1",
|
||||||
|
"VideoCodec=h264",
|
||||||
|
"PlaySessionId=xyz",
|
||||||
|
] {
|
||||||
|
assert!(url.contains(kept), "{kept} must survive: {url}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The server may also be told to burn in unconditionally
|
||||||
|
/// (`alwaysBurnInSubtitleWhenTranscoding`), which is appended to the URL
|
||||||
|
/// rather than expressed as a method — and its keys are not PascalCase.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
#[test]
|
||||||
|
fn an_unconditional_burn_in_flag_is_stripped_whatever_its_casing() {
|
||||||
|
let url = without_server_chosen_subtitle(
|
||||||
|
"/videos/abc/master.m3u8?api_key=k&alwaysBurnInSubtitleWhenTranscoding=true\
|
||||||
|
&subtitlestreamindex=3&SubtitleCodec=ass",
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(!url.to_lowercase().contains("alwaysburnin"), "{url}");
|
||||||
|
assert!(!url.to_lowercase().contains("subtitlecodec"), "{url}");
|
||||||
|
assert!(!url.contains("subtitlestreamindex=3"), "{url}");
|
||||||
|
assert!(url.contains("SubtitleStreamIndex=-1"), "{url}");
|
||||||
|
assert!(url.contains("api_key=k"), "{url}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A URL the server built without any subtitle in it still has to *say* so:
|
||||||
|
/// omitting the index is what makes the server apply the source's default.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
#[test]
|
||||||
|
fn a_url_with_no_subtitle_params_is_still_made_to_ask_for_none() {
|
||||||
|
let url = without_server_chosen_subtitle("/videos/abc/master.m3u8?api_key=k");
|
||||||
|
assert_eq!(
|
||||||
|
url,
|
||||||
|
"/videos/abc/master.m3u8?api_key=k&SubtitleStreamIndex=-1"
|
||||||
|
);
|
||||||
|
|
||||||
|
// A bare URL is rare but must not come out malformed.
|
||||||
|
let bare = without_server_chosen_subtitle("/videos/abc/master.m3u8");
|
||||||
|
assert_eq!(bare, "/videos/abc/master.m3u8?SubtitleStreamIndex=-1");
|
||||||
|
}
|
||||||
|
|
||||||
/// TRACES: UR-020 | DR-176 | UT-168
|
/// TRACES: UR-020 | DR-176 | UT-168
|
||||||
#[test]
|
#[test]
|
||||||
fn text_subtitles_are_advertised_as_external_sidecars() {
|
fn text_subtitles_are_advertised_as_external_sidecars() {
|
||||||
|
|||||||
@@ -1741,7 +1741,16 @@ impl MediaRepository for OnlineRepository {
|
|||||||
if let Some(previous) = adopt_video_play_session(response.play_session_id.clone()) {
|
if let Some(previous) = adopt_video_play_session(response.play_session_id.clone()) {
|
||||||
self.stop_transcode(&previous).await;
|
self.stop_transcode(&previous).await;
|
||||||
}
|
}
|
||||||
format!("{}{}", self.server_url, transcoding_url)
|
// The server built this URL from its *own* subtitle verdict, so it can
|
||||||
|
// hand back the burn-in the request above just declined. Strip it: the
|
||||||
|
// negotiated answer only holds for the stream we actually open.
|
||||||
|
//
|
||||||
|
// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
format!(
|
||||||
|
"{}{}",
|
||||||
|
self.server_url,
|
||||||
|
super::device_profile::without_server_chosen_subtitle(transcoding_url)
|
||||||
|
)
|
||||||
} else if audio_forces_transcode {
|
} else if audio_forces_transcode {
|
||||||
warn!(
|
warn!(
|
||||||
"[PlaybackInfo] Server offered direct play for audio the webview cannot decode ({:?}) — forcing an HLS transcode",
|
"[PlaybackInfo] Server offered direct play for audio the webview cannot decode ({:?}) — forcing an HLS transcode",
|
||||||
@@ -1844,6 +1853,13 @@ impl MediaRepository for OnlineRepository {
|
|||||||
auto_open_live_stream: bool,
|
auto_open_live_stream: bool,
|
||||||
is_playback: bool,
|
is_playback: bool,
|
||||||
max_streaming_bitrate: u64,
|
max_streaming_bitrate: u64,
|
||||||
|
/// "No subtitle", for the same reason as everywhere else: omitting it
|
||||||
|
/// lets the server apply the channel's default track, and broadcast
|
||||||
|
/// subtitles are DVB bitmaps — deliverable only by burning them in,
|
||||||
|
/// which forces a full re-encode of a stream that is already tight.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-020, UR-004 | DR-176 | UT-168
|
||||||
|
subtitle_stream_index: i32,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Deserialize)]
|
#[derive(Debug, Deserialize)]
|
||||||
@@ -1871,6 +1887,7 @@ impl MediaRepository for OnlineRepository {
|
|||||||
// too — a channel opened at the source bitrate would walk straight
|
// too — a channel opened at the source bitrate would walk straight
|
||||||
// past a limit set for the connection. TRACES: UR-074 | DR-162
|
// past a limit set for the connection. TRACES: UR-074 | DR-162
|
||||||
max_streaming_bitrate: streaming_quality().max_bitrate().unwrap_or(20_000_000),
|
max_streaming_bitrate: streaming_quality().max_bitrate().unwrap_or(20_000_000),
|
||||||
|
subtitle_stream_index: super::device_profile::playback_subtitle_stream_index(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let response: OpenLiveStreamResponse = self.post_json_response(&endpoint, &request).await?;
|
let response: OpenLiveStreamResponse = self.post_json_response(&endpoint, &request).await?;
|
||||||
@@ -1886,14 +1903,21 @@ impl MediaRepository for OnlineRepository {
|
|||||||
// The transcoding URL is server-relative; make it absolute. If the server
|
// The transcoding URL is server-relative; make it absolute. If the server
|
||||||
// did not provide one (rare for live), fall back to the HLS master endpoint.
|
// did not provide one (rare for live), fall back to the HLS master endpoint.
|
||||||
let stream_url = match source.transcoding_url {
|
let stream_url = match source.transcoding_url {
|
||||||
Some(url) => format!("{}{}", self.server_url, url),
|
// As in `get_playback_info`: the server chose the subtitle in this
|
||||||
|
// URL, so decline it here too. TRACES: UR-020 | DR-176 | UT-168
|
||||||
|
Some(url) => format!(
|
||||||
|
"{}{}",
|
||||||
|
self.server_url,
|
||||||
|
super::device_profile::without_server_chosen_subtitle(&url)
|
||||||
|
),
|
||||||
None => format!(
|
None => format!(
|
||||||
"{}/Videos/{}/master.m3u8?api_key={}&MediaSourceId={}&LiveStreamId={}&VideoCodec=h264&AudioCodec=aac&TranscodingProtocol=hls&TranscodingContainer=ts",
|
"{}/Videos/{}/master.m3u8?api_key={}&MediaSourceId={}&LiveStreamId={}&VideoCodec=h264&AudioCodec=aac&TranscodingProtocol=hls&TranscodingContainer=ts&SubtitleStreamIndex={}",
|
||||||
self.server_url,
|
self.server_url,
|
||||||
item_id,
|
item_id,
|
||||||
self.access_token,
|
self.access_token,
|
||||||
source.id,
|
source.id,
|
||||||
source.live_stream_id.clone().unwrap_or_default(),
|
source.live_stream_id.clone().unwrap_or_default(),
|
||||||
|
super::device_profile::playback_subtitle_stream_index(),
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user