CDROM: Move audio FIFO to CDROM class and skip sectors when unempty

Fixes cutscene audio in Nickelodeon Rugrats - Search for Reptar.

src/core/cdrom.cpp

@@ -345,7 +345,7 @@ void CDROM::SoftReset()
   m_next_cd_audio_volume_matrix[1][0] = 0x00;
   m_next_cd_audio_volume_matrix[1][1] = 0x80;
   m_cd_audio_volume_matrix = m_next_cd_audio_volume_matrix;
-  ResetXAResampler();
+  ResetAudioDecoder();
 
   m_param_fifo.Clear();
   m_response_fifo.Clear();
@@ -414,6 +414,8 @@ bool CDROM::DoState(StateWrapper& sw)
     sw.Do(&m_sector_buffers[i].size);
   }
 
+  sw.Do(&m_audio_fifo);
+
   u32 requested_sector = (sw.IsWriting() ? (m_reader.WaitForReadToComplete(), m_reader.GetLastReadSector()) : 0);
   sw.Do(&requested_sector);
 
@@ -1551,7 +1553,7 @@ void CDROM::BeginReading(TickCount ticks_late /* = 0 */, bool after_seek /* = fa
   m_current_read_sector_buffer = 0;
   m_current_write_sector_buffer = 0;
   ResetCurrentXAFile();
-  ResetXAResampler();
+  ResetAudioDecoder();
 
   m_reader.QueueReadSector(m_current_lba);
 }
@@ -1593,6 +1595,8 @@ void CDROM::BeginPlaying(u8 track_bcd, TickCount ticks_late /* = 0 */, bool afte
   m_drive_event->Schedule(first_sector_ticks);
   m_current_read_sector_buffer = 0;
   m_current_write_sector_buffer = 0;
+  ResetAudioDecoder();
+  ResetCurrentXAFile();
 
   m_reader.QueueReadSector(m_current_lba);
 }
@@ -2079,14 +2083,21 @@ static constexpr s16 SaturateVolume(s32 volume)
 }
 
 template<bool STEREO, bool SAMPLE_RATE>
-static void ResampleXAADPCM(const s16* frames_in, u32 num_frames_in, SPU* spu, s16* left_ringbuf, s16* right_ringbuf,
+void CDROM::ResampleXAADPCM(const s16* frames_in, u32 num_frames_in)
-                            u8* p_ptr, u8* sixstep_ptr, const std::array<std::array<u8, 2>, 2>& volume_matrix)
 {
-  u8 p = *p_ptr;
+  // Since the disc reads and SPU are running at different speeds, we might be _slightly_ behind, which is fine, since
-  u8 sixstep = *sixstep_ptr;
+  // the SPU will over-read in the next batch to catch up.
-
+  if (m_audio_fifo.GetSize() > AUDIO_FIFO_LOW_WATERMARK)
-  spu->EnsureCDAudioSpace(((num_frames_in * 7) / 6) << BoolToUInt8(SAMPLE_RATE));
+  {
+    Log_DevPrintf("Dropping %u XA frames because audio FIFO still has %u frames", num_frames_in,
+                  m_audio_fifo.GetSize());
+    return;
+  }
 
+  s16* left_ringbuf = m_xa_resample_ring_buffer[0].data();
+  s16* right_ringbuf = m_xa_resample_ring_buffer[1].data();
+  u8 p = m_xa_resample_p;
+  u8 sixstep = m_xa_resample_sixstep;
   for (u32 in_sample_index = 0; in_sample_index < num_frames_in; in_sample_index++)
   {
     const s16 left = *(frames_in++);
@@ -2113,19 +2124,18 @@ static void ResampleXAADPCM(const s16* frames_in, u32 num_frames_in, SPU* spu, s
           const s16 left_interp = ZigZagInterpolate(left_ringbuf, s_zigzag_table[j].data(), p);
           const s16 right_interp = STEREO ? ZigZagInterpolate(right_ringbuf, s_zigzag_table[j].data(), p) : left_interp;
 
-          const s16 left_out = SaturateVolume(ApplyVolume(left_interp, volume_matrix[0][0]) +
+          const s16 left_out = SaturateVolume(ApplyVolume(left_interp, m_cd_audio_volume_matrix[0][0]) +
-                                              ApplyVolume(right_interp, volume_matrix[1][0]));
+                                              ApplyVolume(right_interp, m_cd_audio_volume_matrix[1][0]));
-          const s16 right_out = SaturateVolume(ApplyVolume(left_interp, volume_matrix[0][1]) +
+          const s16 right_out = SaturateVolume(ApplyVolume(left_interp, m_cd_audio_volume_matrix[0][1]) +
-                                               ApplyVolume(right_interp, volume_matrix[1][1]));
+                                               ApplyVolume(right_interp, m_cd_audio_volume_matrix[1][1]));
-
+          AddCDAudioFrame(left_out, right_out);
-          spu->AddCDAudioSample(left_out, right_out);
         }
       }
     }
   }
 
