fix(player,reporting): report real positions, and count an audio-only episode as watched
Returning to the foreground before the background-audio stream had started
playing handed the frontend 0.0s, so the video reloaded at StartTimeTicks=0 —
the episode restarted from the beginning — and the stop report that followed
wrote that zero to Jellyfin as the resume point. Caught on device: locked at
18.4s, unlocked 3.5s later with ExoPlayer still IDLE.
The base that turns a handoff's relative timeline into the episode's is applied
once at the native tick boundary (DR-159), so before the first tick nothing has
applied it. The same blind spot covers webview-rendered media, where nothing is
loaded into the native backend at all and its position is a permanent 0 — which
is why 14 of 14 stop reports in a 35-minute trace were zeroes, one landing 40s
after the frontend had correctly reported 15:22 for the same episode.
- absolute_position(): the maximum of the backend's reading, the last position
webview media reported, and the handoff base. Exact rather than heuristic —
at most one term is ever meaningful, and the base is a floor the stream
cannot physically be behind. duration() gains the same fallback.
- Withhold zero-position stop reports. A zero is never information, and
Jellyfin stores the reported position as the resume point, so sending one
only ever destroys a real one.
- Report progress from the controller's own position ticks, through the 30s
throttler it already shared with the native audio path.
/Sessions/Playing/Progress was previously requested zero times in 35 minutes.
- Report a finished audio-only episode stopped at its runtime before advancing,
so Jellyfin's 90% rule marks it played. Nothing else can: the webview is
suspended and its <video> was torn down at the handoff.
- Split the handoff by source — a downloaded file takes no base and a real
seek, a stream keeps its StartTimeTicks base and no seek — and stop routing a
downloaded handoff's absolute seek through the stream rebuild, which refuses
a non-remote source outright.
Reports go through a PlaybackReportSink, which also collapses three copies of
spawn-a-task-and-hope into one and is what let each of these be written as a
failing test first.
TRACES: UR-005, UR-025, UR-040, UR-071 | DR-178, DR-179, DR-180 |
UT-176, UT-177, UT-178, UT-179, UT-180, UT-181
This commit is contained in:
@@ -454,6 +454,49 @@ pub(super) fn background_audio_source(
|
||||
}
|
||||
}
|
||||
|
||||
/// How a background-audio handoff must start playback, given where its audio
|
||||
/// actually begins.
|
||||
///
|
||||
/// TRACES: UR-040, UR-071 | DR-180 | UT-181
|
||||
pub(super) struct BackgroundAudioPlan {
|
||||
/// The position the stream's own zero corresponds to, recorded as the
|
||||
/// handoff base so later readings can be shifted back to the episode's
|
||||
/// timeline.
|
||||
pub base_seconds: f64,
|
||||
/// Where to seek after loading, if the source does not already start there.
|
||||
pub seek_to: Option<f64>,
|
||||
}
|
||||
|
||||
/// Decide the base and the seek for a handoff at `position_seconds`.
|
||||
///
|
||||
/// The two sources start in different places. An audio-only **stream** is built
|
||||
/// with `StartTimeTicks`, so the server makes the handoff point that stream's
|
||||
/// zero: the base is the handoff position, and seeking would skip *past* the
|
||||
/// content by that much again. A downloaded **file** has no such parameter and
|
||||
/// begins at the episode's own zero, so it needs the opposite — no base, and a
|
||||
/// real seek. Treating a file like a stream is why backgrounding a downloaded
|
||||
/// episode restarted it from 0:00 while the lockscreen showed the right time.
|
||||
///
|
||||
/// TRACES: UR-040, UR-071 | DR-180 | UT-181
|
||||
pub(super) fn background_audio_plan(
|
||||
is_local_file: bool,
|
||||
position_seconds: f64,
|
||||
) -> BackgroundAudioPlan {
|
||||
let position = position_seconds.max(0.0);
|
||||
|
||||
if is_local_file {
|
||||
BackgroundAudioPlan {
|
||||
base_seconds: 0.0,
|
||||
seek_to: (position > 0.0).then_some(position),
|
||||
}
|
||||
} else {
|
||||
BackgroundAudioPlan {
|
||||
base_seconds: position,
|
||||
seek_to: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the on-disk file backing a completed download, if there is one.
|
||||
///
|
||||
/// A `downloads` row is not proof of a file: it can outlive the bytes (manual
|
||||
@@ -710,6 +753,9 @@ pub async fn player_enter_background_audio(
|
||||
item.id
|
||||
);
|
||||
}
|
||||
// A downloaded file starts at the episode's zero; a stream starts at the
|
||||
// handoff point. Only one of them has a base, and only the other needs a seek.
|
||||
let plan = background_audio_plan(local_path.is_some(), position_seconds);
|
||||
let source = background_audio_source(local_path, item.stream_url, &item.id);
|
||||
|
||||
// Build an AUDIO media item pointing at the audio-only stream. We do not use
|
||||
@@ -753,21 +799,28 @@ pub async fn player_enter_background_audio(
|
||||
// Same base offset drives the lockscreen scrubber: ExoPlayer reports position
|
||||
// relative to the stream's StartTimeTicks zero, but the metadata duration is
|
||||
// absolute, so shift the reported position back to absolute for the scrubber.
|
||||
let _ = crate::player::set_lockscreen_position_offset(position_seconds.max(0.0));
|
||||
let _ = crate::player::set_lockscreen_position_offset(plan.base_seconds);
|
||||
|
||||
let controller = player.0.lock().await;
|
||||
// Remember where the video was: the audio stream's zero == this position
|
||||
// (the URL was built with StartTimeTicks=position_seconds), so on exit we add
|
||||
// this base to the native player's relative position to get the absolute one.
|
||||
// The controller owns it so a backend-driven advance to the next episode
|
||||
// clears it along with the stream it described.
|
||||
controller.enter_background_audio(position_seconds);
|
||||
// Remember where the video was: for a stream the audio's zero == this
|
||||
// position (the URL was built with StartTimeTicks=position_seconds), so on
|
||||
// exit we add this base to the native player's relative position to get the
|
||||
// absolute one. The controller owns it so a backend-driven advance to the
|
||||
// next episode clears it along with the stream it described.
|
||||
controller.enter_background_audio(plan.base_seconds);
|
||||
controller
|
||||
.play_item(media_item)
|
||||
.map_err(|e| e.to_string())?;
|
||||
// NOTE: do NOT seek here. The audio-only URL already starts at the handoff
|
||||
// position via StartTimeTicks; the stream's timeline begins at 0 == that
|
||||
// point, so an extra seek(position_seconds) would jump PAST the content.
|
||||
// Seek ONLY a local file. The audio-only URL already starts at the handoff
|
||||
// position via StartTimeTicks — its timeline begins at 0 == that point — so
|
||||
// seeking a stream would jump PAST the content by the handoff position again.
|
||||
if let Some(seek_to) = plan.seek_to {
|
||||
info!(
|
||||
"player_enter_background_audio: seeking the downloaded file to {:.1}s",
|
||||
seek_to
|
||||
);
|
||||
controller.seek(seek_to).map_err(|e| e.to_string())?;
|
||||
}
|
||||
|
||||
controller.emit_queue_changed();
|
||||
if let Some(emitter) = controller.event_emitter() {
|
||||
@@ -801,7 +854,14 @@ pub async fn player_exit_background_audio(
|
||||
// moment it matters most. Capturing into a `let` before stop() is also the
|
||||
// lock discipline from CLAUDE.md: never hold work across a re-entrant call.
|
||||
// (DR-159)
|
||||
let absolute = controller.position();
|
||||
//
|
||||
// `absolute_position` rather than `position`, because a tick that has not
|
||||
// landed *yet* is the same hazard from the other side: returning to the
|
||||
// foreground while the audio-only transcode is still opening read 0.0, and
|
||||
// the video reloaded at StartTimeTicks=0 — the episode restarting from the
|
||||
// beginning. Flooring at the handoff base cannot overshoot: the stream is
|
||||
// physically incapable of being behind its own starting point. (DR-178)
|
||||
let absolute = controller.absolute_position();
|
||||
|
||||
// Now safe to tear the handoff down, native side first.
|
||||
let _ = crate::player::set_lockscreen_position_offset(0.0);
|
||||
@@ -1846,7 +1906,11 @@ pub(super) fn get_player_status(controller: &PlayerController) -> PlayerStatus {
|
||||
|
||||
PlayerStatus {
|
||||
state: controller.state(),
|
||||
position: controller.position(),
|
||||
// The position on the item's timeline, whichever of the three paths is
|
||||
// rendering it — the native backend answers for only one of them, and
|
||||
// reads 0 for webview video and for a handoff that has not ticked yet.
|
||||
// TRACES: UR-005 | DR-178
|
||||
position: controller.absolute_position(),
|
||||
duration: controller.duration(),
|
||||
volume: controller.volume(),
|
||||
muted: controller.muted(),
|
||||
@@ -2838,6 +2902,49 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The two sources start in different places, so the handoff cannot treat
|
||||
/// them alike.
|
||||
///
|
||||
/// An audio-only *stream* is built with `StartTimeTicks`, so the server makes
|
||||
/// the handoff point that stream's zero: the base is the handoff position and
|
||||
/// seeking would jump past the content. A *downloaded file* has no such
|
||||
/// parameter — it starts at the episode's own zero — so basing it at the
|
||||
/// handoff position claims 18 minutes of audio that is about to play from the
|
||||
/// beginning. That is the downloaded-episode version of "it restarts when the
|
||||
/// screen sleeps", and it needs the opposite treatment: no base, and a seek.
|
||||
///
|
||||
/// TRACES: UR-040, UR-071 | DR-180 | UT-181
|
||||
#[test]
|
||||
fn test_background_audio_plan_seeks_a_file_and_bases_a_stream() {
|
||||
use super::background_audio_plan;
|
||||
|
||||
let local = background_audio_plan(true, 1104.0);
|
||||
assert_eq!(local.base_seconds, 0.0);
|
||||
assert_eq!(local.seek_to, Some(1104.0));
|
||||
|
||||
let streamed = background_audio_plan(false, 1104.0);
|
||||
assert_eq!(streamed.base_seconds, 1104.0);
|
||||
assert_eq!(
|
||||
streamed.seek_to, None,
|
||||
"the URL already starts at the handoff point; seeking again skips past it"
|
||||
);
|
||||
}
|
||||
|
||||
/// Handing off at the very start has nothing to seek to and nothing to base:
|
||||
/// both sources are already where they need to be.
|
||||
///
|
||||
/// TRACES: UR-040, UR-071 | DR-180 | UT-181
|
||||
#[test]
|
||||
fn test_background_audio_plan_at_the_start_neither_seeks_nor_bases() {
|
||||
use super::background_audio_plan;
|
||||
|
||||
for local in [true, false] {
|
||||
let plan = background_audio_plan(local, 0.0);
|
||||
assert_eq!(plan.base_seconds, 0.0);
|
||||
assert_eq!(plan.seek_to, None);
|
||||
}
|
||||
}
|
||||
|
||||
/// A downloaded item must resolve to its file, and a `downloads` row whose
|
||||
/// file has gone must resolve to `None` so the caller falls back to
|
||||
/// streaming instead of handing the player a path that cannot be opened.
|
||||
|
||||
Reference in New Issue
Block a user