Split PSP audio feeder into reader + player threads to fix crackle

sceAudioOutputPannedBlocking() occupies the calling thread for the full
duration of the chunk it just submitted. That was fine while PCM chunks
came out of a fully-resident buffer (a near-instant memcpy), but now that
they're read from the asset on demand, that same read happens in between
output calls on the same thread - any read slower than a memcpy opens a
real gap in the hardware channel, heard as crackle.

Split the single feeder thread in two: a reader thread that does all PCM
I/O (seek/read, loop-wrap, fade prep) ahead of playback into a small
3-slot queue, and a player thread that only pulls ready chunks off the
queue and outputs them. This overlaps I/O with hardware playback instead
of serializing them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/duskpsp/audio/audiostreampsp.c

@@ -21,10 +21,11 @@
 // PSP priorities are inverted (lower = higher priority). This project's
 // main thread runs at the PSPSDK default of 32 (PSP_MAIN_THREAD_PRIORITY
 // is never overridden). A pthread created with default attributes runs at
-// 60 - LOWER priority than the main/render thread - so under load the
-// feeder thread starves and the hardware channel underruns, heard as
+// 60 - LOWER priority than the main/render thread - so under load either
+// worker thread would starve and the hardware channel underruns, heard as
 // jitter/crackle that gets worse the busier (lower-fps) a frame is. Raise
-// it above main so audio feeding always wins scheduling contention.
+// both the player and reader threads above main so audio feeding always
+// wins scheduling contention.
 #define AUDIO_PSP_THREAD_PRIORITY 18
 
 // How many frames at the very end of a stream get linearly faded to zero,
@@ -33,10 +34,12 @@
 // just stopping outright on an exact-multiple-length one).
 #define AUDIO_PSP_FADE_FRAMES 32
 
