onSkipToNext/onSkipToPrevious forwarded a bare next/previous to Rust, which always advanced the queue. Correct for music, wrong for a video whose audio is running through a background-audio handoff (UR-040): pressing skip to re-hear a line jumped to the next episode instead of scrubbing. resolve_skip_action in player/seek.rs maps the command to Advance or SeekTo, and is_background_audio_active() is the whole test — the handoff exists only for video, and an episode played through it reports MediaType::Audio, so media type cannot distinguish the case. Forward 30s, back 10s, both clamped to [0, duration] so a skip near either end cannot seek negative or read as EOF and advance. Routed through the same spawn-then-seek_absolute path as the scrubber, because a handoff seek re-opens the stream and must not run under the blocking lock (DR-159). Kotlin keeps sending the opaque command; it only gains FAST_FORWARD/ REWIND in the PlaybackStateCompat so the system stops drawing skip arrows for a control that scrubs. The remote-volume action block is deliberately untouched: the handoff never applies to cast sessions, where skip really does mean advance. Tests written first and watched fail (left: Advance, right: SeekTo). 706 Rust tests pass, clippy 0, coverage 90%.
283 lines
10 KiB
Rust
283 lines
10 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;
|
|
}
|
|
|
|
// HLS streams and direct play (non-transcoded) support native seeking
|
|
if is_hls || !needs_transcoding {
|
|
if use_html5 {
|
|
// HTML5 backend - frontend handles seeking via videoElement.currentTime
|
|
// We don't call backend.seek() because video is in HTML5 element, not in MPV
|
|
VideoSeekStrategy::Html5NativeSeek
|
|
} else {
|
|
// Native backend (MPV) - backend handles seeking
|
|
VideoSeekStrategy::BackendNativeSeek
|
|
}
|
|
} else {
|
|
// Transcoded non-HLS streams need server-side seek (reload from new position)
|
|
if use_html5 {
|
|
VideoSeekStrategy::Html5ReloadStream
|
|
} else {
|
|
VideoSeekStrategy::BackendReloadStream
|
|
}
|
|
}
|
|
}
|
|
|
|
// 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
|
|
);
|
|
}
|
|
|
|
/// 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);
|
|
}
|
|
}
|