Sms_Apu.cpp: Assert of "end_time >= last_time" fails with one VGM file

[Wohlstand]

Original report by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).

---

I have got a file that makes a crash of the library in the void Sms_Apu::run_until( blip_time_t end_time ) call:

• end_time in a moment is equal to -9276 (why it's negative? because of to_blip_time()!!!)
• last_time is equal to 0.

Song didn't began to play: it caused a crash on attempt to start_track().

[Wohlstand]

Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).

---

Okay, I have researched a problem, and I have found that this VGM has too high PSG clock value ( 3224297472 which is 00 E0 2E C0 in raw bytes). I made an ugly workaround in Vgm_Emu.cpp to avoid the crash:

I think, here it must have a better fix than this ugly crap…

[Wohlstand]

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

---

Do you think it's possible that the most-significant byte (which I'm assuming is C0 here) is meant to be masked to zero by actual hardware? 00 2E E0 00 is equal to 3072000 which is very close indeed to the 3079545 you have here in your workaround.

[Wohlstand]

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

---

diff --git a/gme/Vgm_Emu.cpp b/gme/Vgm_Emu.cpp
index 8f19b7d..33f8633 100644
--- a/gme/Vgm_Emu.cpp
+++ b/gme/Vgm_Emu.cpp
@@ -303,7 +303,7 @@ blargg_err_t Vgm_Emu::load_mem_( byte const* new_data, long new_size )
        check( get_le32( h.version ) <= 0x150 );
        
        // psg rate
-       psg_rate = get_le32( h.psg_rate );
+       psg_rate = get_le32( h.psg_rate ) & 0x00FFFFFFu;
        if ( !psg_rate )
                psg_rate = 3579545;
        blip_buf.clock_rate( psg_rate );

This patch seems to fix the file you’ve provided without breaking the only other VGM file I’m personally familiar with.

[Wohlstand]

Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).

---

Then, it’s a question: what greatest byte means here? 🤔
Anyway, I see the note here: https://vgmrips.net/wiki/VGM_Specification

Bit 31 (0x80000000) is used on combination with the dual-chip-bit to indicate that this is a T6W28. (PSG variant used in Neo Geo Pocket)

So, yeah, these two last bits ast should be cut-off, and we need to use something to give correct T6W28 thing as yeah, this VGM is the song from a Neo Geo Pocket game. So, I guess, a mask would be 0x3FFFFFFFu, but, yeah, keep this 0x00FFFFFFu as it safer and should never lead a crash, otherwise, I’ll try to set `3FFFFFFF` clock value for a crash test, will it happen or not 🤔

[Wohlstand]

Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).

---

Ok, 3F makes crash, however, 0F (`0x0FFFFFFF`) doesn't, so, feel free to use 0x0FFFFFFF mask :fox:

[Wohlstand]

Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).

---

So, this issue: #33/vgm-dual-chips-support and it’s related pull-request does fixes this.

[Wohlstand]

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

---

OK, so I’ll track this together with your PR for dual chips support and close this once we land that.

[Wohlstand]

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

---

Dual chip support has been merged and addresses this issue, closing