Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AWE32 NRPNs #1346

Merged
merged 23 commits into from
Oct 31, 2024
Merged

Add support for AWE32 NRPNs #1346

merged 23 commits into from
Oct 31, 2024

Conversation

derselbst
Copy link
Member

@derselbst derselbst commented Jun 29, 2024

This adds a compatibility layer for AWE32-style NRPNs, see #1342.

Essentially, it adds a few more members to fluid_chan_t, allowing us to memorize any generators overridden by AWE32 NRPNs.

fluid_hz2ct() has been revived and fluid_sec2tc() has been added.

And fluid_mod_transform_source_value() is now extern.

@derselbst derselbst added this to the 2.4 milestone Jun 29, 2024
@derselbst derselbst linked an issue Jun 29, 2024 that may be closed by this pull request
Copy link

sonarcloud bot commented Jun 29, 2024

@klerg
Copy link

klerg commented Jun 29, 2024

This is great that work on this has already begun and hope to see more here

Not really sure what any if this means, but it does not matter at this point, and look forward for updates in this and other two to three threads related to it

I'm eager to follow this thread and await AWE32 NRPN support in Fluidsynth 2.4

@derselbst
Copy link
Member Author

@klerg
Copy link

klerg commented Sep 3, 2024

Appreciate it, I had no idea that a beta or test version was available, and will check this out then

Thanks in adv

@derselbst
Copy link
Member Author

@klerg Did you have a chance to test this? Pls. let me know if you find it useful.

@klerg
Copy link

klerg commented Sep 21, 2024

Yes, it is not bad, but with The Nervous Filter.mid the filter sounds too light compared to the SB Live! And in the end it does not sweep out to sound like the beginning, any way to fix this ?

@derselbst
Copy link
Member Author

any way to fix this ?

Not really. The filter design of the SB! is very different to the one used in fluidsynth. The frequency cutoff is much steeper. Researches from the DosBox guys also confirmed that.

All I could find to make this particular tune sound a bit better is to lower the frequency cutoff by 500Hz. This may break other tunes though...

Binaries:
https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=10560&view=artifacts&pathAsName=false&type=publishedArtifacts

@klerg
Copy link

klerg commented Sep 21, 2024

I see then. Sure but can the filter design of fluidsynth more closely match the SB Live! ? How much steeper is the frequency cutoff ? Nice to know Dosbox guys looked at this.

Yes that sounds good if you can go any lower on the frequency cutoff that is going to help. This version sounds better, but how will it break the other tunes ?

@derselbst
Copy link
Member Author

Sure but can the filter design of fluidsynth more closely match the SB Live! ?

No. Firstly, because the concrete filter design of the SB! is unknown. And secondly, even if it was known, the goal of fluidsynth is not to emulate EMU hardware.

How much steeper is the frequency cutoff ?

I don't know.

how will it break the other tunes ?

I don't know, since I don't have any "other tunes" that I could try. I am just saying that this modification is specific that that MIDI, and it's technically against what the AWE32 NRPN documentation says.

This version sounds better

Ok, thanks. So, I'll take this version then.

@klerg
Copy link

klerg commented Sep 21, 2024

I thought the AWE32 Developer's Info Pack and the NRPN doc's from AWE32 FAQ had this. Yes, but is the goal of FluidSynth to be compatible with EMU hardware ?

Sure, it is quite steeper

Well, so with both of the FluidSynth test versions the Uplift.mid has almost no sound for the first minute or so of the piece, any clue what is going on with that ? Wow that is odd maybe the AWE32 docs are misleading or even wrong.

Sounds good, can you fix the Uplift.mid file next ?

@derselbst
Copy link
Member Author

I thought the AWE32 Developer's Info Pack and the NRPN doc's from AWE32 FAQ had this.

No, not at this detail.

Yes, but is the goal of FluidSynth to be compatible with EMU hardware ?

No! Fluidsynth's goal is to be compatible with the SF2 spec, which is an open standard. EMU hardware is a closed implementation. See my comment: #1342 (comment)

Sounds good, can you fix the Uplift.mid file next ?

I'll see.

@klerg
Copy link

klerg commented Sep 22, 2024

Well, that may be too much detail then

Ok, I did not know the SF2 spec is an open standard, but it is quite obvious the EMU hardware is closed and little to nothing is open source or known on how it works.

Sure, how can I help with Uplift.mid ?

@derselbst
Copy link
Member Author

derselbst commented Oct 6, 2024

I found some time to look into Uplift.mid. And it's very interesting. The NRPN AWE32 documentation says, that 8192 is considered to be the middle value, i.e. zero. Therefore I always subtract 8192 from the composed Data Entry value. The Nervous Filter is using it correctly: it sets the DataMSB to 64 and the LSB value to the desired filter cutoff, e.g MSB: 64, LSB: 103, giving a composed 14 bit value of 8295. Subtract 8192 and you get 103, i.e. the value of the LSB bit which becomes your filter cutoff frequency. Great.

Uplift.mid however usually keeps the DataMSB at zero, sometimes even at 24. If you subtract 8192, you'll always get a negative value, which corresponds to the lowest filter cutoff frequency. It appears that AWE32, SBLive and friends only inspect the DataLSB when setting things like filter fc.

Also, Uplift.mid doesn't initialize the filter coefficient. A default value is not documented, hence I'm assuming zero.

