problem transmitting some 'x...' values

[nosuchtim]

Trying to transmit 'xb06500b06400' to another instance of keykit doesn't work.

Also, transmitting 'xb065' (which is an incomplete message) ends up being received as 'xb065XX', where XX are bytes that shouldn't be included.

[pbarada]

How do I setup two keykits to talk to each other(e.g. how do I reproduce the issue on Linux)? Might turn into another test case...

[nosuchtim]

Just invoke key twice, and in one of the instances open a port for input, and in the second instance open the same port for output. In the first instance type “midimon()” in the console, and in the second instance type phrase constants (e.g. ‘a’ ) which should show up in the midimon output of the first. In the case of a note like ‘a’ you’ll see the note on and note off. The behavior of things like ‘xb064006500’ (which aren’t really sysex, they’re just using the ‘x…’ note notation) is broken. I’m traveling today so won’t be able to debug it till later. It’s likely to be Windows-specific due to the new use of rtmidi instead of portmidi, or something about running status.

  …Tim…

On Fri, Mar 24, 2023 at 4:22 AM pbarada @.***> wrote:

How do I setup two keykits to talk to each other(e.g. how do I reproduce the issue on Linux)? Might turn into another test case...

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1482647647, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABESNSXPO4QKQ2D5OVPR5LW5V7YBANCNFSM6AAAAAAWF3RZPI . You are receiving this because you authored the thread.Message ID: @.***>

[pbarada]

Interesting. On Ubuntu I fired up two instances (labeling them "K#" where "#" is the instance, "Shell" is a bash shell): K1: Through Port Enabler enable "in 1" (causes ALSA to list keykit connection 128:0 K2: Through Port Enabler enable "out 1" (causes ALSA to list keykit connection 129:0 Shell: "aconnect 129:0 128:0" (to connect K2 to K1) K1: "midimon()" (no double quotes) K2: Type "'xb064006500'" (no double quotes) K1 Get "'xb06400P65t43468,10'" (no double quotes)

Adding debug into mdep/linux/midi.c to dump the bytes presented to snd_midi_event_encode shows: mdep_putnmidi:303 5 mdep_putnmidi:307 b0 64 00 65 00

Dumping the bytes received from snd_midi_event_decode shows: mdep_getnmidi:269 3 mdep_getnmidi:273 b0 64 00

Indicating ALSA isn't properly handling running status on controller change messages correctly. I'll poke around in ALSA to see if I can understand how it handles controller messages better...

[pbarada]

Turns out the Linux/ALSA port is broken. In linux putnmidi() it calls snd_midi_event_encode and hands it the entire midi buffer (which can include multiple MIDI mesages). Digging into seq_midi_event.c, snd_midi_event_encode only encodes the first MIDI message into an event and returns. I'm working on a change to instead spoon feed the MIDI data one byte at a time to snd_midi_event_encode_byte() (which snd_midi_event_encode calls...) and when it returns a 1, call snd_seq_event_output() on the just-encoded event. Repeat over the rest of the bytes. With this change I see both controller MIDI messages showing up from midimon(). I also want to see about how to handle calls to reset the encoder periodically (since it can break running status from previous putnmidi() calls).

I have to do some more testing to make sure I'm not creating a bigger problem...

[nosuchtim]

I think internally to Keykit the intention (if I had to guess my intention from several decades ago :-) is to interpret running status on input and produce (in this example) two complete notes (or controller messages) internally. But it’s possible that’s broken.

   …Tim…

On Fri, Mar 24, 2023 at 12:46 PM pbarada @.***> wrote:

Interesting. On Ubuntu I fired up two instances (labeling them "K#" where "#" is the instance, "Shell" is a bash shell): K1: Through Port Enabler enable "in 1" (causes ALSA to list keykit connection 128:0 K2: Through Port Enabler enable "out 1" (causes ALSA to list keykit connection 129:0 Shell: "aconnect 129:0 128:0" (to connect K2 to K1) K1: "midimon()" (no double quotes) K2: Type "'xb064006500'" (no double quotes) K1 Get "'xb06400P65t43468,10'" (no double quotes)

Adding debug into mdep/linux/midi.c to dump the bytes presented to snd_midi_event_encode shows: mdep_putnmidi:303 5 mdep_putnmidi:307 b0 64 00 65 00

while dumping the bytes received from snd_midi_event_decode shows: mdep_getnmidi:269 3 mdep_getnmidi:273 b0 64 00

Indicating ALSA isn't properly handling running status on controller change messages correctly. I'll poke around in ALSA to see if I can understand how it handles controller messages better...

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1483328090, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABESNV6YZUDM26CMCK2UV3W5X22TANCNFSM6AAAAAAWF3RZPI . You are receiving this because you authored the thread.Message ID: @.***>

[nosuchtim]

Wouldn’t be surprising for rtmidi (which is newly introduced in the 64-bit Windows version) to have the same problem.

On Fri, Mar 24, 2023 at 1:27 PM pbarada @.***> wrote:

Turns out the Linux/ALSA port is broken. In linux putnmidi() it calls snd_midi_event_encode and hands it the entire midi buffer (which can include multiple MIDI mesages). Digging into seq_midi_event.c, snd_midi_event_encode only encodes the first MIDI message into an event and returns. I'm working on a change to instead spoon feed the MIDI data one byte at a time to snd_midi_event_encode_byte() (which snd_midi_event_encode calls...) and when it returns a 1, call snd_seq_event_output() on the just-encoded event. Repeat over the rest of the bytes. With this change I see both controller MIDI messages showing up from midimon(). I also want to see about how to handle calls to reset the encoder periodically (since it can break running status from previous putnmidi() calls).

I have to do some more testing to make sure I'm not creating a bigger problem...

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1483370342, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABESNWPM5IGM5LMN4DKQMLW5X7TTANCNFSM6AAAAAAWF3RZPI . You are receiving this because you authored the thread.Message ID: @.***>

[pbarada]

In the Linux ALSA implementation, the problem I currently see is on the encode side - as in encoding a MIDI stream to ALSA sequencer events. With my tweaks (thus far), if I send 'xb064006500' from one keykit instance, midimon() on the the other instance produces: 'xb06400P65t16037.10' 'xb06500P65t16037.10'

which I believe is expected. My concern is if I don't "reset" the stream then running status can be go for quite a while - what I think would make sense is to have a "reset running status" variable that causes the output side to reset tracking of running status - rather than assuming receiver can track running status forever. Issue is mdep_putnmidi() is (probably?) too low a level to force the running status reset (need to dig deeper to know for sure). I'll do some testing over weekend and create another pull request for the ALSA chances. Any idea if there are people using keykit w/ALSA that can beat on it?

Next step (after fixing this) is to implement some type of copy/log for console so its simpler to capture the console output (figure a menu item to burn the console scroll content to a file) since copy/paste on various targets is tedious at best.

[pbarada]

With my above changes (so far), if I split the controller message stream into 'xb064' then '00' midimon() in the other instance finally shows the full controller event 'xb06500P65Txxx,10', so looks like I'm on the right path(ALSA wise)...

[pbarada]

Tim,

See https://github.com/nosuchtim/keykit/commit/b3ad39e02c8713331e22bf0097c52ba2cf64c02d for base changes to fix ALSA MIDI output (and handle your 'xb064006500' test on Ubuntu).  This isn't complete, still need to better understand context snd_midi_event_reset_encode is needed; figure it should be done periodically, but only when know when a MIDI output buffer has a starting MIDI status.

On 3/24/23 16:36, Tim Thompson wrote:

Wouldn’t be surprising for rtmidi (which is newly introduced in the 64-bit Windows version) to have the same problem.

On Fri, Mar 24, 2023 at 1:27 PM pbarada @.***> wrote:

Turns out the Linux/ALSA port is broken. In linux putnmidi() it calls snd_midi_event_encode and hands it the entire midi buffer (which can include multiple MIDI mesages). Digging into seq_midi_event.c, snd_midi_event_encode only encodes the first MIDI message into an event and returns. I'm working on a change to instead spoon feed the MIDI data one byte at a time to snd_midi_event_encode_byte() (which snd_midi_event_encode calls...) and when it returns a 1, call snd_seq_event_output() on the just-encoded event. Repeat over the rest of the bytes. With this change I see both controller MIDI messages showing up from midimon(). I also want to see about how to handle calls to reset the encoder periodically (since it can break running status from previous putnmidi() calls).

I have to do some more testing to make sure I'm not creating a bigger problem...

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1483370342, or unsubscribe

https://github.com/notifications/unsubscribe-auth/AABESNWPM5IGM5LMN4DKQMLW5X7TTANCNFSM6AAAAAAWF3RZPI . You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1483377634, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXGS2YIWOUWMDEQ5M22LTW5YATLANCNFSM6AAAAAAWF3RZPI. You are receiving this because you commented.Message ID: @.***>

-- Peter Barada @.***

[nosuchtim]

Okay, I think I've got the Windows version addressing these issues.

It should now handle running status output (e.g. you can send 'xb064006500'), however note that when you send running status to a second key.exe instance, it will be automatically turned into complete messages on input. So, it's hard to confirm that it's sending running status unless you use a debugger or have another program that doesn't do this automatic transformation on input.

Multiple messages can now be sent in a single value, i.e. 'xb06400b06500' or (using running status) 'xb065006400'. Incomplete messages on Windows are not supported, so if you try to send 'xb0' it will produce an error message and not send anything.

[vizicist]

Peter - what issues if any remain in the Linux version?

On Sun, Mar 26, 2023 at 3:42 PM Tim Thompson @.***> wrote:

Okay, I think I've got the Windows version addressing these issues.

It should now handle running status output (e.g. you can send 'xb064006500'), however note that when you send running status to a second key.exe instance, it will be automatically turned into complete messages on input. So, it's hard to confirm that it's sending running status unless you use a debugger or have another program that doesn't do this automatic transformation on input.

Multiple messages can now be sent in a single value, i.e. 'xb06400b06500' or (using running status) 'xb065006400'. Incomplete messages on Windows are not supported, so if you try to send 'xb0' it will produce an error message and not send anything.

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1484247083, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANZ5NKJWEEGURYXC4J7AXBTW6DA5DANCNFSM6AAAAAAWF3RZPI . You are receiving this because you commented.Message ID: @.***>

[pbarada]

Tim,

Unfortunately your latest change prevents the linux version from starting.  FIring up GDB on a non-optimized version, I see it die in execerror:

(gdb) b execerror Breakpoint 1 at 0x55555557ae95: file main.c, line 471. (gdb) set target-async on Warning: 'set target-async', an alias for the command 'set mi-async', is deprecated. Use 'set mi-async'.

(gdb) run Starting program: /home/peter/src/keykit/keykit.github/bin/key [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". [Detaching after vfork from child process 506444] [Detaching after vfork from child process 506446]

Breakpoint 1, execerror (fmt=0x5555555b5b21 "no value for variable \"%s\", \n") at main.c:471 (gdb) up

1  0x0000555555596f76 in i_lvareval () at task.c:789

(gdb) p s->name->u.str $1 = (Symstr) 0x5555556bfa10 "keysize" (gdb) p d $2 = {type = 1, u = {val = 9223372036854775807, dbl = -nan(0x7fffff),     str = 0x7fffffffffffffff <error: Cannot access memory at address 0x7fffffffffffffff>, sym = 0x7fffffffffffffff, phr = 0x7fffffffffffffff,     arr = 0x7fffffffffffffff,     codep = 0x7fffffffffffffff <error: Cannot access memory at address 0x7fffffffffffffff>, frm = 0x7fffffffffffffff, datum = 0x7fffffffffffffff,     note = 0x7fffffffffffffff, task = 0x7fffffffffffffff,     fifo = 0x7fffffffffffffff, wind = 0x7fffffffffffffff,     obj = 0x7fffffffffffffff}} (gdb)

I believe the issue is exacerbated by linux mdep_mdep returning Noval.  I'll dig deeper tonight/tomorrow by commenting out the added code in keyrc(that calls mdep()) and instead try following at the console:

{ keysize = mdep("env", "get", "KEYSIZE); if (keysize != "") { eval "TMP="+keysize; screen("size",TMP); } }

to see what happens....

On 3/26/23 18:43, Tim Thompson wrote:

Peter - what issues if any remain in the Linux version?

On Sun, Mar 26, 2023 at 3:42 PM Tim Thompson @.***> wrote:

Okay, I think I've got the Windows version addressing these issues.

It should now handle running status output (e.g. you can send 'xb064006500'), however note that when you send running status to a second key.exe instance, it will be automatically turned into complete messages on input. So, it's hard to confirm that it's sending running status unless you use a debugger or have another program that doesn't do this automatic transformation on input.

Multiple messages can now be sent in a single value, i.e. 'xb06400b06500' or (using running status) 'xb065006400'. Incomplete messages on Windows are not supported, so if you try to send 'xb0' it will produce an error message and not send anything.

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1484247083, or unsubscribe

https://github.com/notifications/unsubscribe-auth/ANZ5NKJWEEGURYXC4J7AXBTW6DA5DANCNFSM6AAAAAAWF3RZPI . You are receiving this because you commented.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1484247396, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXGSZOILIEZXQ6M4CF753W6DBCVANCNFSM6AAAAAAWF3RZPI. You are receiving this because you commented.Message ID: @.***>

-- Peter Barada @.***

[nosuchtim]

The mdep_mdep() routine in the linux version needs to implement the "env" stuff to get environment variables, as exemplified in the mdep_mdep() of the windows version (which is in nt/midi.c).

[nosuchtim]

Looks like I need to set up a Linux box at home so I can find these things sooner.

[pbarada]

Hmm, guessing all targets need to get updated to recognize ‘mdep(“env”,…)’ - and update the doco library as well. Guess at minima to return an empty string.Splat fingered from my iPhoneOn Mar 26, 2023, at 9:25 PM, Tim Thompson @.***> wrote: The mdep_mdep() routine in the linux version needs to implement the "env" stuff to get environment variables, as exemplified in the mdep_mdep() of the windows version (which is in nt/midi.c).

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[pbarada]

Tim,

Would it be simpler to modify library code to check that the return value from mdep() is defined see attached patch)?

Another aside, with the attached patch, keykit startup in Linux is much slower now - takes about five seconds before the console is painted, I wonder where the time is being spend during boot; will take a look in the morning.

On 3/26/23 22:55, Peter Barada wrote:

Hmm, guessing all targets need to get updated to recognize ‘mdep(“env”,…)’ - and update the doco library as well. Guess at minima to return an empty string.

Splat fingered from my iPhone

On Mar 26, 2023, at 9:25 PM, Tim Thompson @.***> wrote:



The mdep_mdep() routine in the linux version needs to implement the "env" stuff to get environment variables, as exemplified in the mdep_mdep() of the windows version (which is in nt/midi.c).

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1484332231, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXGS7SP6CGQCRMDBHVE4LW6DUATANCNFSM6AAAAAAWF3RZPI. You are receiving this because you commented.Message ID: @.***>

-- Peter Barada @.***

[pbarada]

Tim,

My bad - the slowdown was caused by not compiling with -O (to make debug work better)...

On 3/26/23 23:37, Peter Barada wrote:

Tim,

Would it be simpler to modify library code to check that the return value from mdep() is defined see attached patch)?

Another aside, with the attached patch, keykit startup in Linux is much slower now - takes about five seconds before the console is painted, I wonder where the time is being spend during boot; will take a look in the morning.

On 3/26/23 22:55, Peter Barada wrote:

Hmm, guessing all targets need to get updated to recognize ‘mdep(“env”,…)’ - and update the doco library as well. Guess at minima to return an empty string.

Splat fingered from my iPhone

On Mar 26, 2023, at 9:25 PM, Tim Thompson @.***> wrote:



The mdep_mdep() routine in the linux version needs to implement the "env" stuff to get environment variables, as exemplified in the mdep_mdep() of the windows version (which is in nt/midi.c).

— Reply to this email directly, view it on GitHub https://github.com/nosuchtim/keykit/issues/13#issuecomment-1484332231, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXGS7SP6CGQCRMDBHVE4LW6DUATANCNFSM6AAAAAAWF3RZPI. You are receiving this because you commented.Message ID: @.***>

-- Peter Barada @.***

-- Peter Barada @.***

[nosuchtim]

Would it be simpler to modify library code to check that the return value from mdep() is defined see attached patch)?

Yes, that's certainly better in the long run. ...Tim...

Message ID: @.***>