From e9e87a1c3c74e503d00e5166f91f0b4c81c66e07 Mon Sep 17 00:00:00 2001
From: Kae <80987908+Novaenia@users.noreply.github.com>
Date: Mon, 29 Jul 2024 09:23:27 +1000
Subject: [PATCH] Avoid crashing when a OGG file is broken (thanks to
 @kblaschke !)

Also added a name tag to Audio for logging so that it's easier to find the audio asset that's causing it
---
 source/base/StarAssets.cpp |  2 +-
 source/base/StarMixer.cpp  | 83 ++++++++++++++++++++------------------
 source/core/StarAudio.cpp  | 21 +++++++---
 source/core/StarAudio.hpp  |  6 ++-
 4 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/source/base/StarAssets.cpp b/source/base/StarAssets.cpp
index a56abd8..c47b71c 100644
--- a/source/base/StarAssets.cpp
+++ b/source/base/StarAssets.cpp
@@ -1241,7 +1241,7 @@ shared_ptr<Assets::AssetData> Assets::loadImage(AssetPath const& path) const {
 shared_ptr<Assets::AssetData> Assets::loadAudio(AssetPath const& path) const {
   return unlockDuring([&]() {
     auto newData = make_shared<AudioData>();
-    newData->audio = make_shared<Audio>(open(path.basePath));
+    newData->audio = make_shared<Audio>(open(path.basePath), path.basePath);
     newData->needsPostProcessing = newData->audio->compressed();
     return newData;
   });
diff --git a/source/base/StarMixer.cpp b/source/base/StarMixer.cpp
index 72a6cd6..bbcb23a 100644
--- a/source/base/StarMixer.cpp
+++ b/source/base/StarMixer.cpp
@@ -310,49 +310,54 @@ void Mixer::read(int16_t* outBuffer, size_t frameCount, ExtraMixFunction extraMi
           m_mixBuffer[i] = 0;
         ramt += silentSamples * channels;
       }
-      ramt += audioInstance->m_audio.resample(channels, sampleRate, m_mixBuffer.ptr() + ramt, bufferSize - ramt, pitchMultiplier);
-      while (ramt != bufferSize && !finished) {
-        // Only seek back to the beginning and read more data if loops is < 0
-        // (loop forever), or we have more loops to go, otherwise, the sample is
-        // finished.
-        if (audioInstance->m_loops != 0) {
-          audioInstance->m_audio.seekSample(0);
-          ramt += audioInstance->m_audio.resample(channels, sampleRate, m_mixBuffer.ptr() + ramt, bufferSize - ramt, pitchMultiplier);
-          if (audioInstance->m_loops > 0)
-            --audioInstance->m_loops;
-        } else {
-          finished = true;
-        }
-      }
-      if (audioInstance->m_clockStop && *audioInstance->m_clockStop < sampleEndTime) {
-        for (size_t s = 0; s < ramt / channels; ++s) {
-          unsigned millisecondsInBuffer = (s * 1000) / sampleRate;
-          auto sampleTime = sampleStartTime + millisecondsInBuffer;
-          if (sampleTime > *audioInstance->m_clockStop) {
-            float volume = 0.0f;
-            if (audioInstance->m_clockStopFadeOut > 0)
-              volume = 1.0f - (float)(sampleTime - *audioInstance->m_clockStop) / (float)audioInstance->m_clockStopFadeOut;
-
-            if (volume <= 0) {
-              for (size_t c = 0; c < channels; ++c)
-                m_mixBuffer[s * channels + c] = 0;
-            } else {
-              for (size_t c = 0; c < channels; ++c)
-                m_mixBuffer[s * channels + c] *= volume;
-            }
+      try {
+        ramt += audioInstance->m_audio.resample(channels, sampleRate, m_mixBuffer.ptr() + ramt, bufferSize - ramt, pitchMultiplier);
+        while (ramt != bufferSize && !finished) {
+          // Only seek back to the beginning and read more data if loops is < 0
+          // (loop forever), or we have more loops to go, otherwise, the sample is
+          // finished.
+          if (audioInstance->m_loops != 0) {
+            audioInstance->m_audio.seekSample(0);
+            ramt += audioInstance->m_audio.resample(channels, sampleRate, m_mixBuffer.ptr() + ramt, bufferSize - ramt, pitchMultiplier);
+            if (audioInstance->m_loops > 0)
+              --audioInstance->m_loops;
+          } else {
+            finished = true;
           }
         }
-        if (sampleEndTime > *audioInstance->m_clockStop + audioInstance->m_clockStopFadeOut)
-          finished = true;
-      }
+        if (audioInstance->m_clockStop && *audioInstance->m_clockStop < sampleEndTime) {
+          for (size_t s = 0; s < ramt / channels; ++s) {
+            unsigned millisecondsInBuffer = (s * 1000) / sampleRate;
+            auto sampleTime = sampleStartTime + millisecondsInBuffer;
+            if (sampleTime > *audioInstance->m_clockStop) {
+              float volume = 0.0f;
+              if (audioInstance->m_clockStopFadeOut > 0)
+                volume = 1.0f - (float)(sampleTime - *audioInstance->m_clockStop) / (float)audioInstance->m_clockStopFadeOut;
 
-      for (size_t s = 0; s < ramt / channels; ++s) {
-        float vol = lerp((float)s / frameCount, beginVolume * groupVolume * audioStopVolBegin, endVolume * groupEndVolume * audioStopVolEnd);
-        for (size_t c = 0; c < channels; ++c) {
-          float sample = m_mixBuffer[s * channels + c] * vol * audioState.positionalChannelVolumes[c] * audioInstance->m_volume.value;
-          int16_t& outSample = outBuffer[s * channels + c];
-          outSample = clamp(sample + outSample, -32767.0f, 32767.0f);
+              if (volume <= 0) {
+                for (size_t c = 0; c < channels; ++c)
+                  m_mixBuffer[s * channels + c] = 0;
+              } else {
+                for (size_t c = 0; c < channels; ++c)
+                  m_mixBuffer[s * channels + c] *= volume;
+              }
+            }
+          }
+          if (sampleEndTime > *audioInstance->m_clockStop + audioInstance->m_clockStopFadeOut)
+            finished = true;
         }
+
+        for (size_t s = 0; s < ramt / channels; ++s) {
+          float vol = lerp((float)s / frameCount, beginVolume * groupVolume * audioStopVolBegin, endVolume * groupEndVolume * audioStopVolEnd);
+          for (size_t c = 0; c < channels; ++c) {
+            float sample = m_mixBuffer[s * channels + c] * vol * audioState.positionalChannelVolumes[c] * audioInstance->m_volume.value;
+            int16_t& outSample = outBuffer[s * channels + c];
+            outSample = clamp(sample + outSample, -32767.0f, 32767.0f);
+          }
+        }
+      } catch (Star::AudioException const& e) {
+        Logger::error("Error reading audio '{}': {}", audioInstance->m_audio.name(), e.what());
+        finished = true;
       }
 
       audioInstance->m_volume.value = audioStopVolEnd;
diff --git a/source/core/StarAudio.cpp b/source/core/StarAudio.cpp
index 2dfdaee..a1f2e62 100644
--- a/source/core/StarAudio.cpp
+++ b/source/core/StarAudio.cpp
@@ -239,17 +239,19 @@ public:
 
   size_t readPartial(int16_t* buffer, size_t bufferSize) {
     int bitstream;
-    int read;
+    int read = OV_HOLE;
     // ov_read takes int parameter, so do some magic here to make sure we don't
     // overflow
     bufferSize *= 2;
+    do {
 #if STAR_LITTLE_ENDIAN
-    read = ov_read(&m_vorbisFile, (char*)buffer, bufferSize, 0, 2, 1, &bitstream);
+      read = ov_read(&m_vorbisFile, (char*)buffer, bufferSize, 0, 2, 1, &bitstream);
 #else
-    read = ov_read(&m_vorbisFile, (char*)buffer, bufferSize, 1, 2, 1, &bitstream);
+      read = ov_read(&m_vorbisFile, (char*)buffer, bufferSize, 1, 2, 1, &bitstream);
 #endif
+    } while (read == OV_HOLE);
     if (read < 0)
-      throw AudioException("Error in Audio::read");
+      throw AudioException::format("Error in Audio::read ({})", read);
 
     // read in bytes, returning number of int16_t samples.
     return read / 2;
@@ -349,7 +351,8 @@ private:
   ExternalBuffer m_memoryFile;
 };
 
-Audio::Audio(IODevicePtr device) {
+Audio::Audio(IODevicePtr device, String name) {
+  m_name = name;
   if (!device->isOpen())
     device->open(IOMode::Read);
 
@@ -579,4 +582,12 @@ size_t Audio::resample(unsigned destinationChannels, unsigned destinationSampleR
   }
 }
 
+String const& Audio::name() const {
+  return m_name;
+}
+
+void Audio::setName(String name) {
+  m_name = std::move(name);
+}
+
 }
diff --git a/source/core/StarAudio.hpp b/source/core/StarAudio.hpp
index 571aaf3..443c74e 100644
--- a/source/core/StarAudio.hpp
+++ b/source/core/StarAudio.hpp
@@ -26,7 +26,7 @@ STAR_EXCEPTION(AudioException, StarException);
 // instances is not expensive.
 class Audio {
 public:
-  explicit Audio(IODevicePtr device);
+  explicit Audio(IODevicePtr device, String name = "");
   Audio(Audio const& audio);
   Audio(Audio&& audio);
 
@@ -90,12 +90,16 @@ public:
       int16_t* destinationBuffer, size_t destinationBufferSize,
       double velocity = 1.0);
 
+  String const& name() const;
+  void setName(String name);
+
 private:
   // If audio is uncompressed, this will be null.
   CompressedAudioImplPtr m_compressed;
   UncompressedAudioImplPtr m_uncompressed;
 
   ByteArray m_workingBuffer;
+  String m_name;
 };
 
 }