KungFuFurby / AddMusicKFF

Fork of AddMusicK, a compiler/inserter of music for Super Mario World
23 stars 16 forks source link

Enhanced accuracy in SetPitch (using Note+Tune sample tuning) #209

Open Aikku93 opened 3 years ago

Aikku93 commented 3 years ago

I'll preface this by saying the following: My proposal will break 1:1 correspondence with vanilla SMW's audio. I doubt the difference would be noticeable to anyone that isn't paying very close attention, but I had to mention that (on the chance that this would make my idea a deal-breaker).

SMW (and by extension, AMK) has a... "feature", where notes played using the lowest octaves become a lot flatter than they should, even when accounting for the fixed-point precision of the playback rate. This is caused by computing 2^(Note/12) prior to multiplying by sample tuning, as then low notes have an imprecise rate before they are adjusted with sample pitch (as the octave scaling is already performed by then).

To avoid these issues, I've implemented a completely different system: Instead of specifying instrument sample pitches as a ratio to multiply the playback rate with (eg. $03 $00), instrument sample pitches are specified in semitones and finetuning (0\~255/256; roughly 0\~99.6 cents), and account for the hardware playback rate (1000h = 32kHz) and the pitch table's scaling constant. This allows you to perform all tuning in log space (ie. in note+tuning), and only converting to rate as a final step, preserving precision at all octaves.

For example: In my system, I've had the pitch table use a scaling constant of 2276.0 (this is the maximum precision possible using 8x8=16 multiplication), whereas AMK uses 2143.0. To convert a sample pitch value from the old style, $03 $00 would be converted to $7D $FA (ie. Round[(1<<8) * (Log2[0300h * 2143/2276]*12 + 12)]; the +12 is to avoid underflow); this conversion can be automated when converting music files (just a simple fix-up after reading tuning values in Music.cpp), but must be done by hand for InstrumentData.asm (this is very simple to do, and took me maybe 2 minutes). With those adjustments done, it's a simple matter of modifying the SetPitch routine in main.asm for this system.

(Note that this conversion is only for the sake of backwards compatibility; if we drop the compatibility, users should be able to specify tuning directly as Note+Finetune, and then we only need to add the 32kHz and pitch table scaling constants)

I've re-written the relevant sections already and thoroughly tested, with tuning remaining accurate across all octaves (instead of falling flat at the lower ones). Judging from the code I ended up with, this should also be a bit faster than stock AMK, as it avoids the 16x16 multiplication with sample tuning, instead replacing it with a single ADDW on the incoming note+tune values in $10,$11 (I think it might take a few bytes more memory though, as I needed a separate code path to deal with the highest octaves, as these require the rate to be shifted up rather than down).

While this system should be a lot more accurate across the octaves, it introduces two issues: 1) SFX tuning becomes slightly off, as these tunings are only specified with a single byte; it shouldn't make much of a noticeable difference, though, as only the finetuning would be off (should be within a semitone of the originals, and I couldn't hear any real difference on casual testing), and 2) Nostalgia. I've had feedback about this tuning method that - while sounding fantastic in its accuracy - it sounds "wrong" because people are used to AMK sounding flatter in the lower octaves.

I'd appreciate feedback on this idea, and if it's something that might be considered for implementation (even as a command-line switch or something), I'll create a fork with the relevant modifications so that it can be merged into here as needed.

KungFuFurby commented 3 years ago

That's actually pretty interesting. I have my own workaround for that kind of precision problem... because of a loss of precision on higher notes with finetune, not lower notes (that one's caused by a hardware precision limitation). I just simply use a pitch base of $02 $00 or lower (well, maybe a little bit above that, actually), because any higher than that will simply mean you risk getting a hardware pitch overflow on the highest notes (one of the stock SMW instruments easily falls for this, on that note).

On that note, I was actually thinking of just rearranging the pitch base code so that the pitch base calculations are done first (but only if it's equal to or greater than $01 $00, otherwise this loses precision). Otherwise, I can keep track of 24 bits at once instead of 16 and let the word opcodes shine as needed (the lowest 8 bits are always discarded because they only factor into the first multiplication by the lowest 8 bits of each parameter).

In addition, I had another one up my sleeve... I have a 16-bit constant, $0F39 representing the twelfth root of 2 in 65536ths. I was thinking about multiplying that by the fine tune value (an 8x16 operation), then multiplying the note's pitch by said adjusted fine tune (a 16x16 operation, saving the upper 16 bits) to get the final result. The pitch code already handles shifting, with me having added one going in the opposite direction AND having enhanced the one in the direction its already going in through #37 (however, it's not ready because it doesn't work too well with pitch slides, and it's only achievable through transposition).

