Sound Engine: various fixes for interrupting looping DPCM music samples with SFX samples

[grendell]

These proposed changes fix the following issues:

• Looping music samples now restart (from the beginning) after being interrupted by a SFX sample
  • Looping music samples will not restart if ended while interrupted
• Looping music samples are no longer dropped when started during a SFX sample and will start when the SFX sample ends
• SFX samples are no longer stopped when an interrupted music sample ends

I realize these changes will need to be ported to NESASM, ASM6, and cc65, but since this is my first attempt at a contribution to this project, I wanted to get feedback first.

Thanks!

[BleuBleu]

Hi.

Can we isolate more of that code only when FAMISTUDIO_CFG_SFX_SUPPORT is active? From the description, most of the stuff seem to be related to SFX being interrupted and stuff, yet I dont see a single #if FAMISTUDIO_CFG_SFX_SUPPORT.

Im guessing we probably dont need most of that stuff in ROM/NSF.

And yes, this will need to be ported to other assemblers as well as a few other things.

1. Port to other assemblers, including multi. Keep things aligned as much as possible.
2. Run and pass the sound engine unit test to make sure all assemblers match binary-wise.
3. Make sure the demo ROM still works
4. Recompile all the NSF/ROM kernels, make sure there are no issues there.
5. Run and pass the FamiStudio unit tests, which will ensure NSF arent broken.

[grendell]

Thanks for pointing me to those resources. Just to confirm, is this functionality you'd be interested in having, before I keep going?

I missed the intended use of FAMISTUDIO_CFG_SFX_SUPPORT. However, famistudio_sfx_sample_play is not wrapped in that currently, nor does it seem to depend on famistudio_sfx_init. Is this intended?

[grendell]

1. Done. dcc6cd2

2. SoundEngine\UnitTests\run_tests.bat
   All ROMs are identical!

3. Tested all demo ROM songs in Mesen.

4. Famistudio\Nsf\make.bat and Famistudio\Rom\make.bat run without error, although there are a few pre-existing bank count warnings.
   Is there further testing that needs to be done?

5. FamiStudio\UnitTests\FamiStudioRunTests.bat
   ..\bin\Release\net8.0\FamiStudio.exe TestVRC7.fms unit-test TestVRC7_FamiStudioTest.txt
   Error: Project file appears to be corrupted. Files created in development versions or forks are not compatible.
   Error opening input file TestVRC7.fms.

   Without VRC7 tests:
   FamiStudio unit tests passed!

Please let me know what other validation steps should be taken, if any.

My concern with wrapping this logic in FAMISTUDIO_CFG_SFX_SUPPORT is that users might already be using famistudio_sfx_sample_play without it, which would prevent them from receiving this fix. Wrapping famistudio_sfx_sample_play as well feels like a bigger, breaking change, but I could look into this as well if you prefer.

[grendell]

Apologies, I missed that nsf_famistudio_famitracker_vrc7_ntsc and nsf_famistudio_famitracker_vrc7_pal are now failing due to code size:
ld65: Warning: tmp.cfg(11): Segment 'CODE' overflows memory area 'CODE' by 14 bytes

I assume the only path forward is to optimize a few instructions. Any known low-hanging fruit would be most welcome. 😄

[grendell]

• Changed a few loop counters and one jmp to claw back the 14 bytes for the VRC7 NSF code segment.
• Ported those small changes to all assemblers.
• SoundEngine unit tests pass.
• Demo ROMs appear functionally unchanged in Mesen.
• FamiStudio\Nsf\make.bat and FamiStudio\Rom\make.bat report no issues.
• FamiStudio\UnitTests\FamiStudioRunTests.bat still fails to open TestVRC7.fms, as does a locally-built FamiStudio.exe via the UI.
  • Confirmed this also happens currently on master.
  • FamiStudio 4.4.4 reports the project is file version 19 vs FamiStudio version 18.
  • All other FamiStudio unit tests pass.
• Committed all changes to NSFs, ROMs, and Demo ROMs.

[SteoTheFrog]

Usually another codebank is just added in cases like this, since it will just break again in future when something else is added otherwise. Mat mentioned this to me before when working on FDS and VRC7 related stuff.

[grendell]

I originally tried adding another code bank in the VRC7 configuration, but after it wasn't an automatic fix, I didn't want to introduce bank switching for such a small fix. Of course, I may have just not known how to actually make this work within the system.

I don't think there's any reason not to make these micro-optimizations, but if this is the preferred path, I will look into it.

[grendell]

@BleuBleu any feedback on my previous points or the additional changes? Thanks.

[BleuBleu]

Form a high level I don't have any objection, Ill take a closer look at the code around 4.6.0, the next major release. You started this PR right around the time we release 4.5.0 so unfortunately we missed our window. :(

The release schedule is these days is roughly every ~4-6 months, so before end of year for sure.

-Mat