-  *p_ptr = p;
+  m_xa_resample_p = p;
-  *sixstep_ptr = sixstep;
+  m_xa_resample_sixstep = sixstep;
 }
 
 void CDROM::ResetCurrentXAFile()
@@ -2135,7 +2145,7 @@ void CDROM::ResetCurrentXAFile()
   m_xa_current_set = false;
 }
 
-void CDROM::ResetXAResampler()
+void CDROM::ResetAudioDecoder()
 {
   m_xa_last_samples.fill(0);
   for (u32 i = 0; i < 2; i++)
@@ -2144,6 +2154,7 @@ void CDROM::ResetXAResampler()
     m_xa_resample_p = 0;
     m_xa_resample_sixstep = 6;
   }
+  m_audio_fifo.Clear();
 }
 
 void CDROM::ProcessXAADPCMSector(const u8* raw_sector, const CDImage::SubChannelQ& subq)
@@ -2176,7 +2187,7 @@ void CDROM::ProcessXAADPCMSector(const u8* raw_sector, const CDImage::SubChannel
     m_xa_current_file_number = m_last_sector_subheader.file_number;
     m_xa_current_channel_number = m_last_sector_subheader.channel_number;
     m_xa_current_set = true;
-    ResetXAResampler();
+    ResetAudioDecoder();
   }
   else if (m_last_sector_subheader.file_number != m_xa_current_file_number ||
            m_last_sector_subheader.channel_number != m_xa_current_channel_number)
@@ -2204,33 +2215,17 @@ void CDROM::ProcessXAADPCMSector(const u8* raw_sector, const CDImage::SubChannel
   {
     const u32 num_samples = m_last_sector_subheader.codinginfo.GetSamplesPerSector() / 2;
     if (m_last_sector_subheader.codinginfo.IsHalfSampleRate())
-    {
+      ResampleXAADPCM<true, true>(sample_buffer.data(), num_samples);
-      ResampleXAADPCM<true, true>(sample_buffer.data(), num_samples, m_spu, m_xa_resample_ring_buffer[0].data(),
-                                  m_xa_resample_ring_buffer[1].data(), &m_xa_resample_p, &m_xa_resample_sixstep,
-                                  m_cd_audio_volume_matrix);
-    }
     else
-    {
+      ResampleXAADPCM<true, false>(sample_buffer.data(), num_samples);
-      ResampleXAADPCM<true, false>(sample_buffer.data(), num_samples, m_spu, m_xa_resample_ring_buffer[0].data(),
-                                   m_xa_resample_ring_buffer[1].data(), &m_xa_resample_p, &m_xa_resample_sixstep,
-                                   m_cd_audio_volume_matrix);
-    }
   }
   else
   {
     const u32 num_samples = m_last_sector_subheader.codinginfo.GetSamplesPerSector();
     if (m_last_sector_subheader.codinginfo.IsHalfSampleRate())
-    {
+      ResampleXAADPCM<false, true>(sample_buffer.data(), num_samples);
-      ResampleXAADPCM<false, true>(sample_buffer.data(), num_samples, m_spu, m_xa_resample_ring_buffer[0].data(),
-                                   m_xa_resample_ring_buffer[1].data(), &m_xa_resample_p, &m_xa_resample_sixstep,
-                                   m_cd_audio_volume_matrix);
-    }
     else
-    {
+      ResampleXAADPCM<false, false>(sample_buffer.data(), num_samples);
-      ResampleXAADPCM<false, false>(sample_buffer.data(), num_samples, m_spu, m_xa_resample_ring_buffer[0].data(),
-                                    m_xa_resample_ring_buffer[1].data(), &m_xa_resample_p, &m_xa_resample_sixstep,
-                                    m_cd_audio_volume_matrix);
-    }
   }
 }
 
