nukeykt / nuked-opm Goto Github PK

The suffix _lock for signals latched for a clock cycle, for instance here.

Lock suggests that the signal will lock something else, whereas what happens here is that the signal is being latched.

I suggest using a suffix such as _latch or _flop or _ff, to avoid confusion.

I can make a PR if you agree.

I am changing my LFO implementation to fit the original architecture following your C translation of the chip. I have found a difference between my Verilog implementation (of your code) and your C code.

The top plot is the Verilog output, and the bottom is the C code. Note how the PM sign bit is lost for the first couple of pulses. Note too that there is a glitch in the sign. For the Verilog counterpart, the PM sign tracks the signal as expected. The amplitude part of the modulation seems equal. Only the sign is different.

I am trying to translate your C code to schematics first and then convert it in a more or less 1:1 translation to modern RTL code. So I try to understand the circuit while I translate it. Thus, I may have fixed something inadvertently in the translation.

I still haven't finished debugging other differences, but this one may be a genuine problem in the C code.

It looks like the envelope gets serial data starts showing at chip cycle 5, but the noise channel is latched at cycle 12. After cycle 12, the EG serial bit keeps showing activity for the last two bits of the envelope.

I find this a bit suspicious. Is the synthesizer really throwing away the two LSBs of the envelope?

Here are time waveforms obtained directly from your code illustrating this:

After implementing the DT1 circuit following your code, I got several complaints about music being off-pitch. I have run your code to extract the same detune table as present in the YM2151 application notes:

Nuked-OPM:
Oct  Note        | DT1=0 | DT1=1 | DT1=2 | DT1=3
-----------------|-------|-------|-------|------
0    0 (  17 Hz) | 0.000 | 0.000 | 0.053 | 0.107 
0    1 (  18 Hz) | 0.000 | 0.000 | 0.053 | 0.107 
0    2 (  19 Hz) | 0.000 | 0.000 | 0.053 | 0.107 
0    3 (  21 Hz) | 0.000 | 0.000 | 0.053 | 0.107 
1    0 (  35 Hz) | 0.000 | 0.053 | 0.107 | 0.107 
1    1 (  37 Hz) | 0.000 | 0.053 | 0.107 | 0.107 
1    2 (  39 Hz) | 0.000 | 0.053 | 0.107 | 0.107 
1    3 (  41 Hz) | 0.000 | 0.053 | 0.107 | 0.160 
2    0 (  69 Hz) | 0.000 | 0.053 | 0.107 | 0.213 
2    1 (  73 Hz) | 0.000 | 0.053 | 0.107 | 0.213 
2    2 (  78 Hz) | 0.000 | 0.053 | 0.107 | 0.213 
2    3 (  82 Hz) | 0.000 | 0.053 | 0.160 | 0.213 
3    0 ( 139 Hz) | 0.000 | 0.107 | 0.213 | 0.267 
3    1 ( 147 Hz) | 0.000 | 0.107 | 0.213 | 0.267 
3    2 ( 156 Hz) | 0.000 | 0.107 | 0.213 | 0.267 
3    3 ( 165 Hz) | 0.000 | 0.107 | 0.213 | 0.320 
4    0 ( 277 Hz) | 0.000 | 0.107 | 0.267 | 0.427 
4    1 ( 294 Hz) | 0.000 | 0.107 | 0.267 | 0.427 
4    2 ( 311 Hz) | 0.000 | 0.107 | 0.267 | 0.427 
4    3 ( 330 Hz) | 0.000 | 0.160 | 0.320 | 0.427 
5    0 ( 554 Hz) | 0.000 | 0.213 | 0.427 | 0.587 
5    1 ( 587 Hz) | 0.000 | 0.213 | 0.427 | 0.587 
5    2 ( 622 Hz) | 0.000 | 0.213 | 0.427 | 0.587 
5    3 ( 659 Hz) | 0.000 | 0.213 | 0.427 | 0.640 
6    0 (1109 Hz) | 0.000 | 0.267 | 0.587 | 0.853 
6    1 (1174 Hz) | 0.000 | 0.267 | 0.587 | 0.853 
6    2 (1244 Hz) | 0.000 | 0.267 | 0.587 | 0.853 
6    3 (1319 Hz) | 0.000 | 0.320 | 0.640 | 0.907 
7    0 (2217 Hz) | 0.000 | 0.427 | 0.854 | 1.174 
7    1 (2349 Hz) | 0.000 | 0.427 | 0.854 | 1.174 
7    2 (2489 Hz) | 0.000 | 0.427 | 0.853 | 1.173 
7    3 (2637 Hz) | 0.000 | 0.427 | 0.853 | 1.173 

