fix(player): resume a transcoded video by seeking, not by asking for a stream that starts mid-item
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 5m10s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 5m30s
Traceability Validation / Check Requirement Traces (push) Successful in 19s
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 5m10s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 5m30s
Traceability Validation / Check Requirement Traces (push) Successful in 19s
A resumed transcode played nothing at all: every segment came back 400, hls.js exhausted its retries and gave up, while the same episode from the beginning was fine. Jellyfin builds each segment URI by echoing the master playlist's query string into it, and its segment handler opens by rejecting any request carrying StartTimeTicks > 0 (ArgumentException → 400). So one resume position on the playlist is copied onto every hls1/main/N.ts and 400s all of them — the `> 0` being exactly why starting from the beginning survived. HLS does not need the parameter: a playlist spans the whole item and asking for segment N *is* the seek. It is removed from the URL builder entirely rather than conditionalised — the builder cannot know whether its response will be segmented — and the position becomes a seek issued once the player has loaded. The progressive /Audio/universal builder behind the background-audio handoff has no segments and keeps its StartTimeTicks, which is why audio-only handoffs resumed correctly and video ones did not. Completing that across the boundary, since the URL no longer starts where the caller asked: - reloadSource(url, position) now means "reload and resume AT this absolute position": it seeks the element once the source is playable and clears the transcode offset to zero. It previously set the offset to the position and seeked nothing, which was correct only while the URL itself began there — left in place it would have shown 20:00 on the scrubber while the opening titles played, with no seek ever happening. - The transcoded resume path in the player page collapses into the same "seek after load" branch direct streams already used. - VideoPlayer's background-audio return does the same: no base, seek to the absolute position. - The stale test asserting StartTimeTicks is present is rewritten to keep its other half (an HLS master playlist, never a progressive stream.mp4, carrying the chosen source and audio track). TRACES: UR-004, UR-005, UR-019, UR-021, UR-074 | DR-181 | UT-182, UT-183
This commit is contained in:
@@ -1392,7 +1392,6 @@ pub async fn player_seek_video(
|
||||
.get_video_stream_url(
|
||||
&jellyfin_item_id,
|
||||
media_source_id.as_deref(),
|
||||
Some(position),
|
||||
audio_stream_index,
|
||||
)
|
||||
.await
|
||||
@@ -1403,6 +1402,13 @@ pub async fn player_seek_video(
|
||||
position
|
||||
);
|
||||
|
||||
// `seek_offset` carries the position to RESUME AT, not a base to add
|
||||
// to the element's clock. The reloaded stream starts at the item's
|
||||
// zero — a position on an HLS playlist makes the server 400 every
|
||||
// segment behind it (DR-181) — so the adapter reaches the position by
|
||||
// seeking the element and leaves the transcode offset at zero. The
|
||||
// field keeps its name only because renaming it means regenerating
|
||||
// the specta bindings; `reloadSource` documents the contract.
|
||||
Ok(VideoSeekResponse::ReloadStream {
|
||||
new_url,
|
||||
seek_offset: position,
|
||||
@@ -1416,7 +1422,6 @@ pub async fn player_seek_video(
|
||||
.get_video_stream_url(
|
||||
&jellyfin_item_id,
|
||||
media_source_id.as_deref(),
|
||||
Some(position),
|
||||
audio_stream_index,
|
||||
)
|
||||
.await
|
||||
@@ -1454,6 +1459,11 @@ pub async fn player_seek_video(
|
||||
} else {
|
||||
return Err("No current item after URL update".to_string());
|
||||
}
|
||||
|
||||
// The re-opened stream begins at zero — the position cannot ride
|
||||
// along in the URL without 400ing every segment (DR-181) — so the
|
||||
// seek that the reload was asked for happens here.
|
||||
controller.seek(position).map_err(|e| e.to_string())?;
|
||||
}
|
||||
|
||||
info!(
|
||||
@@ -1504,12 +1514,13 @@ pub async fn player_switch_audio_track(
|
||||
.to_string()
|
||||
};
|
||||
|
||||
// Get new stream URL with selected audio track
|
||||
// Get new stream URL with selected audio track. It starts at zero — an
|
||||
// HLS playlist cannot carry a position (DR-181) — and `position` below
|
||||
// tells the frontend where to seek the reloaded element back to.
|
||||
let new_url = repository
|
||||
.get_video_stream_url(
|
||||
&jellyfin_item_id,
|
||||
media_source_id.as_deref(),
|
||||
current_position,
|
||||
Some(stream_index),
|
||||
)
|
||||
.await
|
||||
@@ -1600,7 +1611,6 @@ pub async fn player_set_stream_quality(
|
||||
.get_video_stream_url(
|
||||
&jellyfin_item_id,
|
||||
media_source_id.as_deref(),
|
||||
current_position,
|
||||
audio_stream_index,
|
||||
)
|
||||
.await
|
||||
@@ -1612,8 +1622,9 @@ pub async fn player_set_stream_quality(
|
||||
|
||||
// Native backend (Android/ExoPlayer): stop, repoint the queue entry at the
|
||||
// new URL, and reload — mirroring `VideoSeekStrategy::BackendReloadStream`.
|
||||
// The URL already carries `StartTimeTicks`, so the reloaded stream begins at
|
||||
// the current position rather than at zero.
|
||||
// The re-opened stream begins at zero (an HLS playlist cannot carry a start
|
||||
// position without 400ing every segment — DR-181), so it is seeked back to
|
||||
// where the picture was.
|
||||
{
|
||||
let controller = player.0.lock().await;
|
||||
controller.stop().map_err(|e| e.to_string())?;
|
||||
@@ -1631,6 +1642,9 @@ pub async fn player_set_stream_quality(
|
||||
controller
|
||||
.load_and_play(updated_item)
|
||||
.map_err(|e| e.to_string())?;
|
||||
if position > 0.0 {
|
||||
controller.seek(position).map_err(|e| e.to_string())?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(StreamQualityResponse::Native { position })
|
||||
|
||||
@@ -571,7 +571,13 @@ pub async fn repository_get_playback_info(
|
||||
.map_err(|e| format!("{:?}", e))
|
||||
}
|
||||
|
||||
/// Get video stream URL with optional seeking support
|
||||
/// Get a video stream URL.
|
||||
///
|
||||
/// There is no start-position parameter on purpose: the URL is an HLS playlist
|
||||
/// covering the whole item, and a position on it makes the server reject every
|
||||
/// segment with `400` (DR-181). Callers resume by seeking after load.
|
||||
///
|
||||
/// TRACES: UR-004 | DR-181 | UT-182
|
||||
#[tauri::command]
|
||||
#[specta::specta]
|
||||
pub async fn repository_get_video_stream_url(
|
||||
@@ -579,17 +585,11 @@ pub async fn repository_get_video_stream_url(
|
||||
handle: String,
|
||||
item_id: String,
|
||||
media_source_id: Option<String>,
|
||||
start_time_seconds: Option<f64>,
|
||||
audio_stream_index: Option<i32>,
|
||||
) -> Result<String, String> {
|
||||
let repo = manager.0.get(&handle).ok_or("Repository not found")?;
|
||||
repo.as_ref()
|
||||
.get_video_stream_url(
|
||||
&item_id,
|
||||
media_source_id.as_deref(),
|
||||
start_time_seconds,
|
||||
audio_stream_index,
|
||||
)
|
||||
.get_video_stream_url(&item_id, media_source_id.as_deref(), audio_stream_index)
|
||||
.await
|
||||
.map_err(|e| format!("{:?}", e))
|
||||
}
|
||||
|
||||
@@ -79,22 +79,20 @@ impl HybridRepository {
|
||||
self.online.get_jray_actors(item_id, t).await
|
||||
}
|
||||
|
||||
/// Get video stream URL with optional seeking support.
|
||||
/// This method is online-only since offline playback uses local file paths.
|
||||
/// Get video stream URL. This method is online-only since offline playback
|
||||
/// uses local file paths.
|
||||
///
|
||||
/// Takes no start position: the URL is an HLS playlist spanning the whole
|
||||
/// item, and a position on it would 400 every segment — see
|
||||
/// `OnlineRepository::get_video_stream_url`. Resume by seeking after load.
|
||||
pub async fn get_video_stream_url(
|
||||
&self,
|
||||
item_id: &str,
|
||||
media_source_id: Option<&str>,
|
||||
start_time_seconds: Option<f64>,
|
||||
audio_stream_index: Option<i32>,
|
||||
) -> Result<String, RepoError> {
|
||||
self.online
|
||||
.get_video_stream_url(
|
||||
item_id,
|
||||
media_source_id,
|
||||
start_time_seconds,
|
||||
audio_stream_index,
|
||||
)
|
||||
.get_video_stream_url(item_id, media_source_id, audio_stream_index)
|
||||
.await
|
||||
}
|
||||
|
||||
|
||||
@@ -494,33 +494,39 @@ impl OnlineRepository {
|
||||
}
|
||||
}
|
||||
|
||||
/// Get a video stream URL for playback at an arbitrary position (resume,
|
||||
/// transcoded seeking, audio-track switching).
|
||||
/// Get a video stream URL (initial play, resume, transcoded seeking,
|
||||
/// audio-track switching).
|
||||
///
|
||||
/// Returns an HLS master playlist (`/Videos/{id}/master.m3u8`) transcoded to
|
||||
/// h264/aac. HLS is used rather than a progressive `stream.mp4` because the
|
||||
/// HTML5 `<video>` element (via HLS.js) starts playing within seconds and can
|
||||
/// seek within the stream, whereas a progressive MP4 transcode of HEVC source
|
||||
/// forces the server to transcode the whole file before playback can begin —
|
||||
/// which manifests as playback never starting. `StartTimeTicks` makes the
|
||||
/// server begin the transcode at the requested position.
|
||||
/// which manifests as playback never starting.
|
||||
///
|
||||
/// **There is deliberately no start-position parameter.** A playlist covers
|
||||
/// the whole item and asking for segment N *is* the seek, so a position would
|
||||
/// be redundant — and actively fatal: Jellyfin builds every segment URI by
|
||||
/// echoing this playlist's query string into it, while its segment handler
|
||||
/// rejects `StartTimeTicks > 0` outright (`ArgumentException` → `400`). One
|
||||
/// resume position here therefore 400s every segment of the stream, which
|
||||
/// presents as a resumed episode that simply never plays while the same
|
||||
/// episode from the beginning is fine. Resume by seeking the player once it
|
||||
/// has loaded. (The progressive `/Audio/universal` builder below has no
|
||||
/// segments and keeps its `StartTimeTicks`.)
|
||||
///
|
||||
/// The stream is built against the current [`streaming_quality`] ceiling:
|
||||
/// `MaxStreamingBitrate`/`VideoBitrate`/`AudioBitrate`, plus a `MaxHeight`
|
||||
/// that suits the budget. `Original` keeps the historical 20/18 Mbps
|
||||
/// allowance, which is a transcode ceiling rather than a user-facing limit.
|
||||
///
|
||||
/// TRACES: UR-004, UR-074 | DR-140, DR-162, DR-177 | UT-130, UT-156, UT-173
|
||||
/// TRACES: UR-004, UR-074 | DR-140, DR-162, DR-177, DR-181 | UT-130, UT-156, UT-173, UT-182
|
||||
pub async fn get_video_stream_url(
|
||||
&self,
|
||||
item_id: &str,
|
||||
media_source_id: Option<&str>,
|
||||
start_time_seconds: Option<f64>,
|
||||
audio_stream_index: Option<i32>,
|
||||
) -> Result<String, RepoError> {
|
||||
// Convert seconds to ticks (10,000,000 ticks per second)
|
||||
let start_time_ticks = start_time_seconds.map(|seconds| (seconds * 10_000_000.0) as i64);
|
||||
|
||||
let quality = streaming_quality();
|
||||
// `Original` is uncapped as a *user* setting, but a transcode still needs
|
||||
// a ceiling to encode against — keep the values this endpoint has always
|
||||
@@ -589,10 +595,6 @@ impl OnlineRepository {
|
||||
params.push(("MediaSourceId", source_id.to_string()));
|
||||
}
|
||||
|
||||
if let Some(ticks) = start_time_ticks {
|
||||
params.push(("StartTimeTicks", ticks.to_string()));
|
||||
}
|
||||
|
||||
// Build query string (values are already safe, no encoding needed)
|
||||
let query = params
|
||||
.iter()
|
||||
@@ -1745,7 +1747,7 @@ impl MediaRepository for OnlineRepository {
|
||||
"[PlaybackInfo] Server offered direct play for audio the webview cannot decode ({:?}) — forcing an HLS transcode",
|
||||
audio_streams.first().and_then(|(codec, _)| *codec)
|
||||
);
|
||||
self.get_video_stream_url(item_id, Some(&source.id), None, None)
|
||||
self.get_video_stream_url(item_id, Some(&source.id), None)
|
||||
.await?
|
||||
} else {
|
||||
// Fall back to direct stream URL. No audioStreamIndex: static=true
|
||||
@@ -2680,7 +2682,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", None, None, None)
|
||||
.get_video_stream_url("vid-1", None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2702,7 +2704,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", None, None, None)
|
||||
.get_video_stream_url("vid-1", None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2740,16 +2742,25 @@ mod tests {
|
||||
assert!(url.contains("MaxStreamingBitrate=384000"), "url: {url}");
|
||||
}
|
||||
|
||||
/// Transcoded video must be an HLS master playlist, not a progressive
|
||||
/// `stream.mp4`: a progressive transcode of an HEVC source makes the server
|
||||
/// convert the whole file before serving a byte, which presents as playback
|
||||
/// that never starts. The chosen source and audio track ride along with it.
|
||||
///
|
||||
/// This is the surviving half of the old
|
||||
/// `test_get_video_stream_url_returns_hls_with_position`, whose other half
|
||||
/// asserted the `StartTimeTicks` that DR-181 removed — the position now
|
||||
/// belongs to a seek after load, never to this URL, so the assertion for it
|
||||
/// is gone rather than inverted (its inverse is UT-182's own test).
|
||||
///
|
||||
/// TRACES: UR-004 | DR-140, DR-181 | UT-130
|
||||
#[tokio::test]
|
||||
async fn test_get_video_stream_url_returns_hls_with_position() {
|
||||
// Transcoded video resume/seek must produce an HLS master playlist with
|
||||
// StartTimeTicks, not a progressive stream.mp4 (which never starts playing
|
||||
// for HEVC sources). See get_video_stream_url docs.
|
||||
async fn test_get_video_stream_url_returns_an_hls_master_playlist() {
|
||||
let _fixture = QualityFixture::set(StreamingQuality::Original);
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", Some("source-1"), Some(193.0), Some(1))
|
||||
.get_video_stream_url("vid-1", Some("source-1"), Some(1))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2760,18 +2771,57 @@ mod tests {
|
||||
assert!(url.contains("VideoCodec=h264"));
|
||||
assert!(url.contains("MediaSourceId=source-1"));
|
||||
assert!(url.contains("AudioStreamIndex=1"));
|
||||
// 193.0 seconds * 10_000_000 ticks/sec
|
||||
assert!(url.contains("StartTimeTicks=1930000000"), "url: {url}");
|
||||
assert!(!url.contains("stream.mp4"));
|
||||
}
|
||||
|
||||
/// Resuming a transcoded video played nothing at all: every segment came back
|
||||
/// `400`, hls.js exhausted its retries and gave up. Starting the same episode
|
||||
/// from the beginning was fine.
|
||||
///
|
||||
/// Jellyfin builds each segment URI by echoing the *master playlist's* query
|
||||
/// string into it (`CreateMainPlaylistRequest(… Request.QueryString …)`), and
|
||||
/// its segment handler opens with
|
||||
///
|
||||
/// ```csharp
|
||||
/// if ((streamingRequest.StartTimeTicks ?? 0) > 0)
|
||||
/// throw new ArgumentException("StartTimeTicks is not allowed.");
|
||||
/// ```
|
||||
///
|
||||
/// so a resume position put on the playlist is copied onto every
|
||||
/// `hls1/main/N.ts` and makes all of them 400. `> 0` is exactly why playing
|
||||
/// from the beginning survived.
|
||||
///
|
||||
/// HLS does not need the parameter: the playlist spans the whole item, and
|
||||
/// asking for segment N *is* the seek — the server transcodes from there. So
|
||||
/// the position never belongs in this URL; the player seeks after load. The
|
||||
/// sibling progressive `/Audio/universal` builder is a different endpoint with
|
||||
/// no segments, and keeps its `StartTimeTicks`.
|
||||
///
|
||||
/// TRACES: UR-004, UR-074 | DR-181 | UT-182
|
||||
#[tokio::test]
|
||||
async fn test_video_stream_url_never_carries_start_time_ticks() {
|
||||
let _fixture = QualityFixture::set(StreamingQuality::Original);
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", Some("source-1"), Some(1))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
!url.contains("StartTimeTicks"),
|
||||
"an HLS playlist must never carry StartTimeTicks — the server copies it \
|
||||
onto every segment URI and then rejects each one with 400: {url}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_get_video_stream_url_omits_position_when_absent() {
|
||||
let _fixture = QualityFixture::set(StreamingQuality::Original);
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", None, None, None)
|
||||
.get_video_stream_url("vid-1", None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2804,7 +2854,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", None, None, None)
|
||||
.get_video_stream_url("vid-1", None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2833,7 +2883,7 @@ mod tests {
|
||||
let repo = create_test_repository();
|
||||
|
||||
let url = repo
|
||||
.get_video_stream_url("vid-1", Some("source-1"), Some(12.0), Some(1))
|
||||
.get_video_stream_url("vid-1", Some("source-1"), Some(1))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user