See... that's the gap between the Creative documentation and their implementation, making things unnecessarily complicated to implement for me. I'll commit a fix shortly.

@derselbst
Copy link
Member Author

@klerg
Copy link

klerg commented Oct 6, 2024

I see good to know you were able to look at Uplift.mid. Yes I'm sure that is the case. Sure, but only the AWE32 Developer's Info Pack says 8192 is the middle value and how come that is zero but not 64 ? I assume you are trying to figure what the NRPN's are and that is why you subtract 8192 from the Data Entry value. Nice to find out The Nervous Filter follows the AWE32 docs and it makes sense how an LSB of 103 ends up being the 14 bit value of 8295, and then you subtract the 8192 to get 103, and that is the value for filter cutoff frequency, yes this works then.

Well, if the MSB is zero in Uplift.mid that will put it at the lowest point, but at 24 it is going to be closer to the middle of 64. Yes, and we need to subtract 8192 to find out what the NRPN values are too. Sure, and that always gives a negative and that is why cannot hear anything as filter cutoff is too low. What do you mean by filter fc by the way ?

I do not know what you mean by the filter coefficient, can you explain a bit more on this and is it able to be something else besides zero ?

Yes, it looks like the AWE32, Live!, and Audigy do not really follow the documentation from the AWE32 Developer's Information Pack especially. I'm sure you will post a fix for this very soon too.

@klerg
Copy link

klerg commented Oct 6, 2024

I was able to try the new test version and while Uplift.mid sounds much better, again The Nervous Filter.mid has almost no filter to it anymore, how to fix that ?

@derselbst
Copy link
Member Author

again The Nervous Filter.mid has almost no filter to it anymore, how to fix that ?

The Nervous Filter sounds exactly as before. The renderings are identical. I cannot improve this, because details about the filter implementation are unclear, as explained earlier.

Sorry, I can't explain every detail that you don't understand.

IMO, this implementation is now good to go.

@klerg
Copy link

klerg commented Oct 6, 2024

The Nervous Filter sounds like build 10458 and not 10560. It is identical to 10458. I know what you mean and you told me this before. So in this latest build 10652 is the frequency cutoff lower by 500 Hz like in the 10560 build then ?

My hope was you could tell me more on filter coefficient, but that is fine then

I just ask if the frequency cutoff is lower by 500 Hz in latest build ?

@derselbst
Copy link
Member Author

The Nervous Filter sounds like build 10458 and not 10560. It is identical to 10458.

This is wrong. 10560.wav and 10652.wav both are identical. Check the md5 hash of the files - they are identical. They both play at a frequency cutoff that is reduced by 500Hz.

10458.wav is the one that plays that the unchanged frequency cutoff.

filter renderings.zip

Also, Uplift.mid doesn't initialize the filter coefficient. A default value is not documented, hence I'm assuming zero.

My hope was you could tell me more on filter coefficient, but that is fine then

I was talking about AWE32 NRPN 22 not being initialized by Uplift.mid.

@klerg
Copy link

klerg commented Oct 12, 2024

Hope I'm wrong. Yes even the file sizes are exactly the same. Sure the md5 hash match for each file. Good to know 10560 and 10652 have a frequency cutoff reduced by 500Hz.

That's right, 10458 sounds the worst of the three here. What soundfont did you use for the renderings ?

Also, one more midi sounds wrong, it is MYSTRAVE.mid, the harpsichord at the start has no filter, can you fix this ?

Do you know why the AWE32 NRPN 22 is not initialized by Uplift.mid ?

Copy link

sonarcloud bot commented Oct 13, 2024

@derselbst
Copy link
Member Author

Also, one more midi sounds wrong, it is MYSTRAVE.mid, the harpsichord at the start has no filter, can you fix this ?

It does have a filter! It's just not as low-pass as the SoundBlaster. That's the problem we're talking about. I have lowered the frequency cutoff by 1000 Hz to compensate this.

I also fixed another bug that caused the frequency changes to be not applied to playing voices. The problem with this is that it produces clicks in the nervous filter. I will write up a bug ticket for this and investigate this separately.

IMO, I have invested enough of my time into this topic now. The solution is good enough and I'll merge this on next occasion.

Binaries: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=10722&view=artifacts&pathAsName=false&type=publishedArtifacts

Do you know why the AWE32 NRPN 22 is not initialized by Uplift.mid ?

Because the composer of that file hasn't set it.

@klerg
Copy link

klerg commented Oct 13, 2024

I could not hear the filter. Yes it was too far off from the SB Live! The problem is the AWE32 docs are misleading and wrong. Sure, now I notice a light filter it is better than nothing

I see, this bug to me does not only happen when "playing voices" and you can spot this in the ALTITUDE.MID too. Yes, I get clicks and noises in ALTITUDE.MID and the nervous filter. Sounds good I hope you can fix this issue also.

Well, that is up to you at this point, but looks like the only way to improve this is to not follow the AWE32 docs, and try to match the audio renderings, right ?

Does 10722 fix the frequency change problem that you outlined above ?

Of course, that makes sense if Uplift.mid does not use AWE32 NRPN 22 then

@derselbst derselbst merged commit e93eab2 into master Oct 31, 2024
77 of 78 checks passed
@derselbst derselbst deleted the awe32-nrpn branch October 31, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWE32 NRPN Compatibility Layer
2 participants