The values for DT1=0/3 seem fine. The values for DT1=1 and DT1=2 are wrong. Could you please review your notes?

Comparing with the application notes, PMS results are reasonably close for all settings except PMS=7. For PMS=7 it should be 700 and the result is almost 800.

LFO (note)    | PMS=0 | PMS=1 | PMS=2 | PMS=3 | PMS=4 | PMS=5 | PMS=6 | PMS=7
--------------|-------|-------|-------|-------|-------|-------|-------|------
  0 ( 440 Hz) |     0 |     0 |     0 |     0 |     0 |     0 |     0 |     0 
 31 ( 440 Hz) |     0 |     0 |     1 |     4 |    10 |    23 |    96 |   192 
 63 ( 440 Hz) |     0 |     1 |     4 |    10 |    23 |    47 |   195 |   392 
127 ( 440 Hz) |     0 |     4 |    10 |    23 |    47 |    97 |   395 |   793 

The key code limiter at the end of all the calculations seems odd. t3 is limited to 126, rather than 127 (which would be 7'b111_1111 in binary). This could be a typo.

This is the line:

When inspecting a full fnum table (1024 entries) produced by the OPM_KCToFNum function, I was surprised to find that for KC values that match KC&0xF0 in between 0xC0 and 0xF0 the fnum progression goes completely out of order and transforms into very small figures.

This looks to me like a bug, but I may be completely loosing out something here. These codes seem to be achieved in the presence of phase modulation. Nonetheless, they do look like a very abrupt change from the regular sequence.

Please look at the attached file. The first column is the KC number in decimal, then in hex, then the output of OPM_KCToFNum and then my expected fnum progression.

fnum.txt

It seems that variables in the opm_t structure are registers in the original design, but some of them are not.

One such example is lfo_out2_b. This variable is stored in function OPM_DoLFOMult and accessed in function OPM_DoLFO1. Both functions are called one after the other in the same clock cycle. Thus lfo_out2_b is actually a way to transfer a variable from one function to the other within the same clock cycle. This is confusing as most members of the opm_t structure seem to represent registers.

Maybe a suffix for this kind of software variables would be advisable. Maybe _soft or _wire would be good.

Values for DT2 frequency offset fade away for octave 7:

Oct  Note        | DT2=0 | DT2=1 | DT2=2 | DT2=3
-----------------|-------|-------|-------|------
7    0 (2217 Hz) | 1.000 | 1.414 | 1.570 | 1.731 
7    1 (2349 Hz) | 1.000 | 1.414 | 1.570 | 1.731 
7    2 (2489 Hz) | 1.000 | 1.414 | 1.569 | 1.731 
7    3 (2637 Hz) | 1.000 | 1.414 | 1.569 | 1.678 
7    4 (2637 Hz) | 1.000 | 1.414 | 1.569 | 1.678 
7    5 (2794 Hz) | 1.000 | 1.414 | 1.569 | 1.584 
7    6 (2960 Hz) | 1.000 | 1.414 | 1.495 | 1.495 
7    7 (3136 Hz) | 1.000 | 1.412 | 1.412 | 1.412 
7    8 (3136 Hz) | 1.000 | 1.412 | 1.412 | 1.412 
7    9 (3322 Hz) | 1.000 | 1.332 | 1.332 | 1.332 
7   10 (3520 Hz) | 1.000 | 1.258 | 1.258 | 1.258 
7   11 (3729 Hz) | 1.000 | 1.187 | 1.187 | 1.187 
7   12 (3729 Hz) | 1.000 | 1.187 | 1.187 | 1.187 
7   13 (3951 Hz) | 1.000 | 1.120 | 1.120 | 1.120 
7   14 (4185 Hz) | 1.000 | 1.058 | 1.058 | 1.058 

The application notes do not describe this behaviour, and it could be wrong.

I don't know if this is a bug or not, but simple left or right panning produces fairly noticeable noise that isn't heard when centered.

Marble Madness Level 3 VGM sounds incorrectly. Presumably bug is in LFO implementation