Joren/qbz
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(player): order gapless slot reset before track_id swap to close race

Browse Source 
The audio-thread iteration that swaps in the next gapless track
mutates several atomics in qbz-player::player::SharedState. The
polling loop in lib.rs reads those atomics independently (one load
per field) at ~250 ms cadence and emits a PlaybackEvent that the
frontend handlePlaybackEvent then classifies. When the polling load
falls inside the swap window, it can observe an inconsistent mix:
current_track_id already advanced to the new track, but
gapless_next_track_id still pointing at that same track because the
reset hasn't run yet.

The frontend's isGaplessTransition predicate
(playerStore.ts) requires both `event.track_id !== currentTrack.id`
AND `event.gapless_next_track_id === 0`. The mid-swap event
satisfies the first and fails the second, so the event is mis-
classified as an external track change. The "Track changed
externally" branch then asks the queue manager for the current
queue track (still the previous track because the gapless flow
hasn't called v2_next_track yet) and prints the now-familiar
"Queue track mismatch or null: 212171 vs event: 216597" warn.

In 1.2.8 this race was self-recovering: subsequent ticks observed
the consistent post-swap state and isGaplessTransition fired one
tick later. dee987c1a tightened the gate with
`gaplessAttemptedForCurrent`, which gets reset to null by the
mid-swap event's external-track path; subsequent ticks then fail
the new gate and the misclassification becomes permanent for the
remainder of the track. UI stays on the previous title while the
audio plays the new one, and the queue cursor never advances —
visible to the user as "the player says it's still on Viento but
Afuera is playing".

Fix the race at the producer side: in both transition swap paths
(mainline pos>=dur detection and ALSA Direct take_source_transition),
clear gapless_next_track_id and gapless_ready BEFORE storing the
new current_track_id. With SeqCst ordering, any reader that
observes the new track_id is guaranteed to also observe the
cleared gapless slot — eliminating the inconsistent intermediate
state by construction. Cheaper than coordinating reads on the
consumer side and keeps the polling loop's lock-free design.

cc
This commit is contained in:
vicrodh committed 2026-04-28 23:47:39 -06:00
1 parent 9f02efb8f8
commit 43ac3e18fa
1 file changed
+32 -9
Download Patch File Download Diff File Expand all files Collapse all files
crates/qbz-player/src/player/mod.rs
+32 -9
View File
@@ -2572,7 +2572,28 @@ impl Player {
let mut transition_consumed_pending = false;
// Gapless transition detection: when position exceeds current
// track duration, the queued next track has started playing
// track duration, the queued next track has started playing.
//
// ORDERING NOTE: the polling loop in lib.rs reads `track_id`,
// `gapless_next_track_id`, and other fields as separate atomic
// loads, so it can observe an inconsistent intermediate state
// if these stores aren't ordered carefully. The frontend's
// `isGaplessTransition` predicate requires
// `event.gapless_next_track_id === 0` AND
// `event.track_id !== currentTrack.id`. If the polling loop
// reads `track_id` post-swap but `gapless_next_track_id`
// pre-reset, both conditions can't be satisfied simultaneously
// and the frontend mis-classifies the gapless transition as an
// external track change — leaving the UI stuck on the previous
// title while the audio plays the new track.
//
// To eliminate that race, clear the "transition complete"
// markers (`gapless_next_track_id`, `gapless_ready`) BEFORE
// mutating `track_id`. Any racing reader either sees the old
// track_id with cleared slots (no transition observed yet, will
// catch up on next tick) or the new track_id with cleared slots
// (clean gapless transition), but never the inconsistent
// mid-swap mix.
if let Some(ref pending) = gapless_pending {
if dur > 0 && pos >= dur {
log::info!(
@@ -2580,6 +2601,12 @@ impl Player {
thread_state.current_track_id.load(Ordering::SeqCst),
pending.track_id, pos, dur
);
// Clear gapless slot markers FIRST so a racing reader
// never sees the inconsistent track_id-changed +
// slot-still-set combination.
thread_state.set_gapless_next_track_id(0);
thread_state.set_gapless_ready(false);
// Now safe to swap the track identity.
thread_state
.current_track_id
.store(pending.track_id, Ordering::SeqCst);
@@ -2591,11 +2618,6 @@ impl Player {
current_normalization_gain = pending.normalization_gain;
thread_state
.set_normalization_gain(pending.normalization_gain);
thread_state.set_gapless_next_track_id(0);
// Reset gapless_ready as well — the previous "approaching end"
// signal has been consumed by the transition, so any sticky
// true value would suppress the next track's signal.
thread_state.set_gapless_ready(false);
gapless_pending = None;
gapless_request_armed = false;
transition_consumed_pending = true;
@@ -2603,7 +2625,8 @@ impl Player {
}
// ALSA Direct gapless: the writer thread signals transitions
// via an atomic flag instead of position-based detection
// via an atomic flag instead of position-based detection.
// Same ordering rationale as above.
if let Some(ref engine) = current_engine {
if engine.take_source_transition() {
if let Some(ref pending) = gapless_pending {
@@ -2614,6 +2637,8 @@ impl Player {
.load(Ordering::SeqCst),
pending.track_id
);
thread_state.set_gapless_next_track_id(0);
thread_state.set_gapless_ready(false);
thread_state
.current_track_id
.store(pending.track_id, Ordering::SeqCst);
@@ -2625,8 +2650,6 @@ impl Player {
current_normalization_gain = pending.normalization_gain;
thread_state
.set_normalization_gain(pending.normalization_gain);
thread_state.set_gapless_next_track_id(0);
thread_state.set_gapless_ready(false);
gapless_pending = None;
gapless_request_armed = false;
transition_consumed_pending = true;