diff --git a/DeepDrftPublic.Client/Services/AudioPlayerService.cs b/DeepDrftPublic.Client/Services/AudioPlayerService.cs index 778cc1d..5221e3c 100644 --- a/DeepDrftPublic.Client/Services/AudioPlayerService.cs +++ b/DeepDrftPublic.Client/Services/AudioPlayerService.cs @@ -118,6 +118,16 @@ public abstract class AudioPlayerService : IPlayerService, IAsyncDisposable IsPlaying = true; IsPaused = false; } + else if (IsPaused) + { + // Play failed while the player is paused — the scheduler may be empty after a + // failed refill (AC6 recovery). Re-issue a seek at the current position: the + // seek path routes to seekBeyondBuffer when the scheduler is empty (Phase 21.3 + // fix), triggering a real refetch rather than returning "Streaming not ready". + // We return early here; Seek owns its own state mutations and NotifyStateChanged. + await Seek(CurrentTime); + return; + } } if (!result.Success) @@ -128,7 +138,7 @@ public abstract class AudioPlayerService : IPlayerService, IAsyncDisposable { ErrorMessage = null; } - + await NotifyStateChanged(); } catch (Exception ex) diff --git a/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs b/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs index 3f1dfca..d3b6439 100644 --- a/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs +++ b/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs @@ -629,6 +629,15 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS await DrainActiveStreamingTaskAsync(); oldCts?.Dispose(); + // Single-writer discipline (C6/AC8): all three failure exits must share the same guard. + // TrackMediaClient.GetTrackMedia swallows OperationCanceledException and returns + // Success==false, so a superseded seek lands in the media-fetch-fail branch below + // rather than in the OCE catch. Without the guard those branches would call + // RecoverFromFailedRefill — running clearForSeek + setPlaybackOffset against the player + // state the NEWER seek now owns. A local predicate keeps all three exits symmetric so a + // future exit cannot forget the check. + bool IsStillActiveSeek() => ReferenceEquals(_streamingCancellation, seekCts); + try { // Update UI immediately @@ -649,7 +658,15 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS { var technicalError = mediaResult.GetMessage() ?? "Failed to load audio from position"; _logger.LogError("Failed to get track media from offset {Offset}: {Error}", byteOffset, technicalError); - await RecoverFromFailedRefill(seekPosition, StreamingErrorHandler.GetUserFriendlyMessage(technicalError)); + // Guard: a superseded seek must NOT touch shared state. The newer seek owns teardown. + if (IsStillActiveSeek()) + { + await RecoverFromFailedRefill(seekPosition, StreamingErrorHandler.GetUserFriendlyMessage(technicalError)); + } + else + { + _logger.LogDebug("Media-fetch failed on superseded seek to {Position} — newer seek owns state, skipping recovery", seekPosition); + } return; } @@ -660,7 +677,15 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS if (!reinitResult.Success) { _logger.LogError("Failed to reinitialize for offset streaming: {Error}", reinitResult.Error); - await RecoverFromFailedRefill(seekPosition, "Failed to seek to position"); + // Guard: same single-writer discipline — only recover when we are still the active seek. + if (IsStillActiveSeek()) + { + await RecoverFromFailedRefill(seekPosition, "Failed to seek to position"); + } + else + { + _logger.LogDebug("Reinit failed on superseded seek to {Position} — newer seek owns state, skipping recovery", seekPosition); + } return; } @@ -682,7 +707,7 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS // still the active seek — if _streamingCancellation has been replaced, a // newer seek is in progress and owns the flag. _logger.LogDebug("Seek beyond buffer cancelled"); - if (ReferenceEquals(_streamingCancellation, seekCts)) + if (IsStillActiveSeek()) { IsSeekingBeyondBuffer = false; } @@ -694,7 +719,7 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS // fire a silent false end. Only when we are still the active seek — a superseding seek owns // the state and the OCE catch above handles its own teardown. _logger.LogError(ex, "Error during seek beyond buffer to position {Position}", seekPosition); - if (ReferenceEquals(_streamingCancellation, seekCts)) + if (IsStillActiveSeek()) { await RecoverFromFailedRefill(seekPosition, StreamingErrorHandler.GetUserFriendlyMessage(ex.Message)); } diff --git a/DeepDrftPublic/Interop/audio/AudioPlayer.test.ts b/DeepDrftPublic/Interop/audio/AudioPlayer.test.ts index f9931e3..1bf4d15 100644 --- a/DeepDrftPublic/Interop/audio/AudioPlayer.test.ts +++ b/DeepDrftPublic/Interop/audio/AudioPlayer.test.ts @@ -65,18 +65,26 @@ function assertTrue(cond: boolean, msg?: string): void { class FakeScheduler { private offset: number; private total: number; + // hasBuffers reflects whether the scheduler holds decoded audio. Starts true when total > 0 + // (a populated window), set to false by clearForSeek() (recovery drains the buffers). + private _hasBuffers: boolean; public clearedForSeek = false; public stoppedAllSources = false; public offsetSetTo: number | null = null; - constructor(offset: number, total: number) { this.offset = offset; this.total = total; } + constructor(offset: number, total: number) { + this.offset = offset; + this.total = total; + this._hasBuffers = total > 0; + } getPlaybackOffset(): number { return this.offset; } getTotalDuration(): number { return this.total; } + hasBuffers(): boolean { return this._hasBuffers; } stopAllSources(): void { this.stoppedAllSources = true; } // seekWithinBuffer calls playFromPosition only when wasPlaying; isPlaying is false in these // unit constructions, so it is never invoked — present for completeness. playFromPosition(_position: number): void { /* no-op */ } - clearForSeek(): void { this.clearedForSeek = true; } + clearForSeek(): void { this.clearedForSeek = true; this._hasBuffers = false; } setPlaybackOffset(o: number): void { this.offset = o; this.offsetSetTo = o; } } @@ -243,6 +251,43 @@ test('recoverFromFailedRefill halts the scheduler and leaves a paused-but-loaded assertEqual(priv(player).streamingStarted, false, 'streaming flagged not-started for a clean retry'); }); +// --- AC6 retry contract: same-target seek after recovery refetches ------------------------- + +// After recoverFromFailedRefill the scheduler is empty (clearForSeek was called). A seek to +// the SAME position (seekPosition == playbackOffset) must route to seekBeyondBuffer — not +// seekWithinBuffer, which would be a silent no-op against the degenerate [P,P] empty window. +test('same-target seek after recovery routes to seekBeyondBuffer (AC6 retry)', () => { + const player = makePlayer(); + const wav = new FakeStreamDecoder(true, 1000); + // Start with a populated window [30, 60), then simulate recovery at position 15: + // clearForSeek empties the scheduler; setPlaybackOffset anchors it to 15. + const scheduler = new FakeScheduler(30, 30); + arm(player, { scheduler, duration: 120, streamDecoder: wav }); + // Drive recovery state manually (the same state recoverFromFailedRefill leaves). + player.recoverFromFailedRefill(15); + // At this point: scheduler.hasBuffers() == false, playbackOffset == 15, totalDuration == 0. + // A seek to 15 (the recovery anchor) must refetch, not silently resolve from the empty window. + const result = player.seek(15); + assertEqual(result.success, true, 'seek succeeds after recovery'); + assertEqual(result.seekBeyondBuffer, true, 'same-target seek after recovery signals a refetch (AC6 retry)'); + assertEqual(wav.requestedOffsetFor, 15, 'WAV resolver used for the retry offset'); +}); + +// AC4 not regressed: a seek within a POPULATED retained window still resolves from buffer. +// This is the same test as the existing AC4 test but named explicitly to confirm the +// hasBuffers() guard does not affect the populated case. +test('seek within populated retained window still resolves in-buffer — AC4 not regressed', () => { + const player = makePlayer(); + // Populated window [30, 60) — hasBuffers() starts true (total=30 > 0). + const scheduler = new FakeScheduler(30, 30); + arm(player, { scheduler, duration: 120, streamDecoder: new FakeStreamDecoder(true, 1000) }); + + const result = player.seek(45); // inside [30, 60) + assertEqual(result.success, true, 'seek succeeds'); + assertEqual(result.seekBeyondBuffer ?? false, false, 'populated in-window seek does NOT signal a refetch'); + assertEqual(scheduler.clearedForSeek, false, 'scheduler not cleared for an in-buffer seek (no refetch)'); +}); + // --- run ------------------------------------------------------------------------------------- if (failures.length > 0) { console.error(failures.join('\n')); diff --git a/DeepDrftPublic/Interop/audio/AudioPlayer.ts b/DeepDrftPublic/Interop/audio/AudioPlayer.ts index 33ead74..d33572f 100644 --- a/DeepDrftPublic/Interop/audio/AudioPlayer.ts +++ b/DeepDrftPublic/Interop/audio/AudioPlayer.ts @@ -440,21 +440,28 @@ export class AudioPlayer { const bufferEnd = this.scheduler.getTotalDuration() + bufferStart; // The window-miss test for BOTH directions, and the 21.3 refill trigger for backward seeks. - // Position must be within [bufferStart, bufferEnd] to resolve from the retained buffers: - // - position >= bufferStart : UC3 — seek back within the retained back-window. Served from - // buffer with NO network refetch. (The lower bound is load-bearing: after eviction or a - // prior seek-beyond-buffer, bufferStart > 0, and a target below it would otherwise produce - // a negative bufferRelativePosition in seekWithinBuffer, silently clamping to position 0.) + // Position must be within [bufferStart, bufferEnd] AND the scheduler must hold buffers to + // resolve from the retained window: + // - position >= bufferStart AND hasBuffers : UC3 — seek back within the retained back-window. + // Served from buffer with NO network refetch. (The lower bound is load-bearing: after + // eviction or a prior seek-beyond-buffer, bufferStart > 0, and a target below it would + // otherwise produce a negative bufferRelativePosition in seekWithinBuffer, silently clamping + // to position 0.) // - position < bufferStart : UC4 — seek back PAST the retained tail (the window was evicted). // Falls through to seekBeyondBuffer, which is the existing Range path run toward an EARLIER // offset. This is the 21.3 window-miss refill: "a seek the listener didn't initiate" reuses // the same per-path resolver + reinit a forward seek-beyond-buffer uses, no new mechanism. // - position > bufferEnd : UC2/UC5 — forward seek beyond buffer, unchanged. - if (position >= bufferStart && position <= bufferEnd) { + // - !hasBuffers (degenerate [P,P] window post-recovery): the window check above would + // spuriously route ANY target to seekWithinBuffer (bufferStart==bufferEnd==seekPosition + // after recoverFromFailedRefill). Force seekBeyondBuffer so a same-target retry actually + // refetches (AC6 retry contract). The !hasBuffers guard only fires in the degenerate case — + // a populated retained window has buffers and is unaffected (AC4 not regressed). + if (position >= bufferStart && position <= bufferEnd && this.scheduler.hasBuffers()) { return this.seekWithinBuffer(position); } else { - // Seeking outside the retained window - signal C# to fetch a new stream from the resolved - // offset (earlier for a back-past-window refill, later for a forward seek). + // Seeking outside the retained window, or to any position in an empty scheduler — + // signal C# to fetch a new stream from the resolved offset. return this.seekBeyondBuffer(position); } }