DhrBaksteen / ArduinoOPL2

Arduino library for use with the OPL2 board (YM3812) and OPL3Duo (YMF262)
MIT License
199 stars 39 forks source link

Teensy MIDI Example buzzing and cracking noise #44

Closed zerbian closed 5 years ago

zerbian commented 5 years ago

Hello there! After modifying the Teensy MIDI Example to work with the standard MIDI Libary an my Arduino Nano the code worked but it produces additionally cracking noises when play some notes. Is there a simple fix I can use or a specific section in the code where I should look at? Greetings

DhrBaksteen commented 5 years ago

Hi,

A similar issue about unwanted noises when playing certain notes was reported to me a few days ago by another user. I'm still investigating the cause of this...

DhrBaksteen commented 5 years ago

This issue is due to bugs in how the code is reusing OPL2 channels. Currently it often picks a channel that just stopped playing a note. The release portion of the note will be cut off when the new note is started and a clicking or popping noise can sometimes be heard depending on the instrument.

virtuaCode commented 5 years ago

I had this problem too. It's like @DhrBaksteen said, the program will always reuse the most recent played channel. A possible fix is to reuse the oldest released channel for the new note. You can find my modification here: virtuaCode@a1a5b0a4e104ff9d81af75b0e65150b4da5d0215

You may want to diff examples/Teensy/TeensyMidi/TeensyMidi.ino examples/SerialMidi/SerialMidi.ino in the serial_midi branch to see exactly what I have changed.

zerbian commented 5 years ago

I had this problem too. It's like @DhrBaksteen said, the program will always reuse the most recent played channel. A possible fix is to reuse the oldest released channel for the new note. You can find my modification here

Thank you, that solved the noise and cracking.

virtuaCode commented 5 years ago

I was able to reduce the cracking noise even further as you can hear here: https://github.com/virtuaCode/opl2_audio

SAMPLE1.mp3 -> Call setInstrument() on every NoteOn (current behavior) SAMPLE2.mp3 -> Without calling setInstrument()

It seems that the setInstrument() function is causing the noise, but I cannot say why.

DhrBaksteen commented 5 years ago

Hi @virtuaCode, I have the same understanding. From my experiments it seems to be depending on what instrument you are using. Also when a channel gets reused while the release portion of note is still playing it can make cracking noises appear. In the latest fixes I pushed for #45 and #46 this is addressed by moving channels for which a key gets released to the back of the list so it has a bigger chance of playing out the release portion of the note.

I'm thinking that the order in which the library is setting the instrument registers may be the cause, but after trying some things I'm not 100% sure. I'm trying to find some old docs or unravel some IMF files to get some ideas...

virtuaCode commented 5 years ago

I guess the problem is caused by the 0x40-0x55 registers. Because when setInstument() is called, these registers will be set to the original instrument output level for a short time. Apparently, this output level is much louder than the previous level which was derived from the MIDI velocity of the played note.

DhrBaksteen commented 5 years ago

Thanks for that comment @virtuaCode I think I have the problem solved! I already tried an alternative setInstrument during the weekend that would not set the output levels like you suggested. But for me it didn't work, so I dismissed it. However when I read your comment this morning I realised my mistake. I was filtering out the bits of the output level, causing them to become 0, or maximum output level on the OPL2. No wonder I got clicks and pops all over. It's a great trap this inverse volume! :)

I have an alternative setInstrument like this for now and I will provide an update of the sketch in the upcoming days.

void setInstrument(byte opl2Channel, const unsigned char *instrument) {
  const byte instrumentBaseRegs[11] = {
    0x20, 0x40, 0x60, 0x80, 0xE0, 0xC0
  };

  for (byte i = 0; i < 11; i ++) {
    byte reg;
    if (i == 5) {
      //Channel parameters C0..C8
      reg = 0xC0 + max(0x00, min(opl2Channel, 0x08));
    } else {
      //Operator parameters 20..35, 40..55, 60..75, 80..95, E0..F5
      reg = instrumentBaseRegs[i % 6] + opl2.getRegisterOffset(opl2Channel, i > 5);
    }

    byte val = pgm_read_byte_near(instrument + i + 1);
    if (i == 1) {
      channelMap[opl2Channel].op1Level = (float)(63 - (val & 0x3F)) / 63.0;
      val = (val & 0xC0) | 0x3F;
    } else if (i == 7) {
      channelMap[opl2Channel].op2Level = (float)(63 - (val & 0x3F)) / 63.0;
      val = (val & 0xC0) | 0x3F;
    }
    opl2.setRegister(reg, val);
  }
}
virtuaCode commented 5 years ago

I would not recommend to set the output level to zero. For example when the output level is currently high and we reset it to zero then there will be again loud clicking noises. Instead just set the output to the current value: (val & 0xC0) | (getRegister(reg) & 0x3F)

This gave me the best result so far :)

DhrBaksteen commented 5 years ago

Yes that works as well