diff --git a/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs b/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs index 5f38290..b90c010 100644 --- a/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs +++ b/DeepDrftPublic.Client/Services/StreamingAudioPlayerService.cs @@ -759,36 +759,7 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS // segment alone clears the threshold). if (!_streamingPlaybackStarted && CanStartStreaming) { - var playbackResult = await _audioInterop.StartStreamingPlayback(PlayerId); - if (playbackResult.Success) - { - _streamingPlaybackStarted = true; - IsPlaying = true; - IsPaused = false; - IsLoaded = true; // loaded and ready, even while still downloading - ErrorMessage = null; - - // Open the play session exactly once per load, at the moment playback - // truly begins (§2.1). The _sessionOpened guard keeps a seek/refill - // re-stream — which re-enters this transition with - // _streamingPlaybackStarted reset — from opening a second session for - // the same play. Duration may already be known, so re-feed it. - if (!_sessionOpened && _currentTrackId is { } trackKey) - { - _sessionOpened = true; - _playTracker?.OnPlaybackStarted(trackKey); - if (Duration is { } d) - _playTracker?.SetDuration(d); - } - - await NotifyStateChanged(); // immediate — critical state change - } - else - { - var technicalError = $"Failed to start streaming playback: {playbackResult.Error}"; - _logger.LogError("Failed to start playback: {Error}", technicalError); - ErrorMessage = StreamingErrorHandler.GetUserFriendlyMessage(technicalError); - } + await TryStartPlaybackAsync(); } // Progress against the total file length (cursor + bytes consumed so far). @@ -888,6 +859,17 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS // residual tail and covers the (rare) case where the total was unknown. await _audioInterop.MarkStreamCompleteAsync(PlayerId); + // Complete-without-start fallback: if the track's total decodable audio never crossed the + // start threshold (e.g. total Opus audio < 1s lead, or WAV < 6 buffers), the in-loop + // CanStartStreaming check never fired and _streamingPlaybackStarted is still false. Now that + // streamComplete is set on the JS scheduler, calling StartStreamingPlayback lets it drain + // the accumulated buffers and fires onPlaybackEnded exactly once — same transition the + // normal path uses, so session/_sessionOpened/Duration handling is identical. + if (!_streamingPlaybackStarted) + { + await TryStartPlaybackAsync(); + } + LoadProgress = 1.0; await NotifyStateChanged(); } @@ -917,6 +899,46 @@ public class StreamingAudioPlayerService : AudioPlayerService, IStreamingPlayerS } } + /// + /// Call StartStreamingPlayback on the JS player and apply the resulting state transitions. + /// This is the single playback-start transition shared by the in-loop threshold path and the + /// completion-path fallback — both callers set the guard and apply session/Duration handling + /// identically so neither path diverges. + /// + private async Task TryStartPlaybackAsync() + { + var playbackResult = await _audioInterop.StartStreamingPlayback(PlayerId); + if (playbackResult.Success) + { + _streamingPlaybackStarted = true; + IsPlaying = true; + IsPaused = false; + IsLoaded = true; // loaded and ready, even while still downloading + ErrorMessage = null; + + // Open the play session exactly once per load, at the moment playback + // truly begins (§2.1). The _sessionOpened guard keeps a seek/refill + // re-stream — which re-enters this transition with + // _streamingPlaybackStarted reset — from opening a second session for + // the same play. Duration may already be known, so re-feed it. + if (!_sessionOpened && _currentTrackId is { } trackKey) + { + _sessionOpened = true; + _playTracker?.OnPlaybackStarted(trackKey); + if (Duration is { } d) + _playTracker?.SetDuration(d); + } + + await NotifyStateChanged(); // immediate — critical state change + } + else + { + var technicalError = $"Failed to start streaming playback: {playbackResult.Error}"; + _logger.LogError("Failed to start playback: {Error}", technicalError); + ErrorMessage = StreamingErrorHandler.GetUserFriendlyMessage(technicalError); + } + } + /// /// In streaming mode, Stop fully resets to Idle state since audio data is consumed. /// This is equivalent to Unload for streaming playback. diff --git a/DeepDrftPublic/Interop/audio/AudioPlayer.ts b/DeepDrftPublic/Interop/audio/AudioPlayer.ts index dfc12dd..91d025f 100644 --- a/DeepDrftPublic/Interop/audio/AudioPlayer.ts +++ b/DeepDrftPublic/Interop/audio/AudioPlayer.ts @@ -285,10 +285,15 @@ export class AudioPlayer { return { success: false, error: 'Opus decode failed' }; } - // "headerParsed" maps to the decoder being configured (codec ready). canStart needs the - // min buffer count, exactly as the WAV path requires before first playback. + // "headerParsed" maps to the decoder being configured (codec ready). canStart needs a + // healthy decoded lead before first playback — measured in SECONDS, not a buffer count. + // An Opus WebCodecs packet is ~20 ms, so the WAV-tuned 6-BUFFER minimum is only ~0.12 s of + // lead: playback would start, drain it before the async decode ramps, and underrun + // immediately. The seconds-based lead gate (same threshold the scheduler's underrun-resume + // hysteresis uses) gives Opus the cushion its decode ramp needs. WAV keeps the buffer-count + // gate below — its large synchronous segments rarely underrun and its start must not change. const headerParsed = decoder.ready; - const canStart = headerParsed && this.scheduler.hasMinimumBuffers(this.minBuffersForPlayback); + const canStart = headerParsed && this.scheduler.hasMinimumPlaybackLead(); return { success: true, diff --git a/DeepDrftPublic/Interop/audio/PlaybackScheduler.test.ts b/DeepDrftPublic/Interop/audio/PlaybackScheduler.test.ts index e3734e0..4f92de1 100644 --- a/DeepDrftPublic/Interop/audio/PlaybackScheduler.test.ts +++ b/DeepDrftPublic/Interop/audio/PlaybackScheduler.test.ts @@ -495,8 +495,8 @@ test('underrun resumes when new buffers arrive', () => { drainAllSources(s, cm); // underrun assertEqual(s.isActive(), false, 'inactive after underrun'); - // Decode catches up: more buffers arrive and the producer schedules them. - for (let i = 0; i < 3; i++) s.addBuffer(buf(0.3)); + // Decode catches up: enough buffers arrive to clear the 1s rebuffer lead (4 × 0.3 = 1.2s). + for (let i = 0; i < 4; i++) s.addBuffer(buf(0.3)); s.scheduleNewBuffers(); assertEqual(s.isActive(), true, 'resumed active after refill'); @@ -507,6 +507,78 @@ test('underrun resumes when new buffers arrive', () => { } }); +// === Rebuffer hysteresis (Opus-startup thrash fix) =========================================== +// +// After a mid-stream underrun the scheduler must NOT resume on the first arriving buffer (which, +// for ~20 ms Opus packets, plays one buffer, drains, and re-parks — the audible start/stop thrash). +// It re-accumulates a healthy decoded LEAD (DEFAULT_MIN_PLAYBACK_LEAD_SECONDS = 1s) first. The +// streamComplete override is the escape hatch so a genuine short tail still plays out, never parking +// forever. These drive the real handleSourceEnded/scheduleNewBuffers/setStreamComplete paths. + +// Below the rebuffer lead: a thin refill must keep the scheduler parked (no resume, no false end); +// once the accumulated lead crosses the threshold, it resumes. +test('underrun does not resume below the rebuffer lead, resumes once it is met', () => { + const cm = new FakeContextManager(); + const s = makeScheduler(cm); + let ended = 0; + s.onPlaybackEnded = () => { ended++; }; + + for (let i = 0; i < 3; i++) s.addBuffer(buf(0.3)); + cm.now = 0; + s.playFromPosition(0); + cm.now = 1.0; + drainAllSources(s, cm); // underrun + assertEqual(s.isActive(), false, 'parked in underrun'); + + // Only 0.6s of fresh lead arrives — below the 1s rebuffer threshold. Must stay parked. + for (let i = 0; i < 2; i++) s.addBuffer(buf(0.3)); + s.scheduleNewBuffers(); + assertEqual(s.isActive(), false, 'still parked — lead below the rebuffer threshold'); + assertEqual(ended, 0, 'no false end while re-accumulating lead'); + const priv = s as unknown as { scheduledSources: unknown[] }; + assertEqual(priv.scheduledSources.length, 0, 'nothing scheduled below the threshold'); + + // More lead arrives, crossing the threshold (0.6 + 0.6 = 1.2s ≥ 1s) → now resume. + for (let i = 0; i < 2; i++) s.addBuffer(buf(0.3)); + s.scheduleNewBuffers(); + assertEqual(s.isActive(), true, 'resumes once the lead crosses the threshold'); + assertEqual(ended, 0, 'still no false end after resume'); +}); + +// Genuine-end tail SHORTER than the rebuffer lead: while parked, a small tail arrives AND the stream +// completes. The threshold is overridden so the tail plays out and the genuine end fires exactly +// once — the scheduler must never park forever waiting for a lead that will never come. +test('streamComplete tail below the rebuffer lead still plays out and fires end once', () => { + const cm = new FakeContextManager(); + const s = makeScheduler(cm); + let ended = 0; + s.onPlaybackEnded = () => { ended++; }; + + for (let i = 0; i < 3; i++) s.addBuffer(buf(0.3)); + cm.now = 0; + s.playFromPosition(0); + cm.now = 1.0; + drainAllSources(s, cm); // underrun + assertEqual(s.isActive(), false, 'parked in underrun'); + + // A short final tail (0.6s, below the 1s threshold) arrives; the hysteresis keeps it parked. + for (let i = 0; i < 2; i++) s.addBuffer(buf(0.3)); + s.scheduleNewBuffers(); + assertEqual(s.isActive(), false, 'parked — tail below threshold, stream not yet complete'); + assertEqual(ended, 0, 'no end before completion'); + + // The stream completes: the threshold no longer applies → the tail schedules and plays out. + s.setStreamComplete(true); + assertEqual(s.isActive(), true, 'resumed to play out the final tail on completion'); + assertEqual(ended, 0, 'end not fired until the tail drains'); + + // Drain the tail → genuine end fires exactly once. + cm.now = 2.0; + drainAllSources(s, cm); + assertEqual(ended, 1, 'genuine end fires exactly once after the tail drains'); + assertEqual(s.isActive(), false, 'inactive after genuine end'); +}); + // GENUINE end: stream complete AND queue drains → onPlaybackEnded fires exactly once. test('genuine end (streamComplete + drained) fires onPlaybackEnded exactly once', () => { const cm = new FakeContextManager(); @@ -617,8 +689,8 @@ test('underrun → resume → genuine end fires exactly once', () => { assertEqual(s.isActive(), false, 'underrun after initial drain'); assertEqual(ended, 0, 'no end count during underrun'); - // Decode catches up: new buffers arrive and the scheduler resumes. - for (let i = 0; i < 3; i++) s.addBuffer(buf(0.3)); + // Decode catches up: enough buffers arrive to clear the 1s rebuffer lead (4 × 0.3 = 1.2s). + for (let i = 0; i < 4; i++) s.addBuffer(buf(0.3)); s.scheduleNewBuffers(); assertEqual(s.isActive(), true, 'resumed active after refill'); assertEqual(ended, 0, 'still no end after resume'); @@ -632,6 +704,76 @@ test('underrun → resume → genuine end fires exactly once', () => { assertEqual(s.isActive(), false, 'inactive after genuine end'); }); +// === Complete-without-start (force-start fallback) ========================================== +// +// The C# producer calls StartStreamingPlayback after MarkStreamCompleteAsync when +// _streamingPlaybackStarted is still false (total audio below the start threshold). The JS-side +// effect is playFromPosition(0) called with streamComplete already true. This section covers the +// scheduler-side guarantee: sub-threshold buffers + streamComplete already set + forced +// playFromPosition drains and fires end exactly once, never zero, never twice. +// +// The C# transition itself is not exercisable here (requires StreamingAudioPlayerService + +// AudioInteropService), so the test covers the scheduler drain-and-end-once contract directly. + +// Forced start after completion: sub-threshold total audio, streamComplete set BEFORE +// playFromPosition(0), sources drain and onPlaybackEnded fires exactly once. +test('forced start on complete stream: sub-threshold buffers drain and fire end exactly once', () => { + const cm = new FakeContextManager(); + const s = makeScheduler(cm); + let ended = 0; + s.onPlaybackEnded = () => { ended++; }; + + // Sub-threshold buffers (0.4s total, below the 1s rebuffer lead). Never started. + for (let i = 0; i < 2; i++) s.addBuffer(buf(0.2)); + + // Stream marks complete BEFORE playback starts — the C# completion-path ordering: + // MarkStreamCompleteAsync fires first, then StartStreamingPlayback is called because + // _streamingPlaybackStarted is false. setStreamComplete with underrun_=false returns + // early (sets the flag but does not schedule/finalize — that is correct, nothing to drain yet). + s.setStreamComplete(true); + assertEqual(ended, 0, 'no end fired at setStreamComplete — playback not yet started'); + assertEqual(s.isActive(), false, 'scheduler inactive before forced start'); + + // Forced start: C# calls startStreamingPlayback() → playFromPosition(0). + // With streamComplete already true and buffers present, this schedules all buffers. + cm.now = 0; + s.playFromPosition(0); + + const priv = s as unknown as { scheduledSources: unknown[] }; + if (priv.scheduledSources.length === 0) { + throw new Error('expected sources scheduled after forced playFromPosition'); + } + assertEqual(ended, 0, 'end not fired yet — sources must drain first'); + assertEqual(s.isActive(), true, 'scheduler active while sources are scheduled'); + + // Drain sources → streamComplete is true → genuine end fires exactly once. + cm.now = 0.5; + drainAllSources(s, cm); + + assertEqual(ended, 1, 'end fires exactly once after forced-start drain'); + assertEqual(s.isActive(), false, 'scheduler inactive after genuine end'); +}); + +// No double-fire: calling setStreamComplete again after end has already fired is a no-op. +test('setStreamComplete after forced-start drain is a no-op (no double end)', () => { + const cm = new FakeContextManager(); + const s = makeScheduler(cm); + let ended = 0; + s.onPlaybackEnded = () => { ended++; }; + + for (let i = 0; i < 2; i++) s.addBuffer(buf(0.2)); + s.setStreamComplete(true); + cm.now = 0; + s.playFromPosition(0); + cm.now = 0.5; + drainAllSources(s, cm); + assertEqual(ended, 1, 'end fired once after forced-start drain'); + + // A redundant setStreamComplete (e.g. called again from a stale C# path) must not re-fire. + s.setStreamComplete(true); + assertEqual(ended, 1, 'still exactly one end after redundant setStreamComplete'); +}); + // --- run ------------------------------------------------------------------------------------- if (failures.length > 0) { console.error(failures.join('\n')); diff --git a/DeepDrftPublic/Interop/audio/PlaybackScheduler.ts b/DeepDrftPublic/Interop/audio/PlaybackScheduler.ts index 68030b9..c3a25c4 100644 --- a/DeepDrftPublic/Interop/audio/PlaybackScheduler.ts +++ b/DeepDrftPublic/Interop/audio/PlaybackScheduler.ts @@ -63,6 +63,21 @@ const DEFAULT_FORWARD_LOW_WATER_SECONDS = 15; const DEFAULT_MAX_DECODED_BYTES = 96 * 1024 * 1024; // ~96 MB of decoded float PCM const BYTES_PER_FLOAT_SAMPLE = 4; +/** + * Rebuffer hysteresis lead — the minimum SECONDS of decoded-but-unscheduled audio that must + * accumulate ahead of the schedule cursor before playback may (re)start after a mid-stream underrun. + * + * Why seconds, not a buffer count: the per-buffer duration differs wildly by format. A WAV/lossless + * segment is a sizeable slab (~0.1–0.4 s); a single Opus WebCodecs packet is ~20 ms. The old resume + * path re-anchored on the FIRST arriving buffer, so for Opus it scheduled ~20 ms, drained it, parked, + * resumed on the next ~20 ms, and so on — the audible start/stop thrash during the WebCodecs decode + * ramp. Gating on a fixed LEAD in seconds gives a resume the same cushion a fresh start has, + * independent of format. 1 s is the same order as the lossless playback-start lead (~6 segments) and + * sits far below the 30 s forward high-water, so back-pressure never throttles production while the + * scheduler is still re-accumulating this lead. Tunable; not magic. + */ +const DEFAULT_MIN_PLAYBACK_LEAD_SECONDS = 1.0; + interface ScheduledSource { source: AudioBufferSourceNode; bufferIndex: number; @@ -100,6 +115,12 @@ export class PlaybackScheduler { private forwardLowWaterSeconds: number = DEFAULT_FORWARD_LOW_WATER_SECONDS; private maxDecodedBytes: number = DEFAULT_MAX_DECODED_BYTES; + // Rebuffer hysteresis lead (seconds). The minimum decoded-but-unscheduled audio that must sit + // ahead of the schedule cursor before playback may (re)start — at a fresh start AND after a + // mid-stream underrun. Without it the underrun resume re-anchored on the first arriving buffer + // and thrashed on the Opus decode ramp. See DEFAULT_MIN_PLAYBACK_LEAD_SECONDS. + private minPlaybackLeadSeconds: number = DEFAULT_MIN_PLAYBACK_LEAD_SECONDS; + // Hysteresis latch for the production pause. Once forward fill crosses the high-water mark we // stay paused until it drains below the low-water mark, so the two producers do not flap // on/off around a single threshold (and a paused producer does not resume for one chunk only @@ -148,13 +169,23 @@ export class PlaybackScheduler { */ setStreamComplete(complete: boolean): void { this.streamComplete = complete; - // If the queue already drained mid-stream (we are parked in underrun) when the genuine-end - // signal arrives, finalise now — the tail produced no more buffers, so this drained state is - // the real end. Gated on underrun_ (logically-playing-but-starved), not isActive_, which is - // false during a parked underrun. A drained queue with no playback in flight (never started, - // or already finished) is left untouched. - if (complete && this.underrun_ && - this.scheduledSources.length === 0 && this.nextBufferIndex >= this.buffers.length) { + // Only act when the genuine-end signal lands while we are parked in underrun (logically + // playing but starved); a drained queue with no playback in flight — never started, or + // already finished — is left untouched. Gated on underrun_, not isActive_, which is false + // during a parked underrun. + if (!complete || !this.underrun_) { + return; + } + // The rebuffer threshold no longer applies — a complete stream yields no further buffers: + // - tail buffers accumulated below the threshold while we were parked (the new hysteresis + // kept us parked) → schedule them out; scheduleNewBuffers' underrun branch now resumes + // because streamComplete overrides the lead gate, and handleSourceEnded fires the genuine + // end when they drain. Without this the buffers would never schedule and we would park + // forever (queue drained, isActive_ false, threshold never met). + // - no tail at all (cursor already at the decoded end) → this drained state IS the end. + if (this.nextBufferIndex < this.buffers.length) { + this.scheduleNewBuffers(); + } else if (this.scheduledSources.length === 0) { this.finishPlayback(); } } @@ -432,6 +463,19 @@ export class PlaybackScheduler { // captured before the gap) so the resumed audio is contiguous from "now" — a stale anchor // would schedule the next source in the past and the browser would drop or rush it. if (this.underrun_) { + // Rebuffer hysteresis: do NOT resume on the first arriving buffer. With an empty scheduled + // tail, resuming on a single buffer plays it (~20 ms for Opus) and immediately re-drains, + // re-parking — the audible start/stop thrash on the Opus WebCodecs decode ramp. Stay parked + // and keep accumulating until a healthy lead has rebuilt, so the resumed playback has the + // same cushion a fresh start does. While parked the playhead is frozen, so each arriving + // buffer grows the lead monotonically toward the threshold (no starvation/deadlock). + // + // streamComplete overrides the gate: a finished stream produces no further buffers, so a + // tail shorter than the lead MUST still play out (here and via setStreamComplete) rather + // than park forever. handleSourceEnded fires the genuine end once that tail drains. + if (!this.streamComplete && !this.hasMinimumPlaybackLead()) { + return; // still re-accumulating the rebuffer lead — remain parked + } this.underrun_ = false; this.isActive_ = true; this.playbackAnchorTime = this.contextManager.currentTime; @@ -663,6 +707,24 @@ export class PlaybackScheduler { return this.buffers.length >= minCount; } + /** + * True once at least `minPlaybackLeadSeconds` of decoded-but-unscheduled audio sits ahead of the + * schedule cursor — the rebuffer-hysteresis gate for both a fresh playback start (cursor at 0, so + * this measures the whole decoded head) and an underrun resume (cursor at the drained tail, so this + * measures only the freshly-accumulated lead). Sums only up to the threshold and short-circuits, so + * it is bounded (~one threshold's worth of buffers) regardless of how much is buffered ahead. + */ + hasMinimumPlaybackLead(): boolean { + let lead = 0; + for (let i = this.nextBufferIndex; i < this.buffers.length; i++) { + lead += this.buffers[i].duration; + if (lead >= this.minPlaybackLeadSeconds) { + return true; + } + } + return false; + } + /** * Check if playback is active */