monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
633 stars 147 forks source link

Fix/enable oneway midi #1595

Closed catfact closed 1 year ago

catfact commented 2 years ago

attempts to address issue #1449

seems pretty straightforward, but (given a very small time window) i'm stumped for the moment: seems OK except for the case of disconnecting devices with no input ports. this causes a crash (!) so it is not an improvement over main right now.

testing with a korg sq-1. we're hitting this OK, setting up input handle as NULL: https://github.com/monome/norns/blob/fix/enable-oneway-midi/matron/src/device/device_midi.c#L80

and therefore we're setting the input loop function to NULL and hitting this: https://github.com/monome/norns/blob/fix/enable-oneway-midi/matron/src/device/device.c#L103

i don't see this being hit, which is good: https://github.com/monome/norns/blob/fix/enable-oneway-midi/matron/src/device/device_midi.c#L307

... and yet, somehow, i am crashing on disconnect with:

midi inconsistency for device: SQ-1 1
ALSA lib rawmidi_hw.c:111:(snd_rawmidi_hw_status) SNDRV_RAWMIDI_IOCTL_STATUS failed: No such device

the "midi inconsistency" line seems only to be coming from the input reader thread, which AFAIK we never started: https://github.com/monome/norns/blob/fix/enable-oneway-midi/matron/src/device/device_midi.c#L319

so what gives?

[edit] oop, nvm. the "inconsistency" is for the other endpoint, and is benign. the crash is somewhere else. time to GDB.

catfact commented 2 years ago