-// 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.
+// How long the persistent player/reader threads sleep between polls - both
+// for a new play request while idle, and for queue slots while active.
+// 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, or between a queue slot
+// freeing up and the waiting thread noticing.
 #define AUDIO_PSP_IDLE_POLL_MICROS 500
 
 errorret_t audioStreamPSPInit(audiostream_t *stream) {
@@ -73,19 +76,43 @@ errorret_t audioStreamPSPInit(audiostream_t *stream) {
   stream->platform.channel = channel;
   stream->platform.finished = false;
   stream->platform.playRequested = false;
+  stream->platform.readRequested = false;
+  stream->platform.readFailed = false;
+  stream->platform.queueHead = 0;
+  stream->platform.queueTail = 0;
+  stream->platform.queueCount = 0;
+
+  const size_t chunkSize =
+    AUDIO_PSP_CHUNK_FRAMES * stream->pcm.channels * sizeof(int16_t);
+  for(size_t i = 0; i < AUDIO_PSP_QUEUE_DEPTH; i++) {
+    stream->platform.queueChunk[i] = memoryAllocate(chunkSize);
+  }
+  threadMutexInit(&stream->platform.queueLock);
+
   threadInit(&stream->platform.thread, audioStreamPSPThreadFeed);
   stream->platform.thread.data = stream;
   threadStartRequest(&stream->platform.thread);
 
+  threadInit(&stream->platform.readerThread, audioStreamPSPThreadRead);
+  stream->platform.readerThread.data = stream;
+  threadStartRequest(&stream->platform.readerThread);
+
   errorOk();
 }
 
 errorret_t audioStreamPSPDispose(audiostream_t *stream) {
   assertNotNull(stream, "Stream cannot be NULL.");
 
+  threadStop(&stream->platform.readerThread);
   threadStop(&stream->platform.thread);
   sceAudioChRelease(stream->platform.channel);
 
+  for(size_t i = 0; i < AUDIO_PSP_QUEUE_DEPTH; i++) {
+    memoryFree(stream->platform.queueChunk[i]);
+    stream->platform.queueChunk[i] = NULL;
+  }
+  threadMutexDispose(&stream->platform.queueLock);
+
   errorOk();
 }
 
@@ -96,6 +123,19 @@ errorret_t audioStreamPSPBuffer(audiostream_t *stream) {
   stream->platform.startFrame = stream->startFrame;
   stream->startFrame = 0;
   stream->seeking = false;
+  stream->platform.readFailed = false;
+
+  threadMutexLock(&stream->platform.queueLock);
+  stream->platform.queueHead = 0;
+  stream->platform.queueTail = 0;
+  stream->platform.queueCount = 0;
+  threadMutexUnlock(&stream->platform.queueLock);
+
+  // Give the reader a head start filling the queue before the player
+  // starts draining it - not required for correctness (the player just
+  // waits for the first ready slot either way), but avoids guaranteeing an
+  // initial stall on every pass.
+  stream->platform.readRequested = true;
   stream->platform.playRequested = true;
 
   errorOk();
@@ -107,7 +147,7 @@ bool_t audioStreamPSPIsFinished(audiostream_t *stream) {
   return stream->platform.finished;
 }
 
-void audioStreamPSPThreadFeed(thread_t *thread) {
+void audioStreamPSPThreadRead(thread_t *thread) {
   assertNotNull(thread, "Thread cannot be NULL.");
 
   sceKernelChangeThreadPriority(sceKernelGetThreadId(), AUDIO_PSP_THREAD_PRIORITY);
@@ -115,18 +155,15 @@ 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 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.
+  // than exiting, same as the player thread.
   while(!threadShouldStop(thread)) {
-    if(!stream->platform.playRequested) {
+    if(!stream->platform.readRequested) {
       sceKernelDelayThread(AUDIO_PSP_IDLE_POLL_MICROS);
       continue;
     }
-    stream->platform.playRequested = false;
+    stream->platform.readRequested = false;
 
     const size_t totalFrames = audioStreamPcmGetTotalFrames(stream);
 
@@ -147,25 +184,28 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
     size_t position = stream->platform.startFrame;
     bool_t reachedEnd = false;
 
-    // Samples are read on demand from the asset (via audioStreamPcmRead(),
-    // sequentially, plus an explicit audioStreamPcmSeek() whenever jumping
-    // backward for a loop wrap) rather than indexed out of a fully
-    // resident buffer - a read/seek failure here (a corrupt or truncated
-    // asset, an I/O error) stops playback cleanly instead of crashing the
-    // thread on bad data.
     bool_t readFailed = errorIsNotOk(audioStreamPcmSeek(stream, position));
+    if(readFailed) stream->platform.readFailed = true;
 
-    // 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 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(!readFailed && !threadShouldStop(thread) && !reachedEnd) {
+      // Wait for a free slot in the queue before preparing the next chunk -
+      // this is the only thing that paces the reader against the player.
+      // If the thread is asked to stop while waiting, give up on this pass
+      // entirely rather than finishing it.
+      bool_t stopRequested = false;
+      for(;;) {
+        threadMutexLock(&stream->platform.queueLock);
+        bool_t hasRoom = stream->platform.queueCount < AUDIO_PSP_QUEUE_DEPTH;
+        threadMutexUnlock(&stream->platform.queueLock);
+        if(hasRoom) break;
+        if(threadShouldStop(thread)) {
+          stopRequested = true;
+          break;
+        }
+        sceKernelDelayThread(AUDIO_PSP_IDLE_POLL_MICROS);
+      }
+      if(stopRequested) break;
+
       // Normally bounded by loopEndFrame (the loop segment's end), but a
       // seek can legitimately land past it (e.g. into an outro after the
       // loop point) - in that case play out to the true end of the buffer
@@ -181,11 +221,15 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
       const bool_t willLoop = reachesEndThisChunk &&
         (stream->state & AUDIO_STREAM_STATE_LOOPING);
 
+      const size_t slot = stream->platform.queueHead;
+      int16_t *chunk = stream->platform.queueChunk[slot];
+
       size_t framesRead = 0;
       if(errorIsNotOk(
         audioStreamPcmRead(stream, chunk, framesThisChunk, &framesRead)
       )) {
         readFailed = true;
+        stream->platform.readFailed = true;
         break;
       }
       if(framesRead < framesThisChunk) {
@@ -229,6 +273,7 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
 
         if(errorIsNotOk(audioStreamPcmSeek(stream, loopToFrame))) {
           readFailed = true;
+          stream->platform.readFailed = true;
           break;
         }
         size_t wrapRead = 0;
@@ -236,6 +281,7 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
           stream, chunk + (framesThisChunk * channels), wrapFrames, &wrapRead
         ))) {
           readFailed = true;
+          stream->platform.readFailed = true;
           break;
         }
         if(wrapRead < remainderFrames) {
@@ -264,6 +310,85 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
         }
       }
 
+      threadMutexLock(&stream->platform.queueLock);
+      stream->platform.queueReachedEnd[slot] = reachesEndThisChunk;
+      stream->platform.queueLooped[slot] = reachesEndThisChunk && willLoop;
+      stream->platform.queueHead = (slot + 1) % AUDIO_PSP_QUEUE_DEPTH;
+      stream->platform.queueCount++;
+      threadMutexUnlock(&stream->platform.queueLock);
+
+      if(reachesEndThisChunk) {
+        if(willLoop) {
+          position = loopToFrame + wrapFrames;
+        } else {
+          reachedEnd = true;
+        }
+      } else {
+        position += framesThisChunk;
+      }
+    }
+  }
+}
+
+void audioStreamPSPThreadFeed(thread_t *thread) {
+  assertNotNull(thread, "Thread cannot be NULL.");
+
+  sceKernelChangeThreadPriority(sceKernelGetThreadId(), AUDIO_PSP_THREAD_PRIORITY);
+
+  audiostream_t *stream = (audiostream_t *) thread->data;
+
+  // 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;
+
+    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 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) {
+      // Wait for the reader thread to have a chunk ready. sceAudioOutput-
+      // PannedBlocking() below blocks this thread for the full duration of
+      // whatever chunk it's given, so there is no spare time here to also
+      // do the PCM read/seek itself without stalling the hardware channel
+      // - that's exactly what the reader thread (audioStreamPSPThreadRead)
+      // exists to do concurrently, ahead of what's currently playing.
+      bool_t stopRequested = false;
+      size_t slot = 0;
+      for(;;) {
+        threadMutexLock(&stream->platform.queueLock);
+        bool_t hasChunk = stream->platform.queueCount > 0;
+        bool_t gaveUp = !hasChunk && stream->platform.readFailed;
+        slot = stream->platform.queueTail;
+        threadMutexUnlock(&stream->platform.queueLock);
+        if(hasChunk) break;
+        // The reader already failed and has nothing left queued - it will
+        // never produce another chunk for this pass, so stop waiting.
+        if(gaveUp || threadShouldStop(thread)) {
+          stopRequested = true;
+          break;
+        }
+        sceKernelDelayThread(AUDIO_PSP_IDLE_POLL_MICROS);
+      }
+      if(stopRequested) break;
+
+      int16_t *chunk = stream->platform.queueChunk[slot];
+      const bool_t chunkReachedEnd = stream->platform.queueReachedEnd[slot];
+      const bool_t chunkLooped = stream->platform.queueLooped[slot];
+
       // Re-read every chunk (~23ms at 44100Hz) so SetVolume/SetDirectionality
       // take effect mid-playback, unlike the platform's other one-shot calls.
       float_t leftFactor, rightFactor;
@@ -277,25 +402,29 @@ void audioStreamPSPThreadFeed(thread_t *thread) {
         stream->platform.channel, leftVolume, rightVolume, chunk
       );
 
-      if(reachesEndThisChunk) {
-        if(willLoop) {
+      // Only freed (queueCount decremented) after the blocking output call
+      // above returns - the reader thread must never be able to reuse this
+      // slot's buffer while sceAudioOutputPannedBlocking is still reading
+      // from it.
+      threadMutexLock(&stream->platform.queueLock);
+      stream->platform.queueTail = (slot + 1) % AUDIO_PSP_QUEUE_DEPTH;
+      stream->platform.queueCount--;
+      threadMutexUnlock(&stream->platform.queueLock);
+
+      if(chunkReachedEnd) {
+        if(chunkLooped) {
           // 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 = loopToFrame + wrapFrames;
         } else {
           reachedEnd = true;
         }
-      } else {
-        position += framesThisChunk;
       }
     }
 
     stream->platform.finished = true;
   }
-
-  memoryFree(chunk);
 }