@@ -2282,7 +2277,12 @@ void CDROM::ProcessCDDASector(const u8* raw_sector, const CDImage::SubChannelQ&
 
   constexpr bool is_stereo = true;
   constexpr u32 num_samples = CDImage::RAW_SECTOR_SIZE / sizeof(s16) / (is_stereo ? 2 : 1);
-  m_spu->EnsureCDAudioSpace(num_samples);
+  const u32 remaining_space = m_audio_fifo.GetSpace();
+  if (remaining_space < num_samples)
+  {
+    Log_WarningPrintf("Dropping %u frames from audio FIFO", num_samples - remaining_space);
+    m_audio_fifo.Remove(num_samples - remaining_space);
+  }
 
   const u8* sector_ptr = raw_sector;
   for (u32 i = 0; i < num_samples; i++)
@@ -2296,7 +2296,7 @@ void CDROM::ProcessCDDASector(const u8* raw_sector, const CDImage::SubChannelQ&
                                     ApplyVolume(samp_right, m_cd_audio_volume_matrix[1][0]));
     const s16 right = SaturateVolume(ApplyVolume(samp_left, m_cd_audio_volume_matrix[0][1]) +
                                      ApplyVolume(samp_right, m_cd_audio_volume_matrix[1][1]));
-    m_spu->AddCDAudioSample(left, right);
+    AddCDAudioFrame(left, right);
   }
 }
 

src/core/cdrom.h

@@ -8,6 +8,7 @@
 #include "types.h"
 #include <array>
 #include <string>
+#include <tuple>
 #include <vector>
 
 class StateWrapper;
@@ -44,6 +45,13 @@ public:
 
   void SetUseReadThread(bool enabled);
 
+  /// Reads a frame from the audio FIFO, used by the SPU.
+  ALWAYS_INLINE std::tuple<s16, s16> GetAudioFrame()
+  {
+    const u32 frame = m_audio_fifo.IsEmpty() ? 0u : m_audio_fifo.Pop();
+    return std::tuple<s16, s16>(static_cast<s16>(frame), static_cast<s16>(frame >> 16));
+  }
+
 private:
   enum : u32
   {
@@ -59,6 +67,8 @@ private:
     RESPONSE_FIFO_SIZE = 16,
     DATA_FIFO_SIZE = RAW_SECTOR_OUTPUT_SIZE,
     NUM_SECTOR_BUFFERS = 8,
+    AUDIO_FIFO_SIZE = 44100 * 2,
+    AUDIO_FIFO_LOW_WATERMARK = 5,
 
     BASE_RESET_TICKS = 400000,
   };
@@ -199,20 +209,25 @@ private:
 
   void SoftReset();
 
