ValleyBell / libvgm

A more modular rewrite of most components from VGMPlay. will include sub-libraries for audio output, sound emulation and VGM playback
irc://irc.digibase.ca/#vgmrips
136 stars 33 forks source link

Issue with Street Fighter EX 2 Plus's Attract track #119

Closed b0x-Cub3d closed 2 months ago

b0x-Cub3d commented 5 months ago

The song crashes within three seconds of it playing. I noticed that it recognizes a section of the file as the clock for the Mikey chip, yet it's a 1.61 VGM file.

mmontag commented 4 months ago

See also https://vgmrips.net/forum/viewtopic.php?t=5722

yoyofr commented 2 months ago

Hello, I'm currently working on having libvgm replacing vgmplay in modizer. I've checked the issue and did fix it by adding a check in vgmplayer.cpp. Basically the file header seems wrong and as mikey was added in v1.72 you can fix it like this: if ( (_fileHdr.fileVer<0x172) && (chipType==DEVID_MIKEY) ) return 0;

ValleyBell commented 2 months ago

Ah, it looks like the VGM header is faulty and contains garbage data. (bytes 0xE4 to 0xFF, which is supposed to be chip clocks) I'll have a look at where it crashes and will try to prevent libvgm from crashing, as well as fixing the VGM itself.

yoyofr commented 2 months ago

when testing I think it was "working" for a few iteration (calling the render function) and then crashing in resampler.c: on this line CurBufR[0] = CAA->lSmpl.R; from static void Resmpl_Exec_LinearDown(RESMPL_STATE* CAA, UINT32 length, WAVE_32BS* retSample)

Resmpl_EnsureBuffers(CAA, CAA->smpNext - CAA->smpLast + 1);
CurBufL = CAA->smplBufs[0];
CurBufR = CAA->smplBufs[1];
CurBufL[0] = CAA->lSmpl.L;

--> CurBufR[0] = CAA->lSmpl.R; StreamPnt[0] = &CurBufL[1]; StreamPnt[1] = &CurBufR[1]; CAA->StreamUpdate(CAA->su_DataPtr, CAA->smpNext - CAA->smpLast, StreamPnt);

ValleyBell commented 2 months ago

fixed by 1271ab3

Cause: The garbage data causes a Mikey chip clock of 146 459 965 to be defined, which results in a 9.1 MHz chip sample rate.
The code for downsampling the 9.1 MHz chip output to 44/48 KHz render output ended up with an overflow in one of the calculations, so it tried to render a negative amount of samples, which crashed the player.

The fix (1) changes the number size to allow for about 10 MHz of input sample rate and (2) tries to catch the overflow and do a post-correction of the result, just in case.

(in short: I basically did everything but ignore the Mikey chip clock)