Fix loud PSP loop crackle + make Dolphin loop natively in hardware

Root cause of the crackle (confirmed on real PSP hardware): the
persistent feeder thread called stream->onLoop directly in the middle
of its tight chunk-feeding loop. onLoop can do arbitrary work (the
test callback does consolePrint, which locks a mutex, moves the
console history buffer, and fflushes stdout) - if that takes anywhere
close to one chunk's playback time (~23ms), the next chunk isn't
ready and the hardware channel starves. Audio callbacks must never be
invoked directly from a real-time audio thread.

Fixed generically: audiostream_t gains loopCount (incremented by
platform code from whatever context it runs in) and lastLoopCount
(main-thread-only bookkeeping). audioStreamUpdate() detects the
change and fires onLoop safely from the main thread, regardless of
which thread/interrupt actually noticed the loop. PSP's feeder thread
now increments the counter instead of calling onLoop inline.

Also eliminated the ~21.7ms of real silence padding baked into every
PSP loop pass (found while chasing the timing gap that preceded the
crackle fix): the final chunk's padding is now filled with the start
of the next loop instead of zero, avoiding sceAudioSetChannelDataLen
(the likely real cause of an earlier, separate click) while keeping
every output call the same constant size. Loop period measured via
PPSSPP is now ~1.000-1.002s for a 1.000s tone, down from a consistent
~1.02-1.03s before.

The PSP feeder thread is also now persistent for the stream's whole
lifetime (created once in Init, idles between plays) rather than
respawned via threadStartRequest on every single loop restart - real,
avoidable OS thread creation overhead that was contributing to the
gap before the padding was identified as the dominant cause.

Brought Dolphin in line architecturally rather than mirroring PSP/
Linux's restart-and-detect approach: ansnd_pcm_voice_config_t has
native loop_start_offset/loop_end_offset fields, so a looping Dolphin
voice loops entirely in DSP hardware with zero host involvement at
the loop boundary - no restart latency to create a gap in the first
place. Trade-off, clearly documented in code: onLoop never fires for
Dolphin this way (no ANSND_VOICE_STATE for "wrapped") and it requires
cleanly-authored loop content (no per-wrap fade like PSP's, matching
the same assumption). Compiles cleanly for both gamecube and wii;
not yet verified on real hardware.

Confirmed on real PSP hardware: no more gap, crackle fix pending
final hardware confirmation.

src/dusk/audio/audiostream.c