(i will come back to this when next i have some time but can't promise when that would be. by all means, take this issue from me if you like.)

catfact commented 2 years ago

further examination:

is weird. using SQ-1 which shows as two devices SQ-1 (1) is bidrectional and SQ-1 (2) is output-only.

crash is actually on disconnecting the first endpoint. used to work. something about initializing the 2nd device is corrupting the handles for the 1st device?

connect:

device_monitor(): adding midi device SQ-1
device_monitor(): adding midi device SQ-1
device opened (bidirectional)
input handle  : 0x68923c50
output handle : 0x68923c80
assigned device name with port: SQ-1 1
starting input thread for midi device: SQ-1 1 (0x68923c50)
_norns.midi.add: 2, SQ-1 1, userdata: 0x689237b8
ALSA lib rawmidi_hw.c:235:(snd_rawmidi_hw_open) open /dev/snd/midiC1D0 failed: No such device
device opened OK, only output ports
input handle  : (nil)
output handle : 0x68924040
assigned device name with port: SQ-1 2
device.c: no `start` function defined (no input); skipping
_norns.midi.add: 3, SQ-1 2, userdata: 0x68923d90

disconnect:

midi inconsistency for device: SQ-1 1
ALSA lib rawmidi_hw.c:111:(snd_rawmidi_hw_status) SNDRV_RAWMIDI_IOCTL_STATUS failed: No such device
midi_deinit   : SQ-1 1
input handle  : 0x68923c50
output handle : 0x68923c80
Segmentation fault

now i really do have to move along for time being.

forrestbaer commented 2 years ago

Wow this is a great start, I just wanted to chime in and say that I tested it with my USB2MIDI interface (usb -> 5 trs) and it is working great, I still see the errors about snd_rawmidi_hw_open but that doesn't seem to matter it's working great and I see 5 midi outputs options under devices.

🎉 🎉 🎉

device_monitor(): adding midi device MSW_USB
ALSA lib rawmidi_hw.c:235:(snd_rawmidi_hw_open) open /dev/snd/midiC1D0 failed: No such device
device opened OK, only output ports
input handle  : (nil)
output handle : 0x59ff90
assigned device name with port: MSW_USB 1

etc, same for devices 2-5. Can't say I've been able to reproduce the crash. Thanks for making an effort on this!

catfact commented 2 years ago

weird ok thanks for looking. i'l get back to it in a few days. crash i'm seeing is surprising to me, i wonder if there is anyone else with SQ-1 who can try and reproduce.

just quick note here that the _errors about snd_rawmidi_hw_open_ are coming from inside that function. here they are not really errors - we are checking and repeating that call to determine which I/O handles are functioning - but can't easily supress that error message.

catfact commented 1 year ago

i did not manage to come back to this and in fact haven't set up norns HW since i moved house.

its a very annoying issue; is it possible that any other developers might be able to test and try to repro my crash? i can even ship a SQ-1 to you. particularly paging anyone who's contributed to the midi device module: @ngwese , @patriciogonzalezvivo, @ryanlaws, in addition of course to @dndrks / @tehn .

tehn commented 1 year ago

short of sending hardware around, is there an assembled list of midi objects that exhibit this problem? (so that i might simply acquire something that also might be of future use)

catfact commented 1 year ago

the only one i have is korg sq-1. (actually i purchased it for this issue.) they run about $80 used and are really quite handy (can be used as a simple CV knob box with 2 banks of 8 outputs.) so that'd be my recomendation. (i thought they were kinda ubiquitous and would be a good reference point but i guess maybe not.)

not sure what else. someone said logidy umi-3 footswitch controller didnt work, and i thought someone else did have it working. (i own one and just need to try connecting it with norns.)

other cases reported are with fancy synthesizers.

tehn commented 1 year ago

got it. just ordered. will look into this on arrival.

catfact commented 1 year ago

finally got norns HW up and running. (reflashed stock silver unit to latest image, updated, pulled/built main.)

seems that logidy umi-3 does also surface this issue. (i also got one pretty much for this issue, but it's a nice thing for laptop music too.)

i'll try a debug session with the footswitch too.

tehn commented 1 year ago

sq-1 should arrive monday here

On Fri, Mar 24, 2023 at 5:59 PM ezra buchla @.***> wrote:

finally got norns HW up and running. (reflashed stock silver unit to latest image, updated, pulled/built main.)

seems that logidy umi-3 https://www.logidy.com/umi3 does also surface this issue. (i also got one pretty much for this issue, but it's a nice thing for laptop music too.)

— Reply to this email directly, view it on GitHub https://github.com/monome/norns/pull/1595#issuecomment-1483474728, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4I4GCTINRSA6MG4BUBKDW5YKLNANCNFSM55XFGLDQ . You are receiving this because you were mentioned.Message ID: @.***>

m-r-m-s commented 1 year ago

sq-1 should arrive monday here … On Fri, Mar 24, 2023 at 5:59 PM ezra buchla @.> wrote: finally got norns HW up and running. (reflashed stock silver unit to latest image, updated, pulled/built main.) seems that logidy umi-3 https://www.logidy.com/umi3 does also surface this issue. (i also got one pretty much for this issue, but it's a nice thing for laptop music too.) — Reply to this email directly, view it on GitHub <#1595 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4I4GCTINRSA6MG4BUBKDW5YKLNANCNFSM55XFGLDQ . You are receiving this because you were mentioned.Message ID: @.>

Not sure if this fix will make it to the latest Norns image release but I am an SQ-1 enthusiast and can test on my end if it would help at some point.

tehn commented 1 year ago

i now have an sq-1. just need to find time to investigate.

m-r-m-s commented 1 year ago

Sounds good and thanks for all your efforts as always 🙌

catfact commented 1 year ago

data point, this awful $12 midi interface also apparently enumerates as 2 separate unidirectional devices.

https://www.amazon.com/TECKEEN-Keyboard-Recording-Converter-Interface/dp/B0BKQ5CDRT/ref=sr_1_17?keywords=usb+midi+cable&qid=1687615736&sprefix=usb+midi+ca%2Caps%2C222&sr=8-17

catfact commented 1 year ago

did anyone get a chance to test this?

tehn commented 1 year ago

ha, i still have an unopened sq-1 ;)

will get this on the list

catfact commented 1 year ago

this has been languishing a long time. can anyone test my changes? i will also try and look at it again with the logidy device instead of sq-1.

to recap:

tehn commented 1 year ago

merged main (screen), recompiled this branch

in the menu (virtual devices) i see SQ-1 and SQ-2

disconnecting the device crashes matron.

this is my log from matron:

dev_monitor: adding midi device SQ-1
dev_monitor: adding midi device SQ-1
device opened (bidirectional)
input handle  : 0x71b23378
output handle : 0x71b233a8
assigned device name with port: SQ-1 1
_norns.midi.add: 2, SQ-1 1, userdata: 0x71b22ec8
ALSA lib rawmidi_hw.c:235:(snd_rawmidi_hw_open) open /dev/snd/midiC1D0 failed: No such device
device opened OK, only output ports
input handle  : (nil)
output handle : 0x71b23768
assigned device name with port: SQ-1 2
device.c: no `start` function defined (no input); skipping
_norns.midi.add: 3, SQ-1 2, userdata: 0x71b234b8
starting input thread for midi device: SQ-1 1 (0x71b23378)
midi inconsistency for device: SQ-1 1
ALSA lib rawmidi_hw.c:111:(snd_rawmidi_hw_status) SNDRV_RAWMIDI_IOCTL_STATUS failed: No such device
midi_deinit   : SQ-1 1
input handle  : 0x71b23378
output handle : 0x71b233a8
Segmentation fault
catfact commented 1 year ago

ok, same as me. thanks! i'll tey and get back to it.

m-r-m-s commented 1 year ago

this has been languishing a long time. can anyone test my changes? i will also try and look at it again with the logidy device instead of sq-1.

to recap:

* i felt like i had addressed the issue properly with these changes, in theory

* but, i get a crash on disconnect with my test device

* others have not repro'd the crash but also our devices are different

Hey yall! Thanks so much for trying to sort this issue out with the SQ-1 ! To clarify: the matron crash comes from a physical disconnect of the SQ-1 hardware? This may be already known but wondering if it could provide a clue to the issue: Norns would see via the SQ-1 USB connection the SQ-1's A sequence on the MIDI channel assigned, and B sequence on the assigned channel+1. Could that channel+1 be causing the crash? Sorting this out is way above my experience level but thought I would share just in case it is helpful.

catfact commented 1 year ago

To clarify: the matron crash comes from a physical disconnect of the SQ-1 hardware?

yes.

i'm not sure i quite follow the rest of your comment; don't think the channel number is relevant but i'll keep it in mind!


gonna see if i can catch the segfault and if the trace is interesting. (i must have done this before but forgot.)

initial notes-to-self for productive GDB session (add to ~/.gdbinit):

handle SIG32 nostop noprint
set print thread-events off
set print inferior-events off
catfact commented 1 year ago

initial stacktrace...

Thread 18 "matron" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xa70ff280 (LWP 2053)]
0xb6f9c440 in __pthread_kill (threadid=0, signo=signo@entry=0) at ../sysdeps/unix/sysv/linux/pthread_kill.c:40
40  ../sysdeps/unix/sysv/linux/pthread_kill.c: No such file or directory.
(gdb) bt
#0  0xb6f9c440 in __pthread_kill (threadid=0, signo=signo@entry=0) at ../sysdeps/unix/sysv/linux/pthread_kill.c:40
#1  0x0004651c in dev_delete (d=0xb2623a48) at ../matron/src/device/device.c:74
#2  0x00047014 in dev_list_remove_node (event_remove=<optimized out>, dn=0xb2623978) at ../matron/src/device/device_list.c:147
#3  dev_list_remove (type=type@entry=DEV_TYPE_MIDI, node=0xb2623c08 "/dev/snd/midiC1D0", node@entry=0x0)
    at ../matron/src/device/device_list.c:163
