a1k0n / jsxm

FastTracker 2 .xm module player in Javascript
http://www.a1k0n.net/code/jsxm/
MIT License
485 stars 36 forks source link

Implement E4x and address some vibrato issues #21

Closed hillerstorm closed 8 years ago

hillerstorm commented 8 years ago

This one was a bit weird.

I found this test case which has two channels, the right one seems to be pre-recorded from FT2 and the left one tries to mimic the right one. At first I thought it was a bit strange that OpenMPT only updates vibratopos on tick 1 and above but without it that test case sounds awful. I also had to multiply the vibrato depth for it to sound correct. This I even verified by loading the .xm file into FT2 and listening. The highs were higher and the lows were lower! And to finish it off I had to implement the vibratopos retrigger on note trigger. I'm commenting on the separate commits below so you can test it out and see where I got the info from.

This will break the 4xy vibrato test which I don't really feel qualified to fix :) the "playing actual audio" part of this is still black magic to me :innocent: For that same reason I didn't add any tests for square and saw, it just sounds right when testing against VibratoWaveforms.xm

a1k0n commented 8 years ago

Oh wow, thanks! Yeah, I'm very, very uncertain about how vibrato is supposed to work (does it update on tick 0? when does it take effect, etc), so thanks for doing the detective work.

a1k0n commented 8 years ago

To fix the test, just put the numbers from "actual" into the expected values in the 4xx test. I think what it's showing now is reasonable, assuming the depth is 2x as much as it was before, and the phase doesn't update on tick 0..

a1k0n commented 8 years ago

Also, add yourself as a collaborator to package.json. :)

hillerstorm commented 8 years ago

Okay! I got 2 tests stashed away for the other waveforms which I just copied almost :)

a1k0n commented 8 years ago

Wow, that test .xm is a really great test.

a1k0n commented 8 years ago

Anyways, the code looks great... i'd just like to keep the tests up to date. Merge when you are ready.

hillerstorm commented 8 years ago

On it!

a1k0n commented 8 years ago

Awesome, vibrato sounds much better now. Thank you!