Print

[Saga Musix]

There's probably no widely spread tracker or player out there right (apart from the latest OpenMPT test versions) now which gets FT2's panning law right, and given that panning laws can greatly change how a module sounds, Milky better should get this right.
Right now, Milky uses a panning law that is similar to OpenMPT's old "headphones" mode. However, this is not quite right. I've found out that FT2 uses the square root pan law, which is close to the linear pan law, but it stresses samples that are played on the far left and right.
In ChannelMixer.cpp, around line 170, the left and right channel volume are computed. I think this should be done as follows:

Code: [Select]

mp_sint32 left = (mp_sint32)round(128.0f * sqrt((256-pan) / 256.0f));
mp_sint32 right = (mp_sint32)round(128.0f * sqrt(pan / 256.0f));

You may want to use a lookup table for that to avoid the constant recalculation of square roots, of course. Here's FT2's original LUT. Note that its indices range from 0 to 256, while the internal panning ranges from 0 to 255, meaning that there is no "true 100% right" panning, only 100% left is possible.

Code: [Select]

Formula: round(65536 * sqrt(n / 256)) for n = 0...256
   dd 0,4096,5793,7094,8192,9159,10033,10837,11585,12288,12953,13585,14189,14768,15326,15864
   dd 16384,16888,17378,17854,18318,18770,19212,19644,20066,20480,20886,21283,21674,22058,22435,22806
   dd 23170,23530,23884,24232,24576,24915,25249,25580,25905,26227,26545,26859,27170,27477,27780,28081
   dd 28378,28672,28963,29251,29537,29819,30099,30377,30652,30924,31194,31462,31727,31991,32252,32511
   dd 32768,33023,33276,33527,33776,34024,34270,34514,34756,34996,35235,35472,35708,35942,36175,36406
   dd 36636,36864,37091,37316,37540,37763,37985,38205,38424,38642,38858,39073,39287,39500,39712,39923
   dd 40132,40341,40548,40755,40960,41164,41368,41570,41771,41972,42171,42369,42567,42763,42959,43154
   dd 43348,43541,43733,43925,44115,44305,44494,44682,44869,45056,45242,45427,45611,45795,45977,46160
   dd 46341,46522,46702,46881,47059,47237,47415,47591,47767,47942,48117,48291,48465,48637,48809,48981
   dd 49152,49322,49492,49661,49830,49998,50166,50332,50499,50665,50830,50995,51159,51323,51486,51649
   dd 51811,51972,52134,52294,52454,52614,52773,52932,53090,53248,53405,53562,53719,53874,54030,54185
   dd 54340,54494,54647,54801,54954,55106,55258,55410,55561,55712,55862,56012,56162,56311,56459,56608
   dd 56756,56903,57051,57198,57344,57490,57636,57781,57926,58071,58215,58359,58503,58646,58789,58931
   dd 59073,59215,59357,59498,59639,59779,59919,60059,60199,60338,60477,60615,60753,60891,61029,61166
   dd 61303,61440,61576,61712,61848,61984,62119,62254,62388,62523,62657,62790,62924,63057,63190,63323
   dd 63455,63587,63719,63850,63982,64113,64243,64374,64504,64634,64763,64893,65022,65151,65279,65408
   dd 65536
With that LUT, the code should be looking somewhat like this:

Code: [Select]

Before (left/right range from 0...128):
mp_sint32 volL = (chn->vol*left*masterVolume)<<6;
mp_sint32 volR = (chn->vol*right*masterVolume)<<6;

After (LUT ranges from 0...65536):
mp_sint32 volL = (chn->vol*LUT[256-pan]*masterVolume)>>3;
mp_sint32 volR = (chn->vol*LUT[pan]*masterVolume)>>3;

Note that this code is duplicated several times throughout the whole mixer code. Might make sense to refactor this into a separate function.

[Deltafire]

Implemented in commit a3ae038.

[Saga Musix]

Great! To facilitate the correct intended playback of older modules (in potential third-party players), I'd propose a further change to the XM files written by MilkyTracker:
Right now, Milky just writes "MilkyTracker" in the tracker version string field in the XM header. There's still enough space to fit the version number in there, e.g. "MilkyTracker 0.99.99" (as long as your numbers remain under 100, they will fit in). Maybe this would be a good addition in the next release as well?

[Saga Musix]

Another thing... round() is not part of C++ until C++11. This makes the new code unable to compile on e.g. VS2010. To generate the panning LUT, you should use floor(0.5 + x) instead of round(x), as it's currently basically the only C++11 feature being used.

[Deltafire]

round() is part of C99 though, surely VS2000 supports that?

[Saga Musix]

That's one of those cases where C++ is (or was) not a superset of C, yes. VS has had notoriously bad support for C99 until very recently (VS2013). Doesn't matter though, since you cannot compile Milky using C99 anyway.

What do you think about putting a version number in the module header?

EDIT: Here's some code for it. In ExporterXM.cpp, replace

Code: [Select]

f.write("MilkyTracker        ", 1, 20);by

Code: [Select]

char milkyVersionString[21];
memcpy(milkyVersionString, "MilkyTracker 0.00.00", 20);
PPTools::convertToHex(milkyVersionString + 13, (TrackerConfig::version>>16)&0xF, 1);
milkyVersionString[14] = '.';
PPTools::convertToHex(milkyVersionString + 15, (TrackerConfig::version>>8)&0xFF, 2);
milkyVersionString[17] = '.';
PPTools::convertToHex(milkyVersionString + 18, TrackerConfig::version&0xFF, 2);
f.write(milkyVersionString, 1, 20);and add these includes at the top:

Code: [Select]

#ifdef MILKYTRACKER
// For version string:
#include "../ppui/Tools.h"
#include "../tracker/TrackerConfig.h"
#endif


[Deltafire]

It's using the C function round() from math.h - I'm getting no compiler warnings about using C++11 features.

Milkytracker now saves its version number into the .xm header

[Saga Musix]

Well, as said, VS prior to VS2013 has very sketchy C99 support, if at all, so that's missing. In general I think in the C++ context, math.h and cmath are the same file with different names, i.e. you cannot expect C99 features to be present there. Is it really that bad to replace a round(x) by a floor(0.5 + x)?
Thanks for implementing the version string, btw.