Collapse duplicate same-track streaming loads to enforce one load per play
A second LoadTrackStreaming for the same in-flight track (UI double-fire, queue re-entry, or JS false-end auto-advance) is now dropped; a different-track load still supersedes. Targets the Opus double-load; keeps load-gen diagnostics.
This commit is contained in:
@@ -53,6 +53,16 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
|
||||
private readonly ILogger<StreamingAudioPlayerService> _logger;
|
||||
private string? _currentTrackId;
|
||||
|
||||
// The track key of the load currently in flight, set at LoadTrackStreaming entry BEFORE ResetToIdle
|
||||
// (which clears _currentTrackId) and cleared only when that load's finally runs as the still-active
|
||||
// operation. It is the idempotency key that collapses a duplicate same-track entry for ONE play
|
||||
// action into the first load: a second SelectTrackStreaming for the SAME track while its load is
|
||||
// still in flight is a redundant re-dispatch (UI double-fire, queue re-entry, or a JS false-end
|
||||
// auto-advance back onto the same track), not a real track switch, so it is dropped. A load for a
|
||||
// DIFFERENT track is a genuine switch and supersedes as before — this guard never suppresses it.
|
||||
// Null when no load is in flight.
|
||||
private string? _loadInFlightTrackId;
|
||||
|
||||
// Monotonic load-generation counter (diagnostic). Incremented on every LoadTrackStreaming entry and
|
||||
// stamped into the load's logs so two loads for ONE user play action — the "Duration set from header
|
||||
// logged twice" double-load hypothesis that needs in-browser confirmation — are unmistakable: a
|
||||
@@ -165,12 +175,29 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
|
||||
|
||||
private async Task LoadTrackStreaming(TrackDto track)
|
||||
{
|
||||
// Idempotency guard (single-load invariant). A load already in flight for THIS SAME track means
|
||||
// a duplicate dispatch of one play action — a UI double-fire, a queue re-entry, or a JS false-end
|
||||
// that auto-advanced back onto the same track. Dropping it collapses the play to exactly one load
|
||||
// without touching the live operation. A load for a DIFFERENT track is a real switch and falls
|
||||
// through to supersede the in-flight one exactly as before, so this never blocks navigation. The
|
||||
// check is on the WASM single-threaded dispatcher, so reading/writing _loadInFlightTrackId needs
|
||||
// no lock; the field is set below before the first await and cleared in finally for the active load.
|
||||
if (_loadInFlightTrackId is { } inFlight && inFlight == track.EntryKey)
|
||||
{
|
||||
_logger.LogInformation(
|
||||
"Streaming load for track {TrackId} skipped — a load for the same track is already in flight (single-load guard)",
|
||||
track.EntryKey);
|
||||
return;
|
||||
}
|
||||
_loadInFlightTrackId = track.EntryKey;
|
||||
|
||||
// Always reset to clean state before loading new track. ResetToIdle
|
||||
// both cancels and awaits any in-flight streaming loop, so by the time
|
||||
// we return from it the previous loop is guaranteed to have exited and
|
||||
// there is no risk of interleaved ProcessStreamingChunk calls against
|
||||
// the single-instance JS StreamDecoder.
|
||||
await ResetToIdle();
|
||||
// the single-instance JS StreamDecoder. clearLoadGuard:false — we just armed
|
||||
// _loadInFlightTrackId for THIS load; the prologue reset must not wipe it.
|
||||
await ResetToIdle(clearLoadGuard: false);
|
||||
|
||||
// Stamp this load with a fresh generation id (diagnostic — see _loadGeneration). Logged at
|
||||
// start and finish so a double-load shows as two overlapping start/finish pairs for one play.
|
||||
@@ -330,10 +357,17 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
|
||||
IsLoading = false;
|
||||
_logger.LogInformation("Streaming load #{Gen} finished for track {TrackId} (superseded={Superseded})",
|
||||
loadGeneration, track.EntryKey, !ReferenceEquals(_streamingCancellation, loadCts));
|
||||
// Only notify if this load is still the active operation. A superseding seek
|
||||
// owns state notifications; firing here mid-seek would push a stale snapshot.
|
||||
// Release the single-load guard only when this load is still the active operation. A
|
||||
// superseding load for a DIFFERENT track has already overwritten _loadInFlightTrackId with
|
||||
// its own key before its first await; clearing it here would unlatch the guard mid-way
|
||||
// through that newer load and let a duplicate of IT slip through. The CTS identity is the
|
||||
// same "am I still the active operation?" test the notify/state guards use. ResetToIdle
|
||||
// (the supersede path) does not touch this field — the incoming load owns its lifecycle.
|
||||
if (ReferenceEquals(_streamingCancellation, loadCts))
|
||||
{
|
||||
_loadInFlightTrackId = null;
|
||||
// Only notify if this load is still the active operation. A superseding seek
|
||||
// owns state notifications; firing here mid-seek would push a stale snapshot.
|
||||
await NotifyStateChanged();
|
||||
}
|
||||
}
|
||||
@@ -935,9 +969,15 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Single method to reset all state - called by both Stop and Unload.
|
||||
/// Single method to reset all state - called by both Stop and Unload, and as the prologue of a new
|
||||
/// load. <paramref name="clearLoadGuard"/> is true for the direct stop/unload/dispose callers and
|
||||
/// false when a fresh <see cref="LoadTrackStreaming"/> calls it: that load has already set
|
||||
/// <c>_loadInFlightTrackId</c> to its own key to arm the single-load guard, so the prologue must not
|
||||
/// wipe it. The direct callers DO clear it so a later replay of the same track is not wrongly
|
||||
/// suppressed by a guard key left over from an interrupted in-flight load (whose CTS-identity check
|
||||
/// in finally fails after ResetToIdle nulls the CTS, so the load itself never clears the field).
|
||||
/// </summary>
|
||||
private async Task ResetToIdle()
|
||||
private async Task ResetToIdle(bool clearLoadGuard = true)
|
||||
{
|
||||
// 0. Close any open play session BEFORE tearing down (§2.1). ResetToIdle is the single funnel
|
||||
// for stop / unload / dispose / track-switch (a new LoadTrackStreaming calls it first), so a
|
||||
@@ -987,6 +1027,12 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
|
||||
_currentTrackId = null;
|
||||
_currentFormat = AudioFormat.Lossless;
|
||||
|
||||
// Direct stop/unload/dispose: release the single-load guard so a later replay of the same track
|
||||
// is not suppressed. NOT cleared on the load prologue (clearLoadGuard:false) — that load owns the
|
||||
// key it just armed.
|
||||
if (clearLoadGuard)
|
||||
_loadInFlightTrackId = null;
|
||||
|
||||
await NotifyStateChanged();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user