DR-246. The strategy used to turn on `is_hls` and `use_html5`, decided in a command handler on behalf of engines it does not own. That is how "who renders" came to mean "how do I seek", and why a transcoded seek silently did nothing the moment native video changed the renderer (DR-238). Engines now declare `Capabilities::seeks_transcoded_in_place` — true for hls.js, which seeks within the VOD playlist it was handed and lets the server catch up; false for mpv, whose HLS demuxer cannot make the server transcode from a new offset. The command asks whichever engine is rendering. Adding an engine no longer means editing a shared truth table. The item's transport is not read at the seek site any more; the compiler flagged it unused, which is the URL-shape input finally disappearing. A deviation from the spec, recorded deliberately: it called for the engine to own the decision outright. It cannot. Re-negotiating a stream needs the repository, which sits above the engine, so the engine states the ability and the caller acts on it. That still removes the defect — nobody guesses on another component's behalf — without pretending an engine can reach upward. Also fixes a latent race in the conformance suite, found by running it: the seek case asserted immediately, which passes on an engine that records the target when it accepts a seek and races on one that waits for the decoder to move. `Harness::await_seek` polls instead, the way the Android suite already did. It failed with machine load rather than with the code, which is the kind of test that teaches people to re-run until green. MpvPlayer 9/9 LegacyPlayer 8/9 - still only the mute/rate gap in the old trait 789 tests, clippy -D warnings clean with and without the feature.
338 lines
13 KiB
Rust
338 lines
13 KiB
Rust
//! Video seek strategy decision logic.
|
|
//!
|
|
//! This is pure logic, extracted from the command layer so it can be unit-tested
|
|
//! in the player core. The `player_seek_video` command translates the resulting
|
|
//! [`VideoSeekStrategy`] into a concrete backend/frontend action.
|
|
|
|
/// Seek strategy for video playback, derived from a stream's characteristics.
|
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
|
pub enum VideoSeekStrategy {
|
|
/// Local file - always use native seek on backend
|
|
LocalNativeSeek,
|
|
/// HLS or direct stream with HTML5 - frontend handles seek, skip backend
|
|
Html5NativeSeek,
|
|
/// HLS or direct stream with native backend - backend handles seek
|
|
BackendNativeSeek,
|
|
/// Transcoded non-HLS with HTML5 - reload stream, frontend handles
|
|
Html5ReloadStream,
|
|
/// Transcoded non-HLS with native backend - reload stream, backend handles
|
|
BackendReloadStream,
|
|
}
|
|
|
|
/// Determine the video seek strategy based on stream characteristics.
|
|
///
|
|
/// This is a pure function extracted for testability.
|
|
///
|
|
/// # Arguments
|
|
/// * `is_local` - Whether the file is a local download
|
|
/// * `seeks_transcoded_in_place` - Whether the engine rendering this stream
|
|
/// can seek a server-side transcode without re-opening it. Declared by the
|
|
/// engine via `Capabilities`, never inferred from the URL or the renderer.
|
|
/// * `needs_transcoding` - Whether the content needs transcoding
|
|
/// * `use_html5` - Whether frontend is using HTML5 video element
|
|
pub fn determine_video_seek_strategy(
|
|
is_local: bool,
|
|
seeks_transcoded_in_place: bool,
|
|
needs_transcoding: bool,
|
|
use_html5: bool,
|
|
) -> VideoSeekStrategy {
|
|
// Local files always support native seeking via backend
|
|
if is_local {
|
|
return VideoSeekStrategy::LocalNativeSeek;
|
|
}
|
|
|
|
// A server-side transcode is produced *from* `StartTimeTicks`, so where the
|
|
// seek lands is a property of the request, not of the stream in hand.
|
|
//
|
|
// hls.js is the exception: handed a VOD playlist it seeks within it and lets
|
|
// the server catch up segment by segment. mpv's HLS demuxer cannot make
|
|
// Jellyfin transcode from a new offset, so for the native backend a
|
|
// transcoded seek must re-negotiate the stream regardless of container.
|
|
//
|
|
// Before native video shipped, `use_html5` was always true for HLS and the
|
|
// native+HLS+transcode cell was unreachable, which is why `is_hls` alone
|
|
// used to be a safe proxy for "seekable in place". It no longer is: turning
|
|
// native video on routed every transcoded seek into a backend seek that
|
|
// silently does nothing, and presents as "resume does not work".
|
|
if needs_transcoding {
|
|
// Whether a transcode can be seeked in place is a property of the
|
|
// engine, and the engine states it. This used to be inferred from
|
|
// `is_hls`, which held only while hls.js was the sole HLS renderer —
|
|
// and stopped holding the moment mpv became one (DR-238).
|
|
return match (seeks_transcoded_in_place, use_html5) {
|
|
(true, true) => VideoSeekStrategy::Html5NativeSeek,
|
|
(true, false) => VideoSeekStrategy::BackendNativeSeek,
|
|
(false, true) => VideoSeekStrategy::Html5ReloadStream,
|
|
(false, false) => VideoSeekStrategy::BackendReloadStream,
|
|
};
|
|
}
|
|
|
|
// Direct play and direct stream are seekable where they sit.
|
|
if use_html5 {
|
|
// The frontend seeks via videoElement.currentTime; calling backend.seek()
|
|
// would move a player that is not the one rendering.
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
} else {
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
}
|
|
}
|
|
|
|
// The four items below are consumed by the Android MediaSessionHandler; on other
|
|
// targets only the tests exercise them, so dead-code analysis would flag them.
|
|
|
|
/// How far a lockscreen skip-forward jumps while background audio owns playback.
|
|
#[cfg_attr(not(target_os = "android"), allow(dead_code))]
|
|
pub const SKIP_FORWARD_SECONDS: f64 = 30.0;
|
|
|
|
/// How far a lockscreen skip-back jumps while background audio owns playback.
|
|
///
|
|
/// Deliberately shorter than the forward jump: the back button is used to replay
|
|
/// dialogue just missed, not to travel.
|
|
#[cfg_attr(not(target_os = "android"), allow(dead_code))]
|
|
pub const SKIP_BACK_SECONDS: f64 = 10.0;
|
|
|
|
/// What a lockscreen skip button means for the playback that is actually running.
|
|
#[cfg_attr(not(target_os = "android"), allow(dead_code))]
|
|
#[derive(Debug, Clone, Copy, PartialEq)]
|
|
pub enum SkipAction {
|
|
/// Move to the next/previous queue entry — a track, or an episode.
|
|
Advance,
|
|
/// Scrub within the current item, to this absolute position in seconds.
|
|
SeekTo(f64),
|
|
}
|
|
|
|
/// Decide whether a lockscreen skip advances the queue or scrubs the current item.
|
|
///
|
|
/// Music gets queue advance, which is what the buttons look like they do. A video
|
|
/// whose audio is playing through a background-audio handoff (UR-040) gets a
|
|
/// relative scrub instead: there is no meaningful "next track" inside a film, and
|
|
/// jumping to the next *episode* because the user wanted to re-hear a line is a
|
|
/// much worse outcome than a scrub.
|
|
///
|
|
/// `is_background_audio` is the whole test, and it is sufficient on its own —
|
|
/// the handoff exists only for video, and an episode played through it reports
|
|
/// `MediaType::Audio`, so media type cannot distinguish this case (see the note
|
|
/// at `PlayerController::auto_advance_to_next_episode`).
|
|
///
|
|
/// Clamped to `[0, duration]` so a skip near either end lands in the item rather
|
|
/// than at a negative offset or past the end, which some backends treat as EOF
|
|
/// and would turn a scrub into an unintended advance.
|
|
///
|
|
/// TRACES: UR-040, UR-006 | DR-201
|
|
#[cfg_attr(not(target_os = "android"), allow(dead_code))]
|
|
pub fn resolve_skip_action(
|
|
is_next: bool,
|
|
is_background_audio: bool,
|
|
position: f64,
|
|
duration: Option<f64>,
|
|
) -> SkipAction {
|
|
if !is_background_audio {
|
|
return SkipAction::Advance;
|
|
}
|
|
|
|
let target = if is_next {
|
|
position + SKIP_FORWARD_SECONDS
|
|
} else {
|
|
position - SKIP_BACK_SECONDS
|
|
};
|
|
|
|
let clamped = match duration {
|
|
Some(d) if d > 0.0 => target.clamp(0.0, d),
|
|
_ => target.max(0.0),
|
|
};
|
|
|
|
SkipAction::SeekTo(clamped)
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
/// Music (no background-audio handoff) keeps queue advance on both buttons.
|
|
///
|
|
/// TRACES: UR-006 | DR-201 | UT-194
|
|
#[test]
|
|
fn test_skip_advances_queue_for_normal_audio() {
|
|
assert_eq!(
|
|
resolve_skip_action(true, false, 42.0, Some(300.0)),
|
|
SkipAction::Advance
|
|
);
|
|
assert_eq!(
|
|
resolve_skip_action(false, false, 42.0, Some(300.0)),
|
|
SkipAction::Advance
|
|
);
|
|
}
|
|
|
|
/// The reported bug: in background-audio mode the lockscreen skip buttons
|
|
/// advanced to the next/previous episode instead of scrubbing, so trying to
|
|
/// re-hear a line jumped out of the film entirely.
|
|
///
|
|
/// TRACES: UR-040 | DR-201 | UT-195
|
|
#[test]
|
|
fn test_skip_scrubs_in_background_audio_mode() {
|
|
assert_eq!(
|
|
resolve_skip_action(true, true, 100.0, Some(3600.0)),
|
|
SkipAction::SeekTo(130.0)
|
|
);
|
|
assert_eq!(
|
|
resolve_skip_action(false, true, 100.0, Some(3600.0)),
|
|
SkipAction::SeekTo(90.0)
|
|
);
|
|
}
|
|
|
|
/// Skipping back near the start clamps to zero rather than going negative,
|
|
/// which backends reject (the "Raw(-10)" class of error).
|
|
///
|
|
/// TRACES: UR-040 | DR-201 | UT-196
|
|
#[test]
|
|
fn test_skip_back_clamps_at_start() {
|
|
assert_eq!(
|
|
resolve_skip_action(false, true, 4.0, Some(3600.0)),
|
|
SkipAction::SeekTo(0.0)
|
|
);
|
|
}
|
|
|
|
/// Skipping forward near the end clamps to the duration instead of running
|
|
/// past it, which would read as end-of-stream and advance — the very thing
|
|
/// this function exists to prevent.
|
|
///
|
|
/// TRACES: UR-040 | DR-201 | UT-197
|
|
#[test]
|
|
fn test_skip_forward_clamps_at_end() {
|
|
assert_eq!(
|
|
resolve_skip_action(true, true, 3590.0, Some(3600.0)),
|
|
SkipAction::SeekTo(3600.0)
|
|
);
|
|
}
|
|
|
|
/// An unknown duration still scrubs, and still refuses to go negative.
|
|
///
|
|
/// TRACES: UR-040 | DR-201 | UT-198
|
|
#[test]
|
|
fn test_skip_without_duration_still_scrubs() {
|
|
assert_eq!(
|
|
resolve_skip_action(true, true, 10.0, None),
|
|
SkipAction::SeekTo(40.0)
|
|
);
|
|
assert_eq!(
|
|
resolve_skip_action(false, true, 3.0, None),
|
|
SkipAction::SeekTo(0.0)
|
|
);
|
|
}
|
|
|
|
/// Test video seek strategy for local files
|
|
#[test]
|
|
fn test_seek_strategy_local_file() {
|
|
// Local files always use native backend seek regardless of other flags
|
|
assert_eq!(
|
|
determine_video_seek_strategy(true, false, false, false),
|
|
VideoSeekStrategy::LocalNativeSeek
|
|
);
|
|
assert_eq!(
|
|
determine_video_seek_strategy(true, false, false, true),
|
|
VideoSeekStrategy::LocalNativeSeek
|
|
);
|
|
assert_eq!(
|
|
determine_video_seek_strategy(true, true, true, true),
|
|
VideoSeekStrategy::LocalNativeSeek
|
|
);
|
|
}
|
|
|
|
/// Non-transcoded streams seek in place regardless of the engine's
|
|
/// transcode ability, which only applies to transcodes.
|
|
#[test]
|
|
fn test_seek_strategy_direct_stream() {
|
|
// HTML5 renders, so the frontend seeks the element
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, false, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
// The native engine renders, so it seeks
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, false, false),
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
);
|
|
// A transcode an engine says it can move: seek in place
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, true, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
}
|
|
|
|
/// A server-side transcode cannot be seeked by the native backend.
|
|
///
|
|
/// Jellyfin produces a transcode from `StartTimeTicks`; hls.js can seek
|
|
/// within the VOD playlist it is handed, but mpv's HLS demuxer cannot make
|
|
/// the server transcode from a new offset, so the stream has to be
|
|
/// re-negotiated. Before native video existed, `use_html5` was always true
|
|
/// for HLS and this case was unreachable — turning native video on routed
|
|
/// every transcoded seek into a native seek that silently does nothing,
|
|
/// which presents as "resume does not work".
|
|
///
|
|
/// TRACES: UR-040 | DR-238, DR-246 | UT-217
|
|
#[test]
|
|
fn test_transcoded_seek_follows_the_engines_declared_ability() {
|
|
// An engine that cannot move a server-side transcode re-opens it,
|
|
// whichever side is rendering.
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, true, false),
|
|
VideoSeekStrategy::BackendReloadStream
|
|
);
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, true, true),
|
|
VideoSeekStrategy::Html5ReloadStream
|
|
);
|
|
// hls.js can, and says so, so it seeks in place.
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, true, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
// The container the stream arrives in no longer decides anything: the
|
|
// same declared ability gives the same answer on the native side.
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, true, false),
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
);
|
|
}
|
|
|
|
/// Test video seek strategy for direct play (non-transcoded) streams
|
|
#[test]
|
|
fn test_seek_strategy_direct_play() {
|
|
// Direct play with HTML5 - frontend handles seek
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, false, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
// Direct play with native backend - backend handles seek
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, false, false),
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
);
|
|
}
|
|
|
|
/// Test video seek strategy for transcoded non-HLS streams
|
|
#[test]
|
|
fn test_seek_strategy_transcoded_non_hls() {
|
|
// Transcoded non-HLS with HTML5 - need to reload stream, frontend handles
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, true, true),
|
|
VideoSeekStrategy::Html5ReloadStream
|
|
);
|
|
// Transcoded non-HLS with native backend - need to reload stream, backend handles
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, false, true, false),
|
|
VideoSeekStrategy::BackendReloadStream
|
|
);
|
|
}
|
|
|
|
/// Test the specific bug fix: HLS + HTML5 should NOT call backend seek
|
|
/// This was the bug causing "Raw(-10)" errors
|
|
#[test]
|
|
fn test_hls_html5_does_not_use_backend_seek() {
|
|
let strategy = determine_video_seek_strategy(false, true, false, true);
|
|
// Should be Html5NativeSeek, NOT BackendNativeSeek
|
|
assert_eq!(strategy, VideoSeekStrategy::Html5NativeSeek);
|
|
assert_ne!(strategy, VideoSeekStrategy::BackendNativeSeek);
|
|
}
|
|
}
|