Audio buffer overflow

[jpcima]

Hi. When I instanciate either the DRO or S98 player, the Render method is going to overflow the output buffer and crash the program. It does not happen with VGM.

Diagnosing the problem, I discovered that it would be resolved, in a non-optimal way, when the smplStep is forced to 1.

libvgm/player/droplayer.cpp

Line 549 in 07c6cc7

Resmpl_Execute(&clDev->resmpl, smplStep, &data[curSmpl]);

Indeed, a difference between VGM and DRO/S98 is that the former always uses the step value 1.

The issue does not happen in libvgm's own player. (possibly, due to allocating sufficient buffer so it doesn't happen)
It has occurred in this context.

Also it probably doesn't matter but those were the files used: https://github.com/Wohlstand/OPL3BankEditor/tree/master/Bank_Examples/DOSBox

EDIT
These have been values logged from DROPlayer before a situation of crash.
curSmpl=0 smplCnt=7560 smplStep=7560

Valgrind trace

==18987== Invalid write of size 8
==18987==    at 0x483E817: memset (vg_replace_strmem.c:1251)
==18987==    by 0x68918A5: adlib_OPL2_getsample (adlibemu_opl_inc.c:1239)
==18987==    by 0x685C568: Resmpl_Exec_LinearDown (Resampler.c:343)
==18987==    by 0x685C568: Resmpl_Execute (Resampler.c:427)
==18987==    by 0x6850C2B: DROPlayer::Render(unsigned int, waveform_32bit_stereo*) (droplayer.cpp:549)
==18987==    by 0x684FA67: vgm_read(input_plugin_data*, char*, int) (vgm.cc:181)
==18987==    by 0x13513A: ip_read (input.c:695)
==18987==    by 0x13F4DF: _prebuffer (player.c:599)
==18987==    by 0x140D14: player_play_file (player.c:1205)
==18987==    by 0x11FEAC: browser_enter (browser.c:414)
==18987==    by 0x128145: run_command (command_mode.c:2882)
==18987==    by 0x11E7A0: u_getch (ui_curses.c:2163)
==18987==    by 0x11E7A0: main_loop (ui_curses.c:2272)
==18987==    by 0x11E7A0: main (ui_curses.c:2556)
==18987==  Address 0x53d2158 is 0 bytes after a block of size 39,768 alloc'd
==18987==    at 0x483777F: malloc (vg_replace_malloc.c:299)
==18987==    by 0x685C3E9: Resmpl_Init (Resampler.c:62)
==18987==    by 0x685151D: DROPlayer::Start() (droplayer.cpp:429)
==18987==    by 0x684FD46: vgm_open_after_map(input_plugin_data*) (vgm.cc:127)
==18987==    by 0x6850165: vgm_open(input_plugin_data*) (vgm.cc:55)
==18987==    by 0x134CF2: open_file_locked (input.c:463)
==18987==    by 0x134CF2: open_file (input.c:481)
==18987==    by 0x134CF2: ip_open (input.c:599)
==18987==    by 0x13F323: _producer_play (player.c:657)
==18987==    by 0x140CD4: player_play_file (player.c:1185)
==18987==    by 0x11FEAC: browser_enter (browser.c:414)
==18987==    by 0x128145: run_command (command_mode.c:2882)
==18987==    by 0x11E7A0: u_getch (ui_curses.c:2163)
==18987==    by 0x11E7A0: main_loop (ui_curses.c:2272)
==18987==    by 0x11E7A0: main (ui_curses.c:2556)

[ValleyBell]

Oops, sorry. The issue is that the resampler only allocates enough memory for outputting 0.1 seconds. You're rendering 7560 samples, i.e. 0.17 seconds, so we get a buffer overflow. (I need to admit that this is one of the parts from the original VGMPlay that I barely worked on.)

I'll put that on my TODO list. (There are additional issues if you change the sample rate mid-song.) Suggestions on how to fix this in a nice way are welcome though.

[jpcima]

It's a conclusion I came to while examining briefly this part of code, thanks for confirming.
At present time, I work around this by a frame limit of 4096, being 92ms at 44.1kHz, working fine.

additional issues if you change the sample rate mid-song

Seems a very unwise thing to do; at least for my case, sample rate will be kept fixed.

Suggestions on how to fix this in a nice way are welcome though.

The cleanest I had in mind, this would be to repeat work in smaller buffer sizes at resampler level.
Trying to divide the count by the allocated size, an overflow condition has still been raised though, and at this point I have stopped trying and went for a workaround.

[FraGag]

I encountered this issue with VGMPlayer, rendering a Mega Drive/Genesis VGM that doesn't use the YM2612's DAC channel.

I just submitted #114 that (hopefully) fixes this. (It fixes the issues I had with some VGM files, at least.)