989snd: sound bank loading overhaul

common/util/BinaryReader.h

@@ -12,30 +12,33 @@
 #include "common/common_types.h"
 #include "common/util/Assert.h"
 
+#include "third-party/span.hpp"
+
 class BinaryReader {
  public:
-  explicit BinaryReader(const std::vector<uint8_t>& _buffer) : m_buffer(_buffer) {}
+  explicit BinaryReader(nonstd::span<const uint8_t> _span) : m_span(_span) {}
 
   template <typename T>
   T read() {
-    ASSERT(m_seek + sizeof(T) <= m_buffer.size());
+    ASSERT(m_seek + sizeof(T) <= m_span.size());
     T obj;
-    memcpy(&obj, m_buffer.data() + m_seek, sizeof(T));
+    memcpy(&obj, m_span.data() + m_seek, sizeof(T));
     m_seek += sizeof(T);
     return obj;
   }
 
   void ffwd(int amount) {
     m_seek += amount;
-    ASSERT(m_seek <= m_buffer.size());
+    ASSERT(m_seek <= m_span.size());
   }
 
-  uint32_t bytes_left() const { return m_buffer.size() - m_seek; }
-  uint8_t* here() { return m_buffer.data() + m_seek; }
+  uint32_t bytes_left() const { return m_span.size() - m_seek; }
+  const uint8_t* here() { return m_span.data() + m_seek; }
+  BinaryReader at(u32 pos) { return BinaryReader(m_span.subspan(pos)); }
   uint32_t get_seek() const { return m_seek; }
   void set_seek(u32 seek) { m_seek = seek; }
 
  private:
-  std::vector<u8> m_buffer;
+  nonstd::span<const u8> m_span;
   uint32_t m_seek = 0;
 };

common/util/dgo_util.cpp

@@ -21,7 +21,7 @@ void assert_string_empty_after(const char* str, int size) {
   }
 }
 