src/duskpsp/audio/audiostreampsp.h

@@ -11,34 +11,92 @@
 
 typedef struct audiostream_s audiostream_t;
 
+// Depth of the read-ahead queue between the reader and player threads (see
+// audiostreampsp_t below) - how many fully-prepared hardware chunks the
+// reader is allowed to get ahead of what the player is currently
+// outputting. Each slot is one AUDIO_PSP_CHUNK_FRAMES chunk (a few KB), so
+// this costs very little memory; it exists purely to give PCM I/O (now a
+// real Memory Stick/zip read per chunk, not a RAM copy - see
+// audioStreamPcmRead()) enough of a cushion to never stall the player
+// thread's real-time output loop. 3 was picked as "more than one" (so a
+// single slow read doesn't immediately starve playback) without holding
+// much more than necessary.
+#define AUDIO_PSP_QUEUE_DEPTH 3
+
 typedef struct {
   // Reserved hardware output channel, from sceAudioChReserve. Reserved with
   // a small fixed chunk size (AUDIO_PSP_CHUNK_FRAMES) - PSP audio hardware
   // expects continuous small-chunk feeding, not one large buffer per call.
   int channel;
 
-  // 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.
+  // Persistent "player" thread, created once in Init and alive for the
+  // stream's whole lifetime - the only thread that ever calls
+  // sceAudioOutputPannedBlocking(), taking chunks off the queue below
+  // rather than reading PCM data itself. 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 audioStreamPSPBuffer() to wake the idling thread into feeding
-  // a pass; cleared by the thread once it picks it up.
+  // Persistent "reader" thread, created once in Init alongside `thread` -
+  // does all PCM I/O (audioStreamPcmSeek()/audioStreamPcmRead(), plus loop
+  // wrap/fade preparation) into the queue below, running ahead of what
+  // `thread` is currently outputting. sceAudioOutputPannedBlocking() blocks
+  // the calling thread for the full duration of the chunk it just
+  // submitted, so there is no spare time on that thread to also do I/O
+  // in between calls without stalling the hardware channel - that stall is
+  // exactly what was heard as severe crackling once PCM reads stopped
+  // being a fully-resident-buffer memcpy (fast, always well inside the
+  // ~23ms chunk budget) and started being real, sometimes-slow reads. This
+  // second thread is what buys that time back.
+  thread_t readerThread;
+
+  // Set by audioStreamPSPBuffer() to wake the idling player thread into a
+  // new pass; cleared by that thread once it picks it up.
   volatile bool_t playRequested;
 
+  // Same as playRequested, but for the reader thread - set/cleared
+  // independently since the two threads pick up a new pass at slightly
+  // different times (whichever wakes from its idle poll first).
+  volatile bool_t readRequested;
+
   // Frame offset the next pass should start from - captured synchronously
   // from audiostream_t.startFrame by audioStreamPSPBuffer() (which also
-  // resets that field to 0) rather than read directly by the feeder thread,
-  // since the thread only wakes up asynchronously and audiostream_t's
-  // shared field may already have moved on to a different value by then.
+  // resets that field to 0) rather than read directly by the reader
+  // thread, since that thread only wakes up asynchronously and
+  // audiostream_t's shared field may already have moved on to a different
+  // value by then.
   size_t startFrame;
 
-  // Set by the thread once it has fed the last chunk of a pass.
+  // Set by the player thread once it has output the last chunk of a pass.
   volatile bool_t finished;
+
+  // Bounded queue of fully-prepared, constant-size (AUDIO_PSP_CHUNK_FRAMES)
+  // hardware chunks handed from the reader thread to the player thread.
+  // queueHead/queueTail/queueCount are only ever touched while holding
+  // queueLock. Each queueChunk[] buffer is allocated once (in Init) and
+  // reused for the stream's whole lifetime.
+  int16_t *queueChunk[AUDIO_PSP_QUEUE_DEPTH];
+
+  // Per-slot bookkeeping the player thread needs once it plays that chunk:
+  // whether this was the pass's true final chunk (queueReachedEnd) and, if
+  // so, whether it was a loop wrap (queueLooped, bump loopCount and keep
+  // going) or the genuine end (stop and set finished).
+  bool_t queueReachedEnd[AUDIO_PSP_QUEUE_DEPTH];
+  bool_t queueLooped[AUDIO_PSP_QUEUE_DEPTH];
+
+  size_t queueHead; // Next slot index the reader thread will fill.
+  size_t queueTail; // Next slot index the player thread will consume.
+  size_t queueCount; // Number of filled-and-ready slots.
+  threadmutex_t queueLock;
+
+  // Set by the reader thread if a seek/read fails mid-pass (corrupt or
+  // truncated asset, I/O error) - observed by the player thread once it
+  // drains whatever was already queued, so the pass still ends cleanly
+  // instead of the player waiting forever for a chunk that will never
+  // arrive.
+  volatile bool_t readFailed;
 } audiostreampsp_t;
 
 /**
@@ -60,9 +118,10 @@ errorret_t audioStreamPSPInit(audiostream_t *stream);
 errorret_t audioStreamPSPDispose(audiostream_t *stream);
 
 /**
- * Wakes the stream's persistent feeder thread to stream PCM data (read on
- * demand from the stream's asset via audioStreamPcmRead()) to its reserved
- * hardware output channel in small chunks until exhausted.
+ * Wakes the stream's persistent reader and player threads to stream PCM
+ * data (read ahead from the stream's asset via audioStreamPcmRead() by the
+ * reader thread) to its reserved hardware output channel in small chunks
+ * until exhausted.
  *
  * @param stream The audio stream to output.
  * @return Error state if any.
@@ -70,7 +129,7 @@ errorret_t audioStreamPSPDispose(audiostream_t *stream);
 errorret_t audioStreamPSPBuffer(audiostream_t *stream);
 
 /**
- * Checks whether the stream's feeder thread has finished feeding its
+ * Checks whether the stream's player thread has finished outputting its
  * currently buffered data.
  *
  * @param stream The audio stream to check.
@@ -79,15 +138,32 @@ errorret_t audioStreamPSPBuffer(audiostream_t *stream);
 bool_t audioStreamPSPIsFinished(audiostream_t *stream);
 
 /**
- * Feeder thread entry point, run once for the stream's whole lifetime.
- * Idles (polling playRequested) until woken by audioStreamPSPBuffer(),
- * then reads and streams the stream's asset PCM data (via the audiostream_t
- * passed as 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.
+ * Player thread entry point, run once for the stream's whole lifetime.
+ * Idles (polling playRequested) until woken by audioStreamPSPBuffer(), then
+ * takes fully-prepared chunks off the queue (filled by the reader thread -
+ * see audioStreamPSPThreadRead()) and outputs them to the hardware channel
+ * one at a time, blocking naturally on each sceAudioOutputPannedBlocking()
+ * call, until the pass's final chunk has been sent - then goes back to
+ * idling, ready for the next play request, until the thread is asked to
+ * stop. Never touches the asset/PCM layer directly.
  *
  * @param thread The running thread_t, with data set to the audiostream_t.
  */
 void audioStreamPSPThreadFeed(thread_t *thread);
+
+/**
+ * Reader thread entry point, run once for the stream's whole lifetime.
+ * Idles (polling readRequested) until woken by audioStreamPSPBuffer(), then
+ * reads the stream's asset PCM data (via audioStreamPcmSeek()/
+ * audioStreamPcmRead()) and prepares fixed-size hardware chunks (applying
+ * loop wrap/fade, same as the player thread used to do inline), pushing
+ * each onto the queue for the player thread to consume - running ahead of
+ * playback rather than in lockstep with it, so PCM I/O latency never
+ * stalls the player thread's real-time output loop. Stops producing once
+ * it queues the pass's final chunk (or a read/seek fails - see
+ * audiostreampsp_t.readFailed), then goes back to idling until the thread
+ * is asked to stop.
+ *
+ * @param thread The running thread_t, with data set to the audiostream_t.
+ */
+void audioStreamPSPThreadRead(thread_t *thread);