search

8bitbubsy / pt23f

ProTracker 2.3F for Amiga
BSD 3-Clause "New" or "Revised" License
82 stars 7 forks source link

Crash when cutting sampledata twice, consecutively, after sampling for less than maximum duration #2

Closed echolevel closed 3 years ago

echolevel commented 3 years ago

This is one I've been experiencing for a while, but I thought it had something to do with my custom build and/or the add4k command in bootPT. However now I've had a chance to test more extensively and here's what I've found:

I get a crash when I:

I can replicate this on A1200, A600, A500+ and A500 hardware, and also on WinUAE (both A1200 and A500 emulation), and in both my custom build and the official (8bitbubsy) build. Also occurs in the official build prior to today's update.

Crash behaviour: mouse locks up for a second, then black screen, then Guru/Software Failure, then needs a cold reboot (or reset in WinUAE). The crash occurs when performing the SECOND cut operation on a sound that's just been sampled. Has also happened when using copy. Happens regardless of whether CUTTOBUF option is true or false.

It does NOT happen if the sampler was allowed to run out to its maximum duration (~64KiB before, ~128KiB now). It seems to only happen if the sampling operation was stopped early with a mouse click. The length of the recorded sample doesn't make a difference, nor the amount of sample data being cut/copied.

It also does NOT happen if the sample was loaded from disk.

If the buffer already contains something, e.g. from copy and pasting 'safe' sample data, then a <64KiB or <128KiB sample is recorded, then the first cut/copy operation on that sample will cause a crash.

Perhaps it's to do with the memory allocation during/after the sample loop, or the way the sample data length is stored? If there's any other info you need, just let me know. Thanks in advance if you have any time to look into this, but I understand if not!

8bitbubsy commented 3 years ago

Have you tried this on PT2.3D? Does it crash there? If not, it might be related to my new memory allocation routines... My guess is that it's passing the wrong memory alloc block size when free'ing, or something. I'll look into it later.

echolevel commented 3 years ago

Yes, tried on PT2.3D this morning and couldn't replicate.

8bitbubsy commented 3 years ago

Ok, good to know. I'll look into this when I feel like, not sure when... Truth is that I absolutely hate going through the ProTracker asm code, I think it's a nightmare, and it's truly a card house that can fall with the simplest code change.

echolevel commented 3 years ago

Completely understood!

8bitbubsy commented 3 years ago

This was fixed just now. I originally wrote hackish "padded" AllocMem/FreeMem routines so that the buggy quadrascope's out-of-bounds read would not read memory past sample data allocation, and this caused some issues with how the sampler code is freeing up unused memory if you don't fill up the whole sampling buffer. I did several commits to the code today, and the ones that actually matter are commit e43ccb341b4b7fe92d1f232891dc74fd14f518c0 and commit 69ee68f8fcf89b2aaa485bcf6b8cd97cc3f99d2a.

I slightly modified the AllocMemPadded/FreeMemPadded routines, then I string-replaced all occurrences of AllocMemPadded with PTAllocMem, and all occurrences of FreeMemPadded with PTFreeMem. Then I did the quadrascope fix so that we don't need extra padded memory anymore.

I suggest you take your time to look at the other commits I did today, and implement them as well.

echolevel commented 3 years ago

Great thanks - will do!

8bitbubsy commented 3 years ago

Oops, I forgot to bugfix the scopes in "real VU-meter" mode (yes, I have separate scope routines for that). The fix has been commited, please add it to your tracker.