-std::string get_object_file_name(const std::string& original_name, u8* data, int size) {
+std::string get_object_file_name(const std::string& original_name, const u8* data, int size) {
   const std::string art_group_text_strings[] = {
       fmt::format("/src/next/data/art-group{}/", versions::jak1::ART_FILE_VERSION),
       fmt::format("/src/jak2/final/art-group{}/", versions::jak2::ART_FILE_VERSION),

common/util/dgo_util.h

@@ -6,4 +6,4 @@
 #include "common/versions/versions.h"
 
 void assert_string_empty_after(const char* str, int size);
-std::string get_object_file_name(const std::string& original_name, u8* data, int size);
+std::string get_object_file_name(const std::string& original_name, const u8* data, int size);

game/overlord/common/sbank.cpp

@@ -19,45 +19,54 @@ SoundBank* gBanks[N_BANKS] = {&gCommonBank,    &gGunBank,       &gBoardBank,
                               &gLevelBanks[0], &gLevelBanks[1], &gLevelBanks[2]};
 
 void sbank_init_globals() {
-  memset((void*)&gCommonBank, 0, sizeof(gCommonBank));
-  memset((void*)&gGunBank, 0, sizeof(gGunBank));
-  memset((void*)&gBoardBank, 0, sizeof(gBoardBank));
-  memset((void*)&gLevelBanks, 0, sizeof(gLevelBanks));
+  gCommonBank = {};
+  gGunBank = {};
+  gBoardBank = {};
+  for (auto& b : gLevelBanks) {
+    b = {};
+  }
 }
 
 void InitBanks() {
   for (auto bank : gBanks) {
     bank->bank_handle = 0;
     bank->sound_count = 0;
 
-    bank->in_use = false;
-    bank->unk4 = 0;
+    // paper over version differences
+    // in_use doesn't exist in jak1, but setting it to true
+    // for all banks results in the same behaviour
+    if (g_game_version == GameVersion::Jak1) {
+      bank->in_use = true;
+    } else {
+      bank->in_use = false;
+    }
 
-    strcpy(bank->name, "<unused>");
+    bank->unk4 = 0;
+    strcpy(bank->name.data(), "<unused>");
   }
 
   if (g_game_version == GameVersion::Jak2) {
-    strncpy(gBanks[0]->name, "common", 16);
+    strncpy(gBanks[0]->name.data(), "common", 16);
     gBanks[0]->spu_loc = 0x20000;
     gBanks[0]->spu_size = 0xAFCC0;
 
-    strncpy(gBanks[1]->name, "gun", 16);
+    strncpy(gBanks[1]->name.data(), "gun", 16);
     gBanks[1]->spu_loc = 0x131740;
     gBanks[1]->spu_size = 0;
 
-    strncpy(gBanks[2]->name, "board", 16);
+    strncpy(gBanks[2]->name.data(), "board", 16);
     gBanks[2]->spu_loc = 0x131740;
     gBanks[2]->spu_size = 0;
 
-    strncpy(gBanks[3]->name, "level0", 16);
+    strncpy(gBanks[3]->name.data(), "level0", 16);
     gBanks[3]->spu_loc = 0x131740;
     gBanks[3]->spu_size = 0x42800;
 
-    strncpy(gBanks[4]->name, "level1", 16);
+    strncpy(gBanks[4]->name.data(), "level1", 16);
     gBanks[4]->spu_loc = 0x173f40;
     gBanks[4]->spu_size = 0x42800;
 
-    strncpy(gBanks[5]->name, "level2", 16);
+    strncpy(gBanks[5]->name.data(), "level2", 16);
     gBanks[5]->spu_loc = 0x1B6740;
     gBanks[5]->spu_size = 0x42800;
   }
@@ -111,7 +120,7 @@ s32 LookupSoundIndex(const char* name, SoundBank** bank_out) {
     }
 
     for (int i = 0; i < (int)bank->sound_count; i++) {
-      if (memcmp(bank->sound[i].name, name, 16) == 0) {
+      if (memcmp(bank->sound[i].name.data(), name, 16) == 0) {
         *bank_out = bank;
         return i;
       }
@@ -129,16 +138,9 @@ SoundBank* LookupBank(const char* name) {
       return nullptr;  // not found.
     }
     auto bank = gBanks[idx];
-    // they had some weird stuff here that took advantage of the fact that this region was
-    // 16-byte aligned, so it probably wasn't a memcmp, but this is easier.
-    if (g_game_version == GameVersion::Jak1) {
-      if ((memcmp(bank->name, name, 16) == 0)) {
-        return bank;
-      }
-    } else {
-      if ((memcmp(bank->name, name, 16) == 0) && bank->in_use) {
-        return bank;
-      }
+
+    if ((memcmp(bank->name.data(), name, 16) == 0) && bank->in_use) {
+      return bank;
     }
     idx--;
   }

game/overlord/common/sbank.h

@@ -1,31 +1,31 @@
 #pragma once
 
+#include <array>
+#include <string>
+#include <vector>
+
 #include "common/common_types.h"
 
+#include "game/sound/sndshim.h"
+
 struct SoundRecord {
-  char name[16];
+  std::array<char, 16> name;
   u32 fallof_params;
 };
 
 struct SoundBank {
-  char name[16];
-  u32 bank_handle;
+  std::array<char, 16> name;
+  snd::BankHandle bank_handle;
   u32 sound_count;
 
-  union {
-    SoundRecord sound[1];
-
-    // Needs to fit the biggest bank (common.sbk)
-    u8 buffer[10 * 2048];
+  // name list, only for jak1
+  std::vector<SoundRecord> sound;
 
-    // Jak 2 additions go here
-    struct {
-      u32 spu_loc;
-      u32 spu_size;
-      u32 unk4;
-      bool in_use;
-    };
-  };
+  // jak2 additions
+  u32 spu_loc;
+  u32 spu_size;
+  u32 unk4;
+  bool in_use;
 };
 
 void sbank_init_globals();

game/overlord/common/soundcommon.cpp

@@ -33,10 +33,10 @@ void PrintBankInfo(SoundBank* bank) {
   // we dont need this and it spams the console too much
   return;
 
-  printf("Bank %s\n\n", bank->name);
+  printf("Bank %s\n\n", bank->name.data());
   for (u32 i = 0; i < bank->sound_count; i++) {
     // Some characters use the full 16 characters (bonelurker-grunt) and dont have a null terminator
-    std::string name = std::string(bank->sound[i].name, 16);
+    std::string name = std::string(bank->sound[i].name.data(), 16);
     printf("%d : %16s : min %d max %d curve %d\n", i, name.c_str(),
            bank->sound[i].fallof_params & 0x3fff, (bank->sound[i].fallof_params >> 14) & 0x3fff,
            bank->sound[i].fallof_params >> 28);

game/overlord/common/srpc.cpp

@@ -1,13 +1,14 @@
 #include "srpc.h"
 
 #include <cstring>
+#include "game/sound/sndshim.h"
 
 // added
 u32 gMusicFadeHack = 0;
 MusicTweaks gMusicTweakInfo;
 s32 gMusicTweak = 0x80;
 int32_t gSoundEnable = 1;
-s32 gMusic = 0;
+snd::BankHandle gMusic = nullptr;
 s32 gMusicPause = 0;
 s32 gSoundInUse = 0;
 u8 gFPS = 60;

game/overlord/common/srpc.h

@@ -117,10 +117,10 @@ struct SoundRpcSetMirrror {
 extern s32 gMusicTweak;
 extern MusicTweaks gMusicTweakInfo;
 extern int32_t gSoundEnable;
-extern s32 gMusic;
+extern snd::BankHandle gMusic;
 extern s32 gMusicPause;
 extern s32 gSoundInUse;
 extern u8 gFPS;
 extern const char* gLanguage;
 extern u32 gFrameNum;
-void srpc_init_globals();
+void srpc_init_globals();

game/overlord/common/ssound.cpp

@@ -395,12 +395,12 @@ void PrintActiveSounds() {
   for (auto& s : gSounds) {
     if (s.id != 0 && s.is_music == 0) {
       if (s.bank_entry != nullptr) {
-        u32 len = strlen(s.bank_entry->name);
+        u32 len = strlen(s.bank_entry->name.data());
         if (len > 16) {
           len = 16;
         }
         sprintf(string, "                 : Vol %d", GetVolume(&s));
-        memcpy(string, s.bank_entry->name, len);
+        memcpy(string, s.bank_entry->name.data(), len);
         printf("%s\n", string);
       } else {  // added for printing jak2 sounds
         u32 len = strlen(s.name);

game/overlord/jak1/fake_iso.cpp

@@ -2,6 +2,7 @@
 
 #include "common/log/log.h"
 #include "common/util/Assert.h"
+#include "common/util/BinaryReader.h"
 #include "common/util/FileUtil.h"
 
 #include "game/overlord/common/fake_iso.h"
@@ -18,7 +19,7 @@ LoadStackEntry* FS_OpenWad(FileRecord* fr, int32_t offset);
 uint32_t FS_LoadSoundBank(char* name, SoundBank* bank);
 void FS_PollDrive();
 uint32_t FS_SyncRead();
-uint32_t FS_LoadMusic(char* name, s32* bank_handle);
+uint32_t FS_LoadMusic(char* name, snd::BankHandle* bank_handle);
 void FS_Close(LoadStackEntry* fd);
 static LoadStackEntry sLoadStack[MAX_OPEN_FILES];  //! List of all files that are "open"
 static LoadStackEntry* sReadInfo;                  // LoadStackEntry for currently reading file
@@ -200,7 +201,7 @@ uint32_t FS_SyncRead() {
  */
 void FS_PollDrive() {}
 
-uint32_t FS_LoadMusic(char* name, s32* bank_handle) {
+uint32_t FS_LoadMusic(char* name, snd::BankHandle* bank_handle) {
   char namebuf[16];
   strcpy(namebuf, name);
   namebuf[8] = 0;
@@ -215,9 +216,27 @@ uint32_t FS_LoadMusic(char* name, s32* bank_handle) {
   return 0;
 }
 
+// original overlord uses good old fread into a struct instance
+// lets use BinaryReader instead
+static void parseSoundBank(BinaryReader data, SoundBank& bank) {
+  bank.name = data.read<std::array<char, 16>>();
+
+  data.read<u32>();
+  bank.bank_handle = nullptr;
+
+  bank.sound_count = data.read<u32>();
+  bank.sound.resize(bank.sound_count);
+
+  for (auto& snd : bank.sound) {
+    snd.name = data.read<std::array<char, 16>>();
+    snd.fallof_params = data.read<u32>();
+  }
+}
+
 uint32_t FS_LoadSoundBank(char* name, SoundBank* bank) {
   char namebuf[16];
 
+  // sector size of sound name list
   int offset = 10 * 2048;
   if (bank->sound_count == 101) {
     offset = 1 * 2048;
@@ -234,11 +253,14 @@ uint32_t FS_LoadSoundBank(char* name, SoundBank* bank) {
       return 0;
   }
 
-  auto fp = file_util::open_file(get_file_path(file), "rb");
-  fread(bank, offset, 1, fp);
-  fclose(fp);
+  // auto fp = file_util::open_file(get_file_path(file), "rb");
+  // fread(bank, offset, 1, fp);
+  // fclose(fp);
+
+  auto data = file_util::read_binary_file(fs::path(get_file_path(file)));
+  parseSoundBank(BinaryReader(data), *bank);
 
-  s32 handle = snd_BankLoadEx(get_file_path(file), offset, 0, 0);
+  snd::BankHandle handle = snd_BankLoadEx(get_file_path(file), offset, 0, 0);
   snd_ResolveBankXREFS();
   PrintBankInfo(bank);
   bank->bank_handle = handle;

game/overlord/jak1/iso_api.cpp

@@ -31,7 +31,7 @@ void LoadSoundBank(const char* bank_name, SoundBank* bank) {
   SleepThread();  // wait for finish.
 }
 
-void LoadMusic(const char* music_name, s32* bank) {
+void LoadMusic(const char* music_name, snd::BankHandle* bank) {
   ASSERT(strlen(music_name) < 16);
   MusicLoadCommand cmd;
   cmd.cmd_id = LOAD_MUSIC;
@@ -203,4 +203,4 @@ s32 LoadISOFileChunkToEE(FileRecord* file, uint32_t dest_addr, uint32_t length,
   return cmd.length_to_copy;
 }
 
-}  // namespace jak1
+}  // namespace jak1

game/overlord/jak1/iso_api.h

@@ -7,7 +7,7 @@ struct SoundBank;
 namespace jak1 {
 struct VagDirEntry;
 void LoadSoundBank(const char* bank_name, SoundBank* bank);
-void LoadMusic(const char* music_name, s32* bank);
+void LoadMusic(const char* music_name, snd::BankHandle* bank);
 
 void QueueVAGStream(FileRecord* file, VagDirEntry* vag, u32 sound_id, u32 unk);
 void PlayVAGStream(FileRecord* file,

game/overlord/jak1/isocommon.h

@@ -58,7 +58,7 @@ struct IsoFs {
   uint32_t (*begin_read)(LoadStackEntry*, void*, int32_t);  // 1c
   uint32_t (*sync_read)();                                  // 20
   uint32_t (*load_sound_bank)(char*, SoundBank*);           // 24
-  uint32_t (*load_music)(char*, s32*);
+  uint32_t (*load_music)(char*, snd::BankHandle*);
   void (*poll_drive)();
 };
 
@@ -173,7 +173,7 @@ struct SoundBankLoadCommand : public IsoMessage {
 
 struct MusicLoadCommand : public IsoMessage {
   char music_name[16];
-  s32* music_handle;
+  snd::BankHandle* music_handle;
 };
 
 /*!

game/overlord/jak1/srpc.cpp

@@ -374,7 +374,7 @@ void* RPC_Loader(unsigned int /*fno*/, void* data, int size) {
         case Jak1SoundCommand::UNLOAD_BANK: {
           SoundBank* bank = LookupBank(cmd->load_bank.bank_name);
           if (bank != nullptr) {
-            s32 id = bank->bank_handle;
+            snd::BankHandle id = bank->bank_handle;
             bank->bank_handle = 0;
             snd_UnloadBank(id);
             snd_ResolveBankXREFS();
@@ -403,7 +403,7 @@ void* RPC_Loader(unsigned int /*fno*/, void* data, int size) {
             }
             snd_UnloadBank(gMusic);
             snd_ResolveBankXREFS();
-            gMusic = 0;
+            gMusic = nullptr;
           }
           LoadMusic(cmd->load_bank.bank_name, &gMusic);
           SignalSema(gSema);
@@ -508,4 +508,4 @@ s32 VBlank_Handler(void*) {
 
   return 1;
 }
-}  // namespace jak1
+}  // namespace jak1