#4  0x00047eac in rm_dev (dev_file=2, dev=0xb2609a40) at ../matron/src/device/device_monitor.c:221
#5  watch_loop (p=<optimized out>) at ../matron/src/device/device_monitor.c:192
#6  0xb6f92300 in start_thread (arg=0xa70ff280) at pthread_create.c:477
#7  0xb68af208 in ?? () at ../sysdeps/unix/sysv/linux/arm/clone.S:73 from /lib/arm-linux-gnueabihf/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

... ok, i'm getting the picture

in frame 1:

#1  0x0004651c in dev_delete (d=0xb2623a48) at ../matron/src/device/device.c:74
74      if (pthread_kill(d->base.tid, 0) == 0) {
(gdb) p d->base.tid
$3 = 0

thread IDs are supposed to be opaque. but tid == 0 sounds wrong. and according to some person on SO, "Passing an invalid thread id to pthread_kill invokes undefined behavior."

and i'm assuming we didn't assign an input thread because this is an output-only device, or something, will look at that next

catfact commented 1 year ago

this still needs cleanup with all the print statements, but seems to be fixed by 67d31cba76ece1718f86bb59f4157577f36830d6 ?

tehn commented 1 year ago

fantastic--- code looks good, will test tomorrow with hardward and merge into the beta. thank you tons

catfact commented 1 year ago

Sorry just noticed that a bunch of other crap snuck in here. Will try and revert irrelevant additions.

tehn commented 1 year ago

sorry i didn't get to this yesterday. i can clean it up later today if you don't get to it first.

catfact commented 1 year ago

pushed cleanup but haven't tested just yet on HW to make sure (e.g.) supercollider doesn't fall down due to a typo

tehn commented 1 year ago

tested on hardware, works well. thank you!