@@ -22,6 +22,8 @@ errorret_t audioStreamInit(audiostream_t *stream) {
   stream->user = NULL;
   stream->onLoop = NULL;
   stream->onEnd = NULL;
+  stream->loopCount = 0;
+  stream->lastLoopCount = 0;
 
   // Type-specific setup (audioStreamPcmInit() / audioStreamMp3Init()) sets
   // stream->type and asks the platform implementation to set up its state.
@@ -85,6 +87,20 @@ errorret_t audioStreamUpdate(audiostream_t *stream) {
     errorOk();
   }
 
+  // Some platforms (PSP) loop entirely on their own background thread for
+  // gaplessness and can only safely notify onLoop by incrementing this
+  // counter from there, rather than calling onLoop directly off the main
+  // thread - see loopCount's own comment. Firing it here, unconditionally,
+  // catches up to the latest count in one call even if multiple loops
+  // happened between two Update() calls.
+  if(stream->loopCount != stream->lastLoopCount) {
+    stream->lastLoopCount = stream->loopCount;
+
+    if(stream->onLoop != NULL) {
+      stream->onLoop(stream);
+    }
+  }
+
   // TODO: Once streamed (rather than fully in-memory) sources exist, this is
   // where new data would be decoded/read in and re-buffered as playback
   // consumes it. Looping currently only supports restarting from the very

src/dusk/audio/audiostream.h

@@ -77,6 +77,19 @@ typedef struct audiostream_s {
   void (*onLoop)(audiostream_t *stream);
   void (*onEnd)(audiostream_t *stream);
 
+  // Incremented by platform code (from whatever thread/context it runs in)
+  // each time a loop happens, instead of calling onLoop directly - onLoop
+  // may do arbitrary, possibly-slow work (console printing, game logic),
+  // which is only safe to run from the main thread inside audioStreamUpdate().
+  // Calling it straight from a real-time audio thread risks starving the
+  // hardware buffer if it takes too long - confirmed as the real cause of
+  // a loud crackle on PSP once its feeder thread called onLoop inline.
+  volatile uint32_t loopCount;
+
+  // audioStreamUpdate()'s own record of the last loopCount it fired
+  // onLoop for - only ever touched from the main thread.
+  uint32_t lastLoopCount;
+
   // Stream type specific data.
   union {
     audiostreampcm_t pcm;

src/duskdolphin/audio/audiostreamdolphin.c

@@ -63,6 +63,26 @@ errorret_t audioStreamDolphinBuffer(audiostream_t *stream) {
   config.stream_callback = NULL; // single-buffer playback
   config.user_pointer = stream;
 
+  // Loop entirely in hardware rather than mirroring PSP/Linux's
+  // detect-finished-then-restart-from-software approach: ansnd's DSP mixer
+  // wraps frame_count-1 back to 0 on its own, so there's no restart latency
+  // to create a gap in the first place (unlike a software restart, which
+  // always costs at least a little). Only restart-from-the-very-start is
+  // supported elsewhere in this codebase (loopTo isn't honored yet), which
+  // matches exactly what loop_start_offset=0 gives here.
+  //
+  // Trade-off: onLoop never fires for a Dolphin voice looping this way -
+  // there's no ANSND_VOICE_STATE for "wrapped", only state transitions like
+  // FINISHED/STOPPED, which a looping voice never reaches. And unlike
+  // PSP's per-restart tail fade, the DSP does no smoothing at the wrap
+  // point - it requires the source data's last frame to already flow
+  // cleanly into its first (true today only because the shared 441Hz test
+  // tone was deliberately chosen to divide evenly into the sample rate).
+  if(stream->state & AUDIO_STREAM_STATE_LOOPING) {
+    config.loop_start_offset = 0;
+    config.loop_end_offset = config.frame_count - 1;
+  }
+
   s32 result = ansnd_configure_pcm_voice((u32) stream->platform.voiceId, &config);
   if(result != ANSND_ERROR_OK) {
     errorThrow(

src/duskpsp/audio/audiostreampsp.c

@@ -32,11 +32,11 @@
 // just stopping outright on an exact-multiple-length one).
 #define AUDIO_PSP_FADE_FRAMES 32
 
-// A couple of extra all-silence chunks fed after the real audio (and
-// after the fade above), so the channel keeps being actively driven at
-// zero for a moment rather than stopping outright - mitigates a possible
-// hardware/DAC settling pop on top of the digital-domain fade.
-#define AUDIO_PSP_TRAILING_SILENT_CHUNKS 2
+// How long the persistent feeder thread sleeps between checks for a new
+// play request while idle. Cheap compared to spawning a whole new OS
+// thread per play (what this replaced) - worst case this is the latency
+// added between one playback pass finishing and the next one starting.
+#define AUDIO_PSP_IDLE_POLL_MICROS 500
 
 errorret_t audioStreamPSPInit(audiostream_t *stream) {
   assertNotNull(stream, "Stream cannot be NULL.");
@@ -71,7 +71,10 @@ errorret_t audioStreamPSPInit(audiostream_t *stream) {
 
   stream->platform.channel = channel;
   stream->platform.finished = false;
+  stream->platform.playRequested = false;
   threadInit(&stream->platform.thread, audioStreamPSPThreadFeed);
+  stream->platform.thread.data = stream;
+  threadStartRequest(&stream->platform.thread);
 
   errorOk();
 }
@@ -89,8 +92,7 @@ errorret_t audioStreamPSPBuffer(audiostream_t *stream) {
   assertNotNull(stream, "Stream cannot be NULL.");
 
   stream->platform.finished = false;
-  stream->platform.thread.data = stream;
-  threadStartRequest(&stream->platform.thread);
+  stream->platform.playRequested = true;
 
   errorOk();
 }
@@ -109,32 +111,76 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
   audiostream_t *stream = (audiostream_t *) thread->data;
   const size_t channels = stream->pcm.channels;
   const size_t frameSize = channels * sizeof(int16_t);
-  const size_t totalFrames = stream->dataSize / frameSize;
   const size_t chunkSize = AUDIO_PSP_CHUNK_FRAMES * frameSize;
-
   int16_t *chunk = memoryAllocate(chunkSize);
+
+  // Runs for the stream's whole lifetime - idles here between plays rather
+  // than exiting, so a loop restart (or any replay) is just a flag flip
+  // audioStreamPSPBuffer() sets, not a whole new thread being spawned.
+  while(!threadShouldStop(thread)) {
+    if(!stream->platform.playRequested) {
+      sceKernelDelayThread(AUDIO_PSP_IDLE_POLL_MICROS);
+      continue;
+    }
+    stream->platform.playRequested = false;
+
+    const size_t totalFrames = stream->dataSize / frameSize;
     size_t position = 0;
+    bool_t reachedEnd = false;
 
     // Every call always sends a full, constant-size AUDIO_PSP_CHUNK_FRAMES
     // buffer - the channel is never re-declared to a different length, to
     // avoid relying on sceAudioSetChannelDataLen's undocumented behavior
-  // mid-stream. The final chunk is zero-padded out to that full size.
-  while(!threadShouldStop(thread) && position < totalFrames) {
+    // mid-stream (the likely real cause of an earlier click). Loops
+    // internally (rather than going idle and waiting for the engine's
+    // once-per-frame Update() to notice finished and re-trigger Buffer())
+    // so a looping stream never has a detection-latency gap - the generic
+    // engine's finished+looping path is designed for platforms with no
+    // better option (see audiostream.c), but here the thread can just
+    // keep going and call onLoop itself instead.
+    while(!threadShouldStop(thread) && !reachedEnd) {
       const size_t framesRemaining = totalFrames - position;
       const size_t framesThisChunk = framesRemaining < AUDIO_PSP_CHUNK_FRAMES
         ? framesRemaining
         : AUDIO_PSP_CHUNK_FRAMES;
-    const bool_t isLastChunk = framesThisChunk == framesRemaining;
+      const bool_t reachesEndThisChunk = framesThisChunk == framesRemaining;
+      const bool_t willLoop = reachesEndThisChunk &&
+        (stream->state & AUDIO_STREAM_STATE_LOOPING);
 
-    memoryZero(chunk, chunkSize);
       memoryCopy(
         chunk, stream->data + (position * frameSize), framesThisChunk * frameSize
       );
 
-    if(isLastChunk) {
-      // Fade the tail down to zero so the waveform never stops (or meets
-      // the zero-padding above) at a non-zero amplitude - that abrupt
-      // jump is what was heard as a click at the end of playback.
+      const size_t remainderFrames = AUDIO_PSP_CHUNK_FRAMES - framesThisChunk;
+      size_t wrapFrames = 0;
+
+      if(reachesEndThisChunk && willLoop) {
+        // Instead of padding the rest of this constant-size chunk with
+        // silence, fill it with the start of the next pass - a looping
+        // stream should never have dead air baked into its output at all.
+        // Not faded (unlike the true-end case below): a wrap only sounds
+        // seamless if the source data already loops cleanly (true for the
+        // shared test tone, deliberately authored at 441Hz for exactly
+        // this), matching the same assumption Dolphin's native hardware
+        // looping makes - imperfectly-authored loop content will still
+        // click here.
+        wrapFrames = remainderFrames < totalFrames ? remainderFrames : totalFrames;
+        memoryCopy(
+          chunk + (framesThisChunk * channels), stream->data, wrapFrames * frameSize
+        );
+        if(wrapFrames < remainderFrames) {
+          memoryZero(
+            chunk + ((framesThisChunk + wrapFrames) * channels),
+            (remainderFrames - wrapFrames) * frameSize
+          );
+        }
+      } else if(reachesEndThisChunk) {
+        // True final chunk (not looping again) - pad with silence and fade
+        // the real tail down to zero so the waveform never stops (or meets
+        // that padding) at a non-zero amplitude, which is what was heard
+        // as a click at the end of playback.
+        memoryZero(chunk + (framesThisChunk * channels), remainderFrames * frameSize);
+
         const size_t fadeFrames = framesThisChunk < AUDIO_PSP_FADE_FRAMES
           ? framesThisChunk
           : AUDIO_PSP_FADE_FRAMES;
@@ -162,20 +208,25 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
         stream->platform.channel, leftVolume, rightVolume, chunk
       );
 
+      if(reachesEndThisChunk) {
+        if(willLoop) {
+          // Never call stream->onLoop directly from this thread - it may do
+          // arbitrary, possibly-slow work (this is exactly what caused a
+          // loud crackle: onLoop console-printing took long enough to
+          // starve the next chunk). audioStreamUpdate() picks this up and
+          // fires onLoop safely from the main thread instead.
+          stream->loopCount++;
+          position = wrapFrames;
+        } else {
+          reachedEnd = true;
+        }
+      } else {
         position += framesThisChunk;
       }
+    }
 
-  // A couple of extra all-silence chunks so the channel keeps being
-  // actively driven at zero for a moment rather than stopping outright.
-  memoryZero(chunk, chunkSize);
-  for(
-    uint8_t i = 0;
-    !threadShouldStop(thread) && i < AUDIO_PSP_TRAILING_SILENT_CHUNKS;
-    i++
-  ) {
-    sceAudioOutputBlocking(stream->platform.channel, 0, chunk);
+    stream->platform.finished = true;
   }
 
   memoryFree(chunk);
-  stream->platform.finished = true;
 }

src/duskpsp/audio/audiostreampsp.h

@@ -17,11 +17,20 @@ typedef struct {
   // expects continuous small-chunk feeding, not one large buffer per call.
   int channel;
 
-  // Dedicated thread that feeds the channel chunk-by-chunk for the
-  // duration of playback, independent of the engine's frame rate.
+  // Persistent thread, created once in Init and alive for the stream's
+  // whole lifetime - feeds the channel chunk-by-chunk for the duration of
+  // playback, independent of the engine's frame rate. Re-spawning a
+  // thread on every Buffer() call (e.g. every loop restart) was real,
+  // avoidable overhead - a plain OS thread creation, on top of everything
+  // else - heard as a small gap between loops; this thread just idles
+  // (polling playRequested) between plays instead of exiting.
   thread_t thread;
 
-  // Set by audioStreamPSPThreadFeed() once it has fed the last chunk.
+  // Set by audioStreamPSPBuffer() to wake the idling thread into feeding
+  // a pass; cleared by the thread once it picks it up.
+  volatile bool_t playRequested;
+
+  // Set by the thread once it has fed the last chunk of a pass.
   volatile bool_t finished;
 } audiostreampsp_t;
 
@@ -44,8 +53,8 @@ errorret_t audioStreamPSPInit(audiostream_t *stream);
 errorret_t audioStreamPSPDispose(audiostream_t *stream);
 
 /**
- * Starts the stream's feeder thread, which streams stream->data to its
- * reserved hardware output channel in small chunks until exhausted.
+ * Wakes the stream's persistent feeder thread to stream stream->data to
+ * its reserved hardware output channel in small chunks until exhausted.
  *
  * @param stream The audio stream to output.
  * @return Error state if any.
@@ -62,10 +71,13 @@ errorret_t audioStreamPSPBuffer(audiostream_t *stream);
 bool_t audioStreamPSPIsFinished(audiostream_t *stream);
 
 /**
- * Feeder thread entry point. Streams stream->data (passed via thread->data)
- * to its hardware channel in fixed-size chunks, blocking naturally on each
- * sceAudioOutputPannedBlocking() call, until the whole buffer has been sent
- * or the thread is asked to stop.
+ * Feeder thread entry point, run once for the stream's whole lifetime.
+ * Idles (polling playRequested) until woken by audioStreamPSPBuffer(),
+ * then streams stream->data (passed via thread->data) to its hardware
+ * channel in fixed-size chunks, blocking naturally on each
+ * sceAudioOutputPannedBlocking() call, until the whole buffer has been
+ * sent - then goes back to idling, ready for the next play request, until
+ * the thread is asked to stop.
  *
  * @param thread The running thread_t, with data set to the audiostream_t.
  */