-  bool IsDriveIdle() const { return m_drive_state == DriveState::Idle; }
+  ALWAYS_INLINE bool IsDriveIdle() const { return m_drive_state == DriveState::Idle; }
-  bool IsSeeking() const
+  ALWAYS_INLINE bool IsSeeking() const
   {
     return (m_drive_state == DriveState::SeekingLogical || m_drive_state == DriveState::SeekingPhysical ||
             m_drive_state == DriveState::Resetting);
   }
-  bool IsReadingOrPlaying() const
+  ALWAYS_INLINE bool IsReadingOrPlaying() const
   {
     return (m_drive_state == DriveState::Reading || m_drive_state == DriveState::Playing);
   }
-  bool CanReadMedia() const { return (m_drive_state != DriveState::ShellOpening && m_reader.HasMedia()); }
+  ALWAYS_INLINE bool CanReadMedia() const { return (m_drive_state != DriveState::ShellOpening && m_reader.HasMedia()); }
-  bool HasPendingCommand() const { return m_command != Command::None; }
+  ALWAYS_INLINE bool HasPendingCommand() const { return m_command != Command::None; }
-  bool HasPendingInterrupt() const { return m_interrupt_flag_register != 0; }
+  ALWAYS_INLINE bool HasPendingInterrupt() const { return m_interrupt_flag_register != 0; }
-  bool HasPendingAsyncInterrupt() const { return m_pending_async_interrupt != 0; }
+  ALWAYS_INLINE bool HasPendingAsyncInterrupt() const { return m_pending_async_interrupt != 0; }
+  ALWAYS_INLINE void AddCDAudioFrame(s16 left, s16 right)
+  {
+    m_audio_fifo.Push(ZeroExtend32(static_cast<u16>(left)) | (ZeroExtend32(static_cast<u16>(right)) << 16));
+  }
+
   void SetInterrupt(Interrupt interrupt);
   void SetAsyncInterrupt(Interrupt interrupt);
   void ClearAsyncInterrupt();
@@ -254,10 +269,13 @@ private:
   void BeginSeeking(bool logical, bool read_after_seek, bool play_after_seek);
   void UpdatePositionWhileSeeking();
   void ResetCurrentXAFile();
-  void ResetXAResampler();
+  void ResetAudioDecoder();
   void LoadDataFIFO();
   void ClearSectorBuffers();
 
+  template<bool STEREO, bool SAMPLE_RATE>
+  void ResampleXAADPCM(const s16* frames_in, u32 num_frames_in);
+
   System* m_system = nullptr;
   DMA* m_dma = nullptr;
   InterruptController* m_interrupt_controller = nullptr;
@@ -327,4 +345,7 @@ private:
   std::array<SectorBuffer, NUM_SECTOR_BUFFERS> m_sector_buffers;
 
   CDROMAsyncReader m_reader;
+
+  // two 16-bit samples packed in 32-bits
+  InlineFIFOQueue<u32, AUDIO_FIFO_SIZE> m_audio_fifo;
 };

src/core/save_state_version.h

@@ -2,7 +2,7 @@
 #include "types.h"
 
 static constexpr u32 SAVE_STATE_MAGIC = 0x43435544;
-static constexpr u32 SAVE_STATE_VERSION = 38;
+static constexpr u32 SAVE_STATE_VERSION = 39;
 
 #pragma pack(push, 4)
 struct SAVE_STATE_HEADER

src/core/spu.cpp

@@ -1,4 +1,5 @@
 #include "spu.h"
+#include "cdrom.h"
 #include "common/audio_stream.h"
 #include "common/log.h"
 #include "common/state_wrapper.h"
@@ -14,10 +15,11 @@ SPU::SPU() = default;
 
 SPU::~SPU() = default;
 
