Each of these was invisible while Linux video played in the webview, and each became reachable the moment mpv started rendering. DR-238 — a transcoded seek re-negotiates the stream on every renderer, not just the webview. `determine_video_seek_strategy` treated `is_hls` as a proxy for "seekable in place", which held only because hls.js was always the HLS renderer: it seeks within the VOD playlist it is handed and lets the server catch up. mpv's HLS demuxer cannot make Jellyfin transcode from a new offset, so with native video on, every transcoded seek became a backend seek that silently did nothing. One cell of the truth table changes; all four webview cells are byte-identical. DR-239 — properties the mpv event loop handles are now observed. libmpv delivers PropertyChange only for properties registered with observe_property, so the `pause` arm was unreachable code that read as implemented: StateChanged was never emitted and the play/pause control never moved. UT-218 asserts the two lists agree, so the class cannot recur. DR-240 — fullscreen moves whatever owns the pixels. requestFullscreen() fullscreens the *document*, which sufficed while the <video> element lived inside it and WebKit scaled it. A native surface is drawn behind the webview at window size, so a document-only fullscreen expanded the page and left the picture at its old size — on WebKitGTK, a maximised window with decorations still holding a strip of the screen. Measured on a 3440x1440 panel: 1361 tall before, 1440 after. DR-241 — a seek issued before mpv has a file to seek in is honoured rather than dropped. loadfile returns as soon as the command is queued, so `time-pos` does not resolve yet and setting it fails. The two callers that always hit that window are resume and a transcoded seek, both of which re-open the stream and then ask for a position; the failed seek was discarded and playback began at zero. Also adds the instrumentation that made the diagnosis possible rather than speculative: an entry log on player_stop, a render-size log that re-fires on change instead of latching once, and decoded-vs-display video geometry on file load. The last of those retired a wrong theory — a picture that does not fill an ultrawide turned out to be a 16:9 source with its letterbox baked in, not a rendering fault.
322 lines
12 KiB
Rust
322 lines
12 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
|
|
/// * `is_hls` - Whether the stream URL contains ".m3u8" (HLS stream)
|
|
/// * `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,
|
|
is_hls: 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 {
|
|
return if use_html5 {
|
|
if is_hls {
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
} else {
|
|
VideoSeekStrategy::Html5ReloadStream
|
|
}
|
|
} else {
|
|
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
|
|
);
|
|
}
|
|
|
|
/// Test video seek strategy for HLS streams
|
|
#[test]
|
|
fn test_seek_strategy_hls_stream() {
|
|
// HLS with HTML5 - frontend handles seek, don't call backend
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, false, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
// HLS with native backend - backend handles seek
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, false, false),
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
);
|
|
// HLS even with needs_transcoding flag - still native seek (HLS supports it)
|
|
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 | UT-217
|
|
#[test]
|
|
fn test_seek_strategy_transcoded_hls_native_backend() {
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, true, false),
|
|
VideoSeekStrategy::BackendReloadStream
|
|
);
|
|
// The HTML5 side of the same case is unchanged: hls.js seeks in-playlist.
|
|
assert_eq!(
|
|
determine_video_seek_strategy(false, true, true, true),
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
);
|
|
}
|
|
|
|
/// 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);
|
|
}
|
|
}
|