Skip to content

ADLMIDI/OPNMIDI: Added dynamically-changing options and others#78

Merged
madame-rachelle merged 11 commits intoZDoom:masterfrom
Wohlstand:master
Jul 15, 2025
Merged

ADLMIDI/OPNMIDI: Added dynamically-changing options and others#78
madame-rachelle merged 11 commits intoZDoom:masterfrom
Wohlstand:master

Conversation

@Wohlstand
Copy link
Contributor

@Wohlstand Wohlstand commented Jul 5, 2025

This update contains multiple changes:

  • Fixed a bug when libOPNMIDI won't be properly restarted on changing of new setup.
  • Implemented the hot change of the rest of setup at libADLMIDI and libOPNMIDI without restart.
  • Ensured thread safety on attempts to change settings of libADLMIDI and libOPNMIDI (API of libADLMIDI and libOPNMIDI is not thread save per every instance).
  • Added gaining factor to allow users adjust it, even to "OPL Synth Emulation" as well.
  • Implemented an ability to use WAD's GENMIDI as a custom bank for libADLMIDI (it gets converted into WOPL format and loaded as a custom bank).

Otherwise, it's too quite
@Wohlstand
Copy link
Contributor Author

P.S. A question: maybe add a separated option to let user adjust the gaining factor outside the general music volume level?

It caused a bug that libOPNMIDI doesn't gets restart when changing these setup.
@Wohlstand
Copy link
Contributor Author

P.P.S. I added a bugfix (to bug I did by myself, that caused new-added options won't let libOPNMIDI to restart).

@Wohlstand
Copy link
Contributor Author

Okay, speaking about gaining factor, I will make a similar option to that Fluidsynth already have.

@Wohlstand
Copy link
Contributor Author

Don't yet merge this, I'll send in adddition the gaining options (I reviewed Fluidsynth's setup and making similar one right now).

Wohlstand added 2 commits July 5, 2025 15:03
…OPNMIDI

These settings has an ability to be changed on the fly, even bank can be changed on the fly with no hurt. However, I didn't yet implemented dynamic bank changing yet. I'll try in the next commit.
@Wohlstand
Copy link
Contributor Author

Wohlstand commented Jul 5, 2025

Huh, now another question: is ANY thread locking is set when changing settings of the synth together with the samples getting? I found the fact when I switch emulator to something heavy, it unexpectedly reports me that memory is danaged (via malloc/calloc), and sounds like no mutex is set 🤔.

Wohlstand added 4 commits July 5, 2025 16:14
Since bank defines the volume model, and the gain factor supposed to be different, then it's need to be refreshed.
Since the same emulator is shared between both OPLSynth and libADLMIDI, and therefore they should use equal base class. And since libADLMIDI needs an extra function to get int16 samples, therefore, add a dummy call to all such emulators at OPLSynth so they has compatible API.
@Wohlstand
Copy link
Contributor Author

Okay, right now it's ready to merge. I finished the rest of updates regarding to this.

@Wohlstand Wohlstand changed the title OPNMIDI: Added default gaining factor ADLMIDI/OPNMIDI: Added dynamically-changing options Jul 5, 2025
@Wohlstand
Copy link
Contributor Author

Also, just now I implemented an option to use WAD's GENMIDI as a custom bank.

Wohlstand added 2 commits July 5, 2025 18:58
Don't try to change settings if not the same synth is running. Otherwise, this might cause various odd behaviour cases like taking of changes at wrong synth.
(Even conditions are fine)
@Wohlstand
Copy link
Contributor Author

Okay so, I am done everything today, I tested this deeply and seems should be fully fine from my side.

@Wohlstand Wohlstand changed the title ADLMIDI/OPNMIDI: Added dynamically-changing options ADLMIDI/OPNMIDI: Added dynamically-changing options and others Jul 5, 2025
@Wohlstand
Copy link
Contributor Author

I updated description to display multiple changes that were done here.

@Wohlstand
Copy link
Contributor Author

@coelckers, ping?

@Wohlstand
Copy link
Contributor Author

I done everything here a while ago, anything also that delays this merge than just a lack of free time?

@madame-rachelle
Copy link
Collaborator

Yes, basically that, this is not a simple button-press merge since we have to update the zmusic stuff in gzdoom before fully accepting this.

@Wohlstand
Copy link
Contributor Author

Do you have anything also than my updates? What about MY updates, I already made suitable update at the GZDoom too: ZDoom/gzdoom#3176. So, that GZDoom's PR awaits when ZMusic will be updated so CI will be built properly. And also, locally I tested and combined everything (that is related to my updates), and it just builds and works (at least on Linux where I am). I will probably need to try build the custom thing for my friend on Windows to let him test my updates I promised to him (he seriously asked them, and right now he sticked on my WinMM drivers to use them like system devices and they work slightly faulty because some controllers aren't properly reset while switching songs, but that probably another bug that I should report independently).

@Wohlstand
Copy link
Contributor Author

Wohlstand commented Jul 15, 2025

At least, I will just rebase all my updates to the latest state. Nope, no needed, since you didn't pushed anything after my last update.

@madame-rachelle
Copy link
Collaborator

Yes, I am working on testing that now. Everything seems good so far.

@Wohlstand
Copy link
Contributor Author

Wohlstand commented Jul 15, 2025

Cool!
Okay so, I just rebased my updates at GZDoom side to the latest state just in a case.

@madame-rachelle madame-rachelle merged commit ceb92b5 into ZDoom:master Jul 15, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants