dagargo / overwitch

JACK client for Overbridge devices
GNU General Public License v3.0
127 stars 15 forks source link

Digitakt not reacting to Program Change messages #12

Closed lentferj closed 2 years ago

lentferj commented 2 years ago

I am trying to send pc messages from my MPC One to my Digitakt, but it doesn't work.

The problem is that the current implementation seems to be wrong.

https://github.com/dagargo/overwitch/blob/master/src/jclient.c#L407-L430

If you add matching printf's to the code

diff --git a/src/jclient.c b/src/jclient.c
index beae4d6..0bcd6c6 100644
--- a/src/jclient.c
+++ b/src/jclient.c
@@ -419,6 +419,7 @@ jclient_j2o_midi (struct jclient *jclient, jack_nframes_t nframes)
       oevent.bytes[0] = 0;
       jack_midi_event_get (&jevent, midi_port_buf, i);
       status_byte = jevent.buffer[0];
+      printf ("jevent.size: %lu, 1st status_byte: %#x\n", jevent.size, status_byte);

       if (jevent.size == 1 && status_byte >= 0xf8 && status_byte <= 0xfc)
        {
@@ -427,6 +428,7 @@ jclient_j2o_midi (struct jclient *jclient, jack_nframes_t nframes)
        }
       else if (jevent.size == 3)
        {
+          printf ("2nd status_byte: %#x\n", status_byte);
          switch (status_byte & 0xf0)
            {
            case 0x80:          //Note-off
@@ -442,6 +444,7 @@ jclient_j2o_midi (struct jclient *jclient, jack_nframes_t nframes)
              oevent.bytes[0] = 0x0b;
              break;
            case 0xc0:          //Program Change
+             printf("Program Change received\n");
              oevent.bytes[0] = 0x0c;
              break;
            case 0xd0:          //Channel Pressure

you will see that program change is not a 3 byte message, but a two byte message.

jevent.size: 2, 1st status_byte: 0xc9
jevent.size: 2, 1st status_byte: 0xc9
jevent.size: 2, 1st status_byte: 0xc9
jevent.size: 2, 1st status_byte: 0xc9
jevent.size: 2, 1st status_byte: 0xc9

It is also defined as such [https://www.recordingblogs.com/wiki/midi-program-change-message]

lentferj commented 2 years ago

I don't fully understand the logic yet, but this works for me

diff --git a/src/jclient.c b/src/jclient.c
index beae4d6..f35a7ce 100644
--- a/src/jclient.c
+++ b/src/jclient.c
@@ -419,12 +419,25 @@ jclient_j2o_midi (struct jclient *jclient, jack_nframes_t nframes)
       oevent.bytes[0] = 0;
       jack_midi_event_get (&jevent, midi_port_buf, i);
       status_byte = jevent.buffer[0];
+      // printf ("jevent.size: %lu, 1st status_byte: %#x\n", jevent.size, status_byte);

       if (jevent.size == 1 && status_byte >= 0xf8 && status_byte <= 0xfc)
        {
          oevent.bytes[0] = 0x0f;       //Single Byte
          oevent.bytes[1] = jevent.buffer[0];
        }
+      else if (jevent.size == 2)
+       {
+         if (status_byte >= 0xc0 && status_byte <= 0xcf)
+             {
+              printf("Program Change message received.\n");
+              oevent.bytes[0] = 0x0c; 
+              oevent.bytes[1] = jevent.buffer[0];
+              oevent.bytes[2] = jevent.buffer[1];
+              oevent.bytes[3] = jevent.buffer[2];
+              printf("jbuffer0: %#x, jbuffer1: %#x\n", jevent.buffer[0], jevent.buffer[1]);
+            }
+      }
       else if (jevent.size == 3)
        {
          switch (status_byte & 0xf0)
@@ -441,9 +454,9 @@ jclient_j2o_midi (struct jclient *jclient, jack_nframes_t nframes)
            case 0xb0:          //Control Change
              oevent.bytes[0] = 0x0b;
              break;
-           case 0xc0:          //Program Change
-             oevent.bytes[0] = 0x0c;
-             break;
+           //case 0xc0:                //Program Change
+           //  oevent.bytes[0] = 0x0c;
+           //  break;
            case 0xd0:          //Channel Pressure
              oevent.bytes[0] = 0x0d;
              break;

Why does oevent.bytes[0] has to be set to 0x0c?

lentferj commented 2 years ago

oevents.bytes[3] doesn't have to be filled, as it is only a 2-byte message, iiuc.So that line can be discarded.

lentferj commented 2 years ago

So, I set up PR #13. I hope this is the correct approach to fix it. I haven't checked all the other messages below jevent.size == 3 yet.

lentferj commented 2 years ago

OK, #13 wasn't really nice and didn't fit in the logic of the code. #14 should be better.

dagargo commented 2 years ago

14 has been merged.

dagargo commented 2 years ago

Same thing was happening with Channel Pressure (After-touch) messages. Fixed in 8ae3c01110adb8ad821b0f46d7fe3483ce492d95.