From dd5d648d21110a139012c4837ebffca011afa7f3 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 20 Jun 2026 16:13:59 +0200 Subject: [PATCH] Workstream C: relocate video seek-strategy logic into player core Move the pure determine_video_seek_strategy function and its VideoSeekStrategy enum (plus the 5 seek-strategy unit tests) out of the command layer into player/seek.rs, where they belong and are testable without the Tauri State harness. commands/player.rs now imports them from crate::player. No behavior change. --- src-tauri/src/commands/player.rs | 140 +----------------------------- src-tauri/src/player/mod.rs | 2 + src-tauri/src/player/seek.rs | 143 +++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 137 deletions(-) create mode 100644 src-tauri/src/player/seek.rs diff --git a/src-tauri/src/commands/player.rs b/src-tauri/src/commands/player.rs index 66c747e..8fe8546 100644 --- a/src-tauri/src/commands/player.rs +++ b/src-tauri/src/commands/player.rs @@ -12,8 +12,9 @@ use super::DatabaseWrapper; use crate::download::cache::{CacheConfig, SmartCache}; use crate::jellyfin::{JellyfinClient, JellyfinConfig}; use crate::player::{ - AutoplaySettings, MediaItem, MediaSource, MediaType, MediaSessionManager, PlayerController, - PlayerState, PlayerStatusEvent, QueueContext, RepeatMode, SleepTimerMode, SleepTimerState, + determine_video_seek_strategy, AutoplaySettings, MediaItem, MediaSource, MediaType, + MediaSessionManager, PlayerController, PlayerState, PlayerStatusEvent, QueueContext, RepeatMode, + SleepTimerMode, SleepTimerState, VideoSeekStrategy, }; use crate::repository::{MediaRepository, types::{GetItemsOptions, ImageOptions, ImageType}}; use crate::settings::{AudioSettings, VideoSettings}; @@ -252,61 +253,6 @@ pub enum VideoSeekResponse { }, } -/// Internal enum for seek strategy decision (used for testing) -#[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 - } - } -} - /// Response for audio track switching operations #[derive(Debug, Serialize)] #[serde(tag = "strategy", rename_all = "camelCase")] @@ -2534,86 +2480,6 @@ pub async fn player_dismiss_session( #[cfg(test)] mod tests { - use super::*; - - /// 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); - } - /// Test track index finding in album /// This reproduces the bug where clicking songs 1-5 always played song 13 #[test] diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index 00bc393..1748ab3 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -7,6 +7,7 @@ pub mod backend; pub mod events; pub mod media; pub mod queue; +pub mod seek; pub mod session; pub mod sleep_timer; pub mod state; @@ -27,6 +28,7 @@ pub use backend::{NullBackend, PlayerBackend, PlayerError}; pub use events::{PlayerEventEmitter, PlayerStatusEvent, TauriEventEmitter}; pub use media::{MediaItem, MediaSource, MediaType, QueueContext}; pub use queue::{QueueManager, RepeatMode}; +pub use seek::{determine_video_seek_strategy, VideoSeekStrategy}; pub use session::{MediaSessionManager, MediaSessionType}; pub use sleep_timer::{SleepTimerMode, SleepTimerState}; pub use state::{EndReason, PlayerState}; diff --git a/src-tauri/src/player/seek.rs b/src-tauri/src/player/seek.rs new file mode 100644 index 0000000..595a8c8 --- /dev/null +++ b/src-tauri/src/player/seek.rs @@ -0,0 +1,143 @@ +//! 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 + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// 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); + } +}