Merge Opus single-load guard (instrument double-load) into streaming-overhaul

This commit is contained in:
daniel-c-harvey
2026-06-24 23:09:08 -04:00
@@ -64,6 +64,16 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
private readonly ILogger<StreamingAudioPlayerService> _logger; private readonly ILogger<StreamingAudioPlayerService> _logger;
private string? _currentTrackId; 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 // 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 // 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 // logged twice" double-load hypothesis that needs in-browser confirmation — are unmistakable: a
@@ -199,12 +209,29 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
private async Task LoadTrackStreaming(TrackDto track, double? startPosition = null) private async Task LoadTrackStreaming(TrackDto track, double? startPosition = null)
{ {
// 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 // Always reset to clean state before loading new track. ResetToIdle
// both cancels and awaits any in-flight streaming loop, so by the time // 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 // we return from it the previous loop is guaranteed to have exited and
// there is no risk of interleaved ProcessStreamingChunk calls against // there is no risk of interleaved ProcessStreamingChunk calls against
// the single-instance JS StreamDecoder. // the single-instance JS StreamDecoder. clearLoadGuard:false — we just armed
await ResetToIdle(); // _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 // 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. // start and finish so a double-load shows as two overlapping start/finish pairs for one play.
@@ -376,10 +403,17 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
IsLoading = false; IsLoading = false;
_logger.LogInformation("Streaming load #{Gen} finished for track {TrackId} (superseded={Superseded})", _logger.LogInformation("Streaming load #{Gen} finished for track {TrackId} (superseded={Superseded})",
loadGeneration, track.EntryKey, !ReferenceEquals(_streamingCancellation, loadCts)); loadGeneration, track.EntryKey, !ReferenceEquals(_streamingCancellation, loadCts));
// Only notify if this load is still the active operation. A superseding seek // Release the single-load guard only when this load is still the active operation. A
// owns state notifications; firing here mid-seek would push a stale snapshot. // 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)) 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(); await NotifyStateChanged();
} }
} }
@@ -1109,9 +1143,15 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
} }
/// <summary> /// <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> /// </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 // 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 // for stop / unload / dispose / track-switch (a new LoadTrackStreaming calls it first), so a
@@ -1161,6 +1201,12 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS
_currentTrackId = null; _currentTrackId = null;
_currentFormat = AudioFormat.Lossless; _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(); await NotifyStateChanged();
} }