vgmrips / vgmtools

A collection of tools for the VGM file format
GNU General Public License v2.0
123 stars 18 forks source link

Channel stripping for YM2612 (and SN76496) #20

Closed meyertime closed 2 years ago

meyertime commented 2 years ago

See #19.

I followed @ValleyBell's instructions to fix the channel stripping for SN76496 and implement it for YM2612. I tested it with some Genesis .vgm files (from Sonic 1), and it is working.

There's only one problem, and I'm hoping to get some guidance. The command line help text indicates that specifying something like -Strip:YM2612:DAC should work for stripping the DAC, but I could not find where that information gets put in ChnMask. Instead, at this point, -Strip:YM2612:6 strips the DAC. Any insight?

ValleyBell commented 2 years ago

DAC as "channel 6" is fine. It looks like the "DAC -> channel 6" assignment is something I never did.

It is supposed to go here: https://github.com/vgmrips/vgmtools/blob/3d597295a9178eec48b0e2b3e3ff85313e17dafb/vgm_ptch.c#L888

else if (CurChip == 0x02 && ! stricmp(ChipPos, "DAC"))
{
    TempChip->ChnMask |= (1 << 6);
}
meyertime commented 2 years ago

Ok, I made the change, and the DAC parameter works now. I also made a couple refinements to the filtering.

I'm pretty sure it filters things accurately, based on my reading the manual and the .vgm files that I tested with. A couple footnotes:

ValleyBell commented 2 years ago

looks good! I may remove the "strip register 2B for channel FM6" though. If you strip "FM6" it may strip the DAC enable write as well, disabling the DAC entirely.

Re timers: Those are removed by vgm_cmp already. I should fix that though, because if CSM mode gets enabled, Timer A does have an effect. If you would want to go advanced and do register modifications, I'd split the stripping code into a separate tool.

YM2612 FM6/DAC handling in general can get difficult, because you can switch between FM6 and DAC whenever you want. So removing data from register B6h can only be done when you can guarantee that no DAC -> FM6 switch occours.

meyertime commented 2 years ago

Regarding register 2B, it should only strip it if both FM6 and DAC are being stripped. My thinking here is that if neither FM6 or DAC is used, there's no reason to care about DAC enable. However, I would of course defer to you, as I'm sure you have much more knowledge of the YM2612 than I do.

It also just occurred to me that we could potentially strip register 22 if all FM channels are stripped (leaving only the DAC), because the LFO is not used by the DAC.

But I'm not worried about it, or doing the more advanced stripping. It works for my purposes, as I'm just trying to isolate each individual channel in its own .vgm file so I can render them separately. The resulting file may still have a little unnecessary information, but it functions as I need it. I put the comments the PR in case anyone else wanted to pick it up in the future for any reason.

Thanks again for your help on this!