Merge Opus rebuffer hysteresis + ultra-short-track start fix into streaming-overhaul

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
         }
     }
 
+    /// <summary>
+    /// Call <c>StartStreamingPlayback</c> 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.
+    /// </summary>
+    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);
+        }
+    }
+
     /// <summary>
     /// In streaming mode, Stop fully resets to Idle state since audio data is consumed.
     /// This is equivalent to Unload for streaming playback.

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,

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'));

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
      */