Technically what we would end up with would actually work more like a fixed transposition of note+finetune... just that the ideas on how to achieve that seem to be a bit different.

An alternative tuning mode wouldn't be a bad idea in hindsight, toggleable by VCMD (though it would be a global setting). After all, I can just make some sort of branch/jump-based gate that gets toggled accordingly.

Aikku93 commented 3 years ago

Precision loss on the higher notes? Interesting. If I'm reading the code correctly (which I definitely believe I am, having written sound drivers in the past), the pitch calculations only become imprecise as the rate shifts down with the octave (ie. Rate >>= Octave (the LSR,ROR loop) drops the lower bits, getting worse the lower you go). So perhaps it's actually the situation that everyone compensates for the imprecise tuning of the lower octaves, and find higher notes to be "out of tune" because of that compensation?

I definitely understand the hardware pitch overflow issues, though. And that's actually one of the neat things of the Note+Tune system I proposed: You can know if you've overflowed as soon as the sample pitch adjustment's ADDW executes (assuming your incoming Note+Tune hasn't underflowed into negative values), as then the carry flag would become set. I haven't accounted for this in the slightest (I figured that if you're running into overflow issues, you're probably doing something weird anyway), but you should be able to catch even hardware pitch overflows if the pitch table's scaling is set to 2048.0 (instead of my 2276.0) and the octave offset is adjusted accordingly, because then you can say that a Note+Tune greater than FFFFh will overflow the hardware register, and so you can BCS to a small snippet that clips the value back to FFFFh (which should then translate to 0FFFh for the hardware pitch, ignoring rounding error). I suppose that in that snippet, you could also inspect the the values to see if you've underflowed... but I figure that if you've set the maximum rate at Note+Tune=FFFFh, underflow shouldn't happen unless your sample's pitch is insanely low and you're using some very, very low pitches that would collapse to a hardware rate of 0000h anyway.

KungFuFurby commented 3 years ago

Precision loss on the higher notes? Interesting. If I'm reading the code correctly (which I definitely believe I am, having written sound drivers in the past), the pitch calculations only become imprecise as the rate shifts down with the octave (ie. Rate >>= Octave (the LSR,ROR loop) drops the lower bits, getting worse the lower you go). So perhaps it's actually the situation that everyone compensates for the imprecise tuning of the lower octaves, and find higher notes to be "out of tune" because of that compensation?

Yeah, it's only keeping track of 16 bits at a time, rather than 24 or more bits.

but I figure that if you've set the maximum rate at Note+Tune=FFFFh, underflow shouldn't happen unless your sample's pitch is insanely low and you're using some very, very low pitches that would collapse to a hardware rate of 0000h anyway.

Surprisingly, I have a sine wave sample that's very small that can put that to the test... because I actually used that at extremely low pitches for https://battleofthebits.org/arena/Entry/New+Wave+Beat/43032/ , which is actually where I debuted #37 in the first place (as well as some of the new VCMDs and remote events).

Aikku93 commented 3 years ago

Yeah, it's only keeping track of 16 bits at a time, rather than 24 or more bits.

Right, but what I'm getting at, is that SetPitch can be thought of as two parts: 1) Converting Note+Tune into a (roughly) 11-bit fixed-point Rate, and 2) Combining said Rate with the sample pitch. If you combine the two steps into one (which your 24bit system would do, if I'm understand properly), then there's not much loss of precision. But if you do them separately (as vanilla SMW does), the biggest hit to precision wouldn't be the hardware precision, but rather it would be step 1 in the lower octaves, because the octave scaling takes place in step 1 rather than step 2, so your 11-bit Rate would become 10-bit, 9-bit, 8-bit, etc., even before you've accounted for the sample pitch.

Surprisingly, I have a sine wave sample that's very small that can put that to the test

Neat. I'll give this a shot with my system (for reference, I'm using AMK 1.0.8 that I downloaded from SMWC, so this doesn't include any bleeding-edge commits like the 'negative note values').

EDIT: My bad, I didn't realize the link was just an SPC file haha.

Aikku93 commented 3 years ago

Here's my proposed modification to SetPitch (again, based on AMK 1.0.8 on SMWC). I've included clipping the rate here, but it can only guard ~12 semitones (technically, you can guard as much as you want if you check the carry flag inside the loop, but I figured that that would be overkill).

If this is an interesting enough proposal, let me know and we'll try and work something out.

Also, is there a better communication channel for you than on here?

KungFuFurby commented 3 years ago

The SNESLab sever on Discord has an #addmusickff channel dedicated to this particular project. You'll find me under the same username (plus some numbers courtesy of Discord) there.

Oh, and this would also require a new parser version just for the sake of having native values with your instrument redefinitions.