fix(player): lockscreen skip scrubs instead of advancing in background audio
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%.
This commit is contained in:
+47
-2
@@ -314,6 +314,10 @@ use download::DownloadManager;
|
||||
use jellyfin::{HttpClient, HttpConfig};
|
||||
#[cfg(target_os = "android")]
|
||||
use playback_mode::PlaybackModeManager;
|
||||
// Only the Android MediaSessionHandler resolves lockscreen skips; on other
|
||||
// targets this would be an unused import.
|
||||
#[cfg(target_os = "android")]
|
||||
use player::seek::{resolve_skip_action, SkipAction};
|
||||
use player::{MediaSessionManager, PlayerBackend, PlayerController, TauriEventEmitter};
|
||||
// NullBackend is used both for platforms without a native backend AND as a graceful
|
||||
// fallback when a native backend (MPV/ExoPlayer) fails to initialize, so the app can
|
||||
@@ -449,14 +453,55 @@ impl MediaSessionHandler {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip means different things depending on what is actually playing, so
|
||||
// the decision belongs here rather than in the Kotlin that drew the
|
||||
// button: music advances the queue, while a video whose audio is running
|
||||
// through a background-audio handoff scrubs instead (UR-040). Routed
|
||||
// through the same spawn-and-seek path as "seek:" above, because
|
||||
// `seek_absolute` rebuilds the stream during a handoff and must not run
|
||||
// under the blocking lock (DR-159).
|
||||
//
|
||||
// TRACES: UR-040, UR-006 | DR-201
|
||||
if command == "next" || command == "previous" {
|
||||
let is_next = command == "next";
|
||||
let player = self.player.clone();
|
||||
tokio::spawn(async move {
|
||||
let controller = player.lock().await;
|
||||
let action = resolve_skip_action(
|
||||
is_next,
|
||||
controller.is_background_audio_active(),
|
||||
controller.position(),
|
||||
controller.duration(),
|
||||
);
|
||||
let label = if is_next { "next" } else { "previous" };
|
||||
let result: Result<(), String> = match action {
|
||||
SkipAction::Advance => if is_next {
|
||||
controller.next()
|
||||
} else {
|
||||
controller.previous()
|
||||
}
|
||||
.map_err(|e| e.to_string()),
|
||||
SkipAction::SeekTo(position) => {
|
||||
info!(
|
||||
"[MediaSession] Background audio: '{}' scrubs to {:.1}s",
|
||||
label, position
|
||||
);
|
||||
controller.seek_absolute(position).await
|
||||
}
|
||||
};
|
||||
if let Err(e) = result {
|
||||
error!("[MediaSession] Skip '{}' failed: {}", label, e);
|
||||
}
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Use blocking_lock since this is called from a non-async JNI callback
|
||||
let controller = self.player.blocking_lock();
|
||||
|
||||
let result = match command {
|
||||
"play" => controller.play(),
|
||||
"pause" => controller.pause(),
|
||||
"next" => controller.next(),
|
||||
"previous" => controller.previous(),
|
||||
"stop" => controller.stop(),
|
||||
_ => {
|
||||
warn!("[MediaSession] Unknown command: {}", command);
|
||||
|
||||
@@ -59,10 +59,149 @@ pub fn determine_video_seek_strategy(
|
||||
}
|
||||
}
|
||||
|
||||
// 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() {
|
||||
|
||||
Reference in New Issue
Block a user