Crash after previewing sample with empty name

Dn-Programming-Core-Management / Dn-FamiTracker

modifications and improvements for 0CC-FamiTracker (based on j0CC-FamiTracker 0.6.3)

Other

372 stars 23 forks source link

Crash after previewing sample with empty name #4

Open nyanpasu64 opened 3 years ago

nyanpasu64 commented 3 years ago

Extends Levant DPCM bass has notes which reference a (n/a) unoccupied sample slot. When you export the DPCM bass and reimport, they turn into a 0-byte sample with an empty name, which turns into 1 byte when you save and reopen the module.

CSoundGen::PlaySample() deletes any sample with an empty name. This causes memory corruption when you preview (but not play) the sample, then close the document or preview the sample again.

Gumball2415 commented 3 years ago

Can you send the instrument file? I can't seem to recreate the corrupt sample having a 0/1 byte size (mine becomes 4 and 17 bytes, respectively) Edit: Turns out I'm using version 0.2.1 of Dn-FT, that's probably why the file sizes are different from what you reported. I tried to import the corrupted instrument in j0CC-FT 0.6.3 and the sizes of before and after saving matches what you reported.

My steps to reproduce:

open application, then open the demo module Extends Levant in the "after" folder from 0CC version 0.3.15r3
save the instrument named "dpcm bass"
load it back in a new module. The corrupted sample shows a filesize of 4 bytes. (playing the sample at this point will cause the application to crash)
save the module, then restart the application and open the module again The corrupted sample now shows a filesize of 17 bytes (playing the sample also crashes the application)

nyanpasu64 commented 3 years ago

dpcm bass.zip has an empty sample, as exported from Dn-FT (I think).

dpcm bass (no crash).zip omits sample 4, causing N/A (which matches the song it was taken from) when importing the .fti. I don't know what program was used to generate this file. But the file dates back from 2017.

Gumball2415 commented 3 years ago

Compared the two instruments using a hex editor, they are exactly the same apart from one byte in address 0xC2. The instrument which omits sample 4 has 0x04 as the value, compared to 0x05 in the instrument from Dn-FT. Maybe this has something to do with the instrument export?

nyanpasu64 commented 3 years ago

instrument export should be fixed, to not write the invalid value (or empty name, idk what's wrong).
possibly instrument import should be fixed, to not produce corrupted or empty names when importing broken instruments.
maybe we should un-blank all empty sample names when loading .0cc files. if so, it's probably a good idea to fix instrument import too.
I don't know of an easy way to fix playback of sample names which are stored in memory as empty.

Gumball2415 commented 3 years ago

I'm theorizing that FT ~~kinda panics~~ doesn't know how to handle when a N/A sample is assigned to a note in instrument export. Previous versions in 0CC (0.3.15r3) and j0CC (0.6.3) exhibited the same behavior, though vanilla 0.4.6 seems to handle it well (it doesn't load the corrupted instrument, but allows the non-crash one)

nyanpasu64 commented 3 years ago

HertzDevil has a separate PlayPreviewSample() which doesn't delete the sample:

https://github.com/HertzDevil/0CC-FamiTracker/blob/3fa2cf46f5e5f53cc880cbbd01ebb699b6f743c8/Source/SoundGen.cpp#L939

Gumball2415 commented 1 year ago

As of commit b68ff8f2b47128da5f5ada1d52c8fd0203c342dd, reproducing the bug using the steps described above will send a "DPCM sample index out of range" error when Module Error Level is set to Strict or Default.