fix(player): expired sleep timer stops without triggering autoplay
Stopping the backend makes the native player fire its ended callback, which lands in on_playback_ended. The timer thread cancels the timer first, so by the time the callback inspects it the mode reads Off — the sleep-timer branch is skipped and the episode path runs, showing a next-episode popup (or advancing outright) right after the user's sleep timer expired. Record EndReason::UserStop before the stop reaches the backend. That is the honest label: the stop was user-initiated, just via the timer they set rather than the stop button. TRACES: UR-023, UR-026 | DR-029
This commit is contained in:
@@ -243,6 +243,23 @@ impl PlayerController {
|
|||||||
self.end_reason.lock_safe().take()
|
self.end_reason.lock_safe().take()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record that playback is being stopped by an expiring sleep timer.
|
||||||
|
///
|
||||||
|
/// Stopping the backend makes it fire its ended callback (ExoPlayer does on
|
||||||
|
/// Android), which lands in `on_playback_ended`. Without an end reason that
|
||||||
|
/// reads as a natural finish and autoplay advances — defeating the timer.
|
||||||
|
/// `UserStop` is the honest label: the stop was user-initiated, just via the
|
||||||
|
/// timer they set rather than the stop button.
|
||||||
|
///
|
||||||
|
/// Takes the shared slot rather than `&self` so the sleep-timer thread —
|
||||||
|
/// which owns clones, not the controller — records it the same way.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-023, UR-026 | DR-029
|
||||||
|
fn note_sleep_timer_stop(end_reason: &Arc<Mutex<Option<EndReason>>>) {
|
||||||
|
log::debug!("[PlayerController] Sleep timer stop: marking end reason UserStop");
|
||||||
|
*end_reason.lock_safe() = Some(EndReason::UserStop);
|
||||||
|
}
|
||||||
|
|
||||||
/// Increment autoplay episode counter. Returns true if limit is reached.
|
/// Increment autoplay episode counter. Returns true if limit is reached.
|
||||||
fn increment_autoplay_count(&self) -> bool {
|
fn increment_autoplay_count(&self) -> bool {
|
||||||
let max = self.autoplay_settings.lock_safe().max_episodes;
|
let max = self.autoplay_settings.lock_safe().max_episodes;
|
||||||
@@ -767,6 +784,7 @@ impl PlayerController {
|
|||||||
let sleep_timer = self.sleep_timer.clone();
|
let sleep_timer = self.sleep_timer.clone();
|
||||||
let event_emitter = self.event_emitter.clone();
|
let event_emitter = self.event_emitter.clone();
|
||||||
let backend = self.backend.clone();
|
let backend = self.backend.clone();
|
||||||
|
let end_reason = self.end_reason.clone();
|
||||||
|
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
loop {
|
loop {
|
||||||
@@ -783,6 +801,14 @@ impl PlayerController {
|
|||||||
debug!("[SleepTimer] Time-based timer expired, stopping playback");
|
debug!("[SleepTimer] Time-based timer expired, stopping playback");
|
||||||
timer.cancel();
|
timer.cancel();
|
||||||
|
|
||||||
|
// Mark the stop *before* it reaches the backend. Stopping
|
||||||
|
// makes the native player fire its ended callback, and
|
||||||
|
// cancelling the timer above means on_playback_ended can no
|
||||||
|
// longer tell this apart from a natural end — without this
|
||||||
|
// it would show the next-episode popup / autoplay right
|
||||||
|
// after the sleep timer fired.
|
||||||
|
Self::note_sleep_timer_stop(&end_reason);
|
||||||
|
|
||||||
// Emit cancelled state
|
// Emit cancelled state
|
||||||
if let Some(emitter) = event_emitter.lock_safe().as_ref() {
|
if let Some(emitter) = event_emitter.lock_safe().as_ref() {
|
||||||
emitter.emit(PlayerStatusEvent::SleepTimerChanged {
|
emitter.emit(PlayerStatusEvent::SleepTimerChanged {
|
||||||
@@ -2040,6 +2066,51 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A time-based sleep timer that fires mid-episode must not let the ended
|
||||||
|
/// callback fall through to autoplay.
|
||||||
|
///
|
||||||
|
/// The timer thread stops the backend directly, which makes ExoPlayer emit
|
||||||
|
/// its ended callback. That callback races the thread's own `timer.cancel()`:
|
||||||
|
/// by the time `on_playback_ended` inspects the sleep timer it reads `Off`,
|
||||||
|
/// so the timer branch is skipped and the episode path runs — showing a
|
||||||
|
/// next-episode popup (or advancing) after the user's sleep timer expired.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_expired_time_sleep_timer_stops_without_autoplay() {
|
||||||
|
let controller = PlayerController::default();
|
||||||
|
|
||||||
|
let items = create_test_items(3);
|
||||||
|
controller.play_queue(items, 0).unwrap();
|
||||||
|
controller.take_end_reason();
|
||||||
|
|
||||||
|
// Arm a time-based timer that is already due, then let the real timer
|
||||||
|
// thread (started in the constructor, 1s tick) observe the expiry and
|
||||||
|
// run its stop path. Driving the actual thread is the point: the bug was
|
||||||
|
// that this path stopped the backend without recording an end reason.
|
||||||
|
let now = chrono::Utc::now().timestamp_millis();
|
||||||
|
controller.set_sleep_timer(SleepTimerMode::Time { end_time: now });
|
||||||
|
|
||||||
|
// Wait for the timer thread to process the expiry (tick is 1s).
|
||||||
|
for _ in 0..40 {
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
|
||||||
|
if !controller.sleep_timer.lock_safe().is_active() {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
!controller.sleep_timer.lock_safe().is_active(),
|
||||||
|
"Timer thread should have expired and cancelled the sleep timer"
|
||||||
|
);
|
||||||
|
|
||||||
|
// The backend stop above makes the native player fire its ended callback.
|
||||||
|
let decision = controller.on_playback_ended().await.unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
matches!(decision, AutoplayDecision::Stop),
|
||||||
|
"Expected Stop after an expired time-based sleep timer, got {:?}",
|
||||||
|
decision
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_empty_queue_stops() {
|
async fn test_empty_queue_stops() {
|
||||||
let controller = PlayerController::default();
|
let controller = PlayerController::default();
|
||||||
|
|||||||
Reference in New Issue
Block a user