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

Update MAME emulation #382

Merged
merged 7 commits into from
Jul 10, 2021
Merged

Update MAME emulation #382

merged 7 commits into from
Jul 10, 2021

Conversation

rerrahkr
Copy link
Member

@rerrahkr rerrahkr commented Jul 3, 2021

The goal of this PR is to avoid violating the license of the MAME emulation file and replace it with a new one.

Currently BT used a MAME emulation modified by VGMPlay, which seems to violate the license between the old MAME license and GPLv2. They are BambooTracker/chip/chipdef.h and BambooTracker/chip/mame/*.

The emulation used in libvgm is from a newer version of MAME, and is under an OSI-approved license. Searching with MAME, the license for each of the files that could be replaced is:

  • fmopn.h/c, fmopn_2608rom.h: GPL v2+
  • ymdelta.h/c: GPL v2+
  • emu2149.h/c: MIT
  • Other files: BSD 3-Clause

The emulation will be replaced with appropriate modifications. This change may possibly affect the audio stream in some way.


First, I made the new emulation interface classes. The nuked emulation has also been modified for consistency.

@rerrahkr
Copy link
Member Author

rerrahkr commented Jul 4, 2021

Replaced all MAME emulations with libvgm ones. emu2149 is now managed as a submodule since it has a separate repository.

The removal of the VGMPlay-derived code means that the "GPL v2 only" files are gone. So I change the license of BT to "GPL v2 or later".
I've also modified the text in the about dialog, so those translations will need to be updated after this PR is merged.

@rerrahkr rerrahkr marked this pull request as ready for review July 4, 2021 10:02
@OPNA2608
Copy link
Member

OPNA2608 commented Jul 7, 2021

The emulation will be replaced with appropriate modifications. This change may possibly affect the audio stream in some way.

I'm hearing differences in No Time To Waste.btm, specifically the SSG-EG.

No Time To Waste - FM4 Comparison.zip

I think it sounds closer to the Nuked core now, but different from what seemed to be the intended (old MAME) sound: https://www.youtube.com/watch?v=toDVpk1VOrQ

Maybe we should check which demo modules sound drastically different after these commits and ask their authors if they want to rework their modules, or if they'd like to have them removed due to the updated emulation not representing their original vision anymore.

.gitmodules Outdated Show resolved Hide resolved
Co-Authored-By: OPNA2608 <christoph.neidahl@gmail.com>
@rerrahkr
Copy link
Member Author

rerrahkr commented Jul 7, 2021

It seems to be a symbolic link error in appveyor.
Adding git config core.symlinks true to the ci job should fix it.

Or should I modify the include path of BambooTracker.pro without changing the git config?

@OPNA2608
Copy link
Member

OPNA2608 commented Jul 7, 2021

I don't think changing local git settings to a non-default value is a good idea, a user might run into this exact build error and report it as a bug later on.

We could remove the symlink and build emu2149 as a static library instead, similar to our versions of RtAudio & RtMidi.

@OPNA2608
Copy link
Member

OPNA2608 commented Jul 7, 2021

Building it as a static library now, AppVeyor should pass.

@rerrahkr
Copy link
Member Author

rerrahkr commented Jul 8, 2021

I sometimes hear noises when an ADPCM sample is finished playing. It seems to be caused by the memory being filled with 0xFF. I will fix it to be zero-initialized.

@rerrahkr
Copy link
Member Author

It works well as far as I can see. I will organize them after the merge of this PR because I have been in contact with every authors of demo modules who use SSG-EG. Thanks for your help!

@rerrahkr rerrahkr merged commit d72094d into master Jul 10, 2021
@rerrahkr rerrahkr deleted the resolve-emu-license branch July 10, 2021 08:38
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.

None yet

2 participants