-void SPU::Initialize(System* system, DMA* dma, InterruptController* interrupt_controller)
+void SPU::Initialize(System* system, DMA* dma, CDROM* cdrom, InterruptController* interrupt_controller)
 {
   m_system = system;
   m_dma = dma;
+  m_cdrom = cdrom;
   m_interrupt_controller = interrupt_controller;
   m_tick_event = m_system->CreateTimingEvent("SPU Sample", SYSCLK_TICKS_PER_SPU_TICK, SYSCLK_TICKS_PER_SPU_TICK,
                                              std::bind(&SPU::Execute, this, std::placeholders::_1), false);
@@ -80,7 +82,6 @@ void SPU::Reset()
 
   m_transfer_fifo.Clear();
   m_ram.fill(0);
-  m_cd_audio_buffer.Clear();
   UpdateEventInterval();
 }
 
@@ -143,7 +144,6 @@ bool SPU::DoState(StateWrapper& sw)
 
   sw.Do(&m_transfer_fifo);
   sw.DoBytes(m_ram.data(), RAM_SIZE);
-  sw.Do(&m_cd_audio_buffer);
 
   if (sw.IsReading())
   {
@@ -730,32 +730,21 @@ void SPU::Execute(TickCount ticks)
       UpdateNoise();
 
       // Mix in CD audio.
-      s16 cd_audio_left;
+      const auto [cd_audio_left, cd_audio_right] = m_cdrom->GetAudioFrame();
-      s16 cd_audio_right;
+      if (m_SPUCNT.cd_audio_enable)
-      if (!m_cd_audio_buffer.IsEmpty())
       {
-        cd_audio_left = m_cd_audio_buffer.Pop();
+        const s32 cd_audio_volume_left = ApplyVolume(s32(cd_audio_left), m_cd_audio_volume_left);
-        cd_audio_right = m_cd_audio_buffer.Pop();
+        const s32 cd_audio_volume_right = ApplyVolume(s32(cd_audio_right), m_cd_audio_volume_right);
-        if (m_SPUCNT.cd_audio_enable)
+
+        left_sum += cd_audio_volume_left;
+        right_sum += cd_audio_volume_right;
+
+        if (m_SPUCNT.cd_audio_reverb)
         {
-          const s32 cd_audio_volume_left = ApplyVolume(s32(cd_audio_left), m_cd_audio_volume_left);
+          reverb_in_left += cd_audio_volume_left;
-          const s32 cd_audio_volume_right = ApplyVolume(s32(cd_audio_right), m_cd_audio_volume_right);
+          reverb_in_right += cd_audio_volume_right;
-
-          left_sum += cd_audio_volume_left;
-          right_sum += cd_audio_volume_right;
-
-          if (m_SPUCNT.cd_audio_reverb)
-          {
-            reverb_in_left += cd_audio_volume_left;
-            reverb_in_right += cd_audio_volume_right;
-          }
         }
       }
-      else
-      {
-        cd_audio_left = 0;
-        cd_audio_right = 0;
-      }
 
       // Compute reverb.
       s32 reverb_out_left, reverb_out_right;
@@ -1748,19 +1737,6 @@ void SPU::ProcessReverb(s16 left_in, s16 right_in, s32* left_out, s32* right_out
   s_last_reverb_output[1] = *right_out = ApplyVolume(out[1], m_reverb_registers.vROUT);
 }
 
-void SPU::EnsureCDAudioSpace(u32 remaining_frames)
-{
-  if (m_cd_audio_buffer.GetSpace() >= (remaining_frames * 2))
-    return;
-
-  const u32 frames_to_drop = (remaining_frames * 2) - m_cd_audio_buffer.GetSpace();
-  Log_WarningPrintf(
-    "SPU CD Audio buffer overflow with %d pending ticks - writing %u frames with %u frames space. Dropping %u frames.",
-    m_tick_event->IsActive() ? (m_tick_event->GetTicksSinceLastExecution() / SYSCLK_TICKS_PER_SPU_TICK) : 0,
-    remaining_frames, m_cd_audio_buffer.GetSpace() / 2, frames_to_drop);
-  m_cd_audio_buffer.Remove(frames_to_drop);
-}
-
 void SPU::DrawDebugStateWindow()
 {
   static const ImVec4 active_color{1.0f, 1.0f, 1.0f, 1.0f};

src/core/spu.h

@@ -14,6 +14,7 @@ class WAVWriter;
 class System;
 class TimingEvent;
 class DMA;
+class CDROM;
 class InterruptController;
 
 class SPU
@@ -22,7 +23,7 @@ public:
   SPU();
   ~SPU();
 
-  void Initialize(System* system, DMA* dma, InterruptController* interrupt_controller);
+  void Initialize(System* system, DMA* dma, CDROM* cdrom, InterruptController* interrupt_controller);
   void Reset();
   bool DoState(StateWrapper& sw);
 
@@ -35,14 +36,6 @@ public:
   // Render statistics debug window.
   void DrawDebugStateWindow();
 
-  // External input from CD controller.
-  void AddCDAudioSample(s16 left, s16 right)
-  {
-    m_cd_audio_buffer.Push(left);
-    m_cd_audio_buffer.Push(right);
-  }
-  void EnsureCDAudioSpace(u32 num_samples);
-
   // Executes the SPU, generating any pending samples.
   void GeneratePendingSamples();
 
@@ -67,7 +60,6 @@ private:
   static constexpr u32 SYSCLK_TICKS_PER_SPU_TICK = MASTER_CLOCK / SAMPLE_RATE; // 0x300
   static constexpr s16 ENVELOPE_MIN_VOLUME = 0;
   static constexpr s16 ENVELOPE_MAX_VOLUME = 0x7FFF;
-  static constexpr u32 CD_AUDIO_SAMPLE_BUFFER_SIZE = 44100 * 2;
   static constexpr u32 CAPTURE_BUFFER_SIZE_PER_CHANNEL = 0x400;
   static constexpr u32 MINIMUM_TICKS_BETWEEN_KEY_ON_OFF = 2;
   static constexpr u32 NUM_REVERB_REGS = 32;
@@ -331,10 +323,7 @@ private:
     };
   };
 
-  static constexpr s32 Clamp16(s32 value)
+  static constexpr s32 Clamp16(s32 value) { return (value < -0x8000) ? -0x8000 : (value > 0x7FFF) ? 0x7FFF : value; }
-  {
-    return (value < -0x8000) ? -0x8000 : (value > 0x7FFF) ? 0x7FFF : value;
-  }
 
   static constexpr s32 ApplyVolume(s32 sample, s16 volume) { return (sample * s32(volume)) >> 15; }
 
@@ -382,6 +371,7 @@ private:
 
   System* m_system = nullptr;
   DMA* m_dma = nullptr;
+  CDROM* m_cdrom = nullptr;
   InterruptController* m_interrupt_controller = nullptr;
   std::unique_ptr<TimingEvent> m_tick_event;
   std::unique_ptr<TimingEvent> m_transfer_event;
@@ -431,6 +421,4 @@ private:
   InlineFIFOQueue<u16, FIFO_SIZE_IN_HALFWORDS> m_transfer_fifo;
 
   std::array<u8, RAM_SIZE> m_ram{};
-
-  InlineFIFOQueue<s16, CD_AUDIO_SAMPLE_BUFFER_SIZE> m_cd_audio_buffer;
 };

src/core/system.cpp

@@ -268,7 +268,7 @@ bool System::InitializeComponents(bool force_software_renderer)
   m_cdrom->Initialize(this, m_dma.get(), m_interrupt_controller.get(), m_spu.get());
   m_pad->Initialize(this, m_interrupt_controller.get());
   m_timers->Initialize(this, m_interrupt_controller.get(), m_gpu.get());
-  m_spu->Initialize(this, m_dma.get(), m_interrupt_controller.get());
+  m_spu->Initialize(this, m_dma.get(), m_cdrom.get(), m_interrupt_controller.get());
   m_mdec->Initialize(this, m_dma.get());
 
   // load settings