jotego / jt12

FM sound source written in Verilog, fully compatible with YM2612, YM3438 (JT12), YM2203 (JT03) and YM2610 (JT10)
GNU General Public License v3.0
120 stars 22 forks source link

PSG Sustain seems too short #56

Closed retrorepair closed 4 years ago

retrorepair commented 4 years ago

01 Neo-Geo Logo.zip

As the title suggests, this instantly sounded wrong to me, if you listen to the 5th note on real hardware (and indeed the attached VGM) it sounds slightly discordant with the previous note as it lingers. JT12 (JT49?) doesn't seem to let the note run long enough.

I don't know of another Neo Geo track with a similar PSG envelope but I bet there is one. I'm going to assume this affects other sounds on many other systems too.

jotego commented 4 years ago

I think this is fixed in commit https://github.com/jotego/jt12/commit/30901f3910ba1fcd08dd00232cd6eebb5a96f60a Please listen to the attached file and confirm it.

01 Neo-Geo Logo.wav.zip

retrorepair commented 4 years ago

The note duration definitely sounds right now. There is definitely something else missing though. It sounds too clean if you know what I mean? Hear the below example:

https://youtu.be/Fqsv7uh-rS4

It sounds like there is some flange or chorus on the note, or two operators? Are there still missing functions in JT49?

jotego commented 4 years ago

There's nothing missing in JT49. Note that the sound is affected by all the signal processing chain used from the sound generator to your ears. So the filtering is different in your example. It will also sound a bit different once moved into MiSTer. But things like sound duration, or notes being played shoult all be consistent.

retrorepair commented 4 years ago

Yeah I'm aware of subtle sound changes due to passive filtering and audio compression etc but even mame sounds like there is some chorus on the PSG notes, see here:

https://youtu.be/aybhwlEV0EM

It sounds very much like the example you posted is missing a PSG channel?

The note duration is definitely right now though :)

jotego commented 4 years ago

There is more than one variation of the Neo Geo jingle. Are you comparing the same ones?

retrorepair commented 4 years ago

Pretty sure I am, the one I sent you was from Art of Fighting which I believe is the same as Last Resort:

https://vgmrips.net/packs/pack/art-of-fighting-ryuko-no-ken-neo-geo

Both have the chorus effect on the PSG anyway

sorgelig commented 4 years ago

With jt49, the PSG sound isn't always right. For example in MetalSlug 1, the sound of "Go" (asking to move along) has much higher pitch than it should be. I don't know if it's overall clock of JT49 is higher or something wrong in registers. With YM2149 module (in latest release) the Go sound is correct.

jotego commented 4 years ago

There was a bug in the envelope generator which I fixed a few weeks ago. That's why I'm asking you to update your copy.

jotego commented 4 years ago

I have simulated the AOF version of the logo too (see it attached). I cannot hear any difference with the reference link. I think this was probably an issue solved with this commit: https://github.com/jotego/jt49/commit/57619f8b06487454d8a7e345590fcce4c14e6fc4

@sorgelig could you please restore JT49 inside your JT12 copy in the NeoGeo repository? The fewer the differences the easier it will be for me to maintain JT12.

AOF.zip

sorgelig commented 4 years ago

Yes, i've restored it (not committed yet as it's unclear when pitch will be fixed), and MetalSlug "go" sound is still broken. There are some other changes in jt12 fixing different bugs but not yet merged in original jt12 repo. If i understood right 2610 had no divisor setting registers, so divisors are hardcoded in 2610.

sorgelig commented 4 years ago

committed

jotego commented 4 years ago

Thank you. I'm going to spend next week looking at JT12 forks in Genesis and NeoGeo repositories. I'll try to make all converge back to the main repository and address the reported issues for those two projects. Please bear with me while I do it.

retrorepair commented 4 years ago

I compiled the latest commit to the MiSTer core but the chorus effect is still missing. Maybe I'll make a wav with the isolated PSG compared to MAME to highlight the difference. Everyone's ears are different. :)

That's another issue anyway, this issue is fixed!

retrorepair commented 4 years ago

I know what Sorg means with the pitch now, if you listen to the powerup effects in Last Resort you can hear it too, as well as league bowling's menu music.

It's weird because it wasn't present until I started updating, maybe one of these commits broke it?

I'll go backwards until the last time it sounded correct and report back.