-
Notifications
You must be signed in to change notification settings - Fork 43
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
Audio: move from old MAME YM core to ymfm #141
Audio: move from old MAME YM core to ymfm #141
Conversation
I don't think there are any technical barriers here, but since there might be philosophical arguments against adding a C++ lib into what is otherwise a pure C project. I want to ensure the top contributors are okay with this before it gets merged. @indigodarkwolf @stefan-b-jakobsson @jestin @irmen if you have any objections, please raise them. x16-emulator is still a C project, and there is a functional interface (ymglue.cpp) that the C code will call to reach the YM2151 C++ code through a shim. |
Personally I like to avoid C++, but if there is a great benefit I think we can use such a library. |
I don't have any concerns about adopting C++ into x16emu, but I would take care to check that the ymfm library is outputting data at an appropriate sample rate for replacing the previous ym2151 code. Box16 needed to resample from the ymfm library's sample rate, which turned into an adventure. That said, all of Box16's contributions should be license-compatible with x16emu, so if needs be, things should be portable. |
It looks like this is already the case. It appears ymfm is true chip emulation so it's that funky ~55.9KHz rate is coming out as x16emu is clocking the chip at 3.579545MHz. Natt's code already handles resampling and mixing with that expectation. |
I remember box16 had some issues being able to be compiled on certain platforms when it upgraded to a newer C++ standard. I am FAR from an expert here, but I see in this makefile you set the c++ standard to c++17. Is this potentially problematic? Is it required? Can we select an older standard this library has to conform to, to avoid such issues? |
C++17 is what ymfm needs, according to its docs. I think Stephen was trying to use C++20 in Box16 which was probably a bit more bleeding edge at the time. |
The existing YM2151 core has a license that could potentially be problematic. ymfm is licensed under BSD, which is compatible with the rest of the project being under MIT.
This introduces a C++ lib into a C project. For now the glue interface replicates the old function calls so that the emu side didn't have to change how it called into the lib.