MIDI management - Githubissues

tehn commented 6 years ago

we need to design a versatile MIDI management system, or set up some best practices.

example use case:

controller sends cc data to PARAM menu (midi managed by menu)
same controller triggers notes in earthsea (midi managed by user script)
same controller is outputting clock sync (midi managed by library)

presently it's a mess. i added a hack to accommodate simultaneous use of the first two cases: https://github.com/monome/norns/blob/3ea866906d23bdefeb466305d6d37ee030af1558/lua/midi.lua#L83

the third could be accommodated by nesting a function send (midi pass-thru) within a user-script function. i just did this in playfair to use BeatClock

tehn commented 6 years ago

but maybe it's ok as is.

okyeron commented 6 years ago

Is this a good place for midi bugs?

Something odd is happening with adding/removing midi devices I added "print("fairplay: midi device added", dev.id, dev.name)" to the playfair script since I was seeing this in other user scripts and wanted to confirm it.

Something seems to be holding on to the usb port each time a device is plugged/unplugged. When I reconnect it gets a new id. lsusb shows the device ID increasing each time I unplug/re-plug the device.

reconnecting midi...
fairplay: midi device added 1   Boutique
fairplay: midi device added 2   TR-8
fairplay: midi device added 3   TR-8
fairplay: midi device added 4   TR-8
fairplay: midi device added 6   TR-8
READING PMAP
dev_delete(): removing device 5
fairplay: midi device added 7   TR-8

catfact commented 6 years ago

that's actually by design. the ID number is totally arbitrary and we made it monotonically increasing. eventually i suppose it could wrap and that would be fine. it does not attempt to uniquely identify a hardware device.

Something seems to be holding on to the usb port

any other evidence of this?

can you describe the series of actions that produced your output?

okyeron commented 6 years ago

OK from a user perspective - from maiden it appears that I have a device connected that is not actually connected - in the above "Boutique" (TR-09) was no longer connected. Why would it show me dead connections?

catfact commented 6 years ago

its true that on "reconnecting..." it should not show things that are not connected, that would be a bug. it would help me understand if i knew what actions produced that output. (you just mentioned that you get a new ID on each connection, which is intended.)

you also mention lsusb but that just looks like output from lua (ID assigned by matron, not the kernel.)

the dev_delete() output shows that matron at least is aware of something being removed.

okyeron commented 6 years ago

I think it's related to another script not properly releasing the midi device (?)

With justmat/foulplay.lua I noticed the following on running the script then plugging in the Tr-8, unplugging it and replugging it again:

reconnecting midi...
READING PMAP
foulplay: midi device added 1   TR-8
dev_delete(): removing device 0
foulplay: midi device removed   1   TR-8
lua: /home/we/dust/scripts/justmat/foulplay.lua:308: attempt to index a nil value (local 'dev')
stack traceback:
    /home/we/dust/scripts/justmat/foulplay.lua:308: in function 'midi.remove'
    /home/we/norns/lua/midi.lua:68: in function </home/we/norns/lua/midi.lua:62>
foulplay: midi device added 2   TR-8

at which point the old connection does not get cleaned up until I stop/start norns again.

EDIT: Perhaps his use of midi.remove is now not needed and causing this error?

catfact commented 6 years ago

ok gotcha. looks like removal callback is possibly firing twice (?) - 1st time OK, 2nd time device is nil because it's already been removed from the Midi module device list.

it's not obvious to me how that would happen. it is possible to have two callbacks - the global callback, used here, and a callback per device (https://github.com/monome/norns/blob/master/lua/midi.lua#L67). but i don't see where it would be possible to call either one with a nil argument.

catfact commented 6 years ago

and, i dunno if this is related, but foulplay as written won't work right with multiple devices. it is just using the global callback for device add/remove, and isn't checking the device.

so one thing that would happen is:

plug first device. foulplay starts using it (assigns local midi_dev)
plug second device. foulplay uses that.
unplug second device. foulplay now uses nothing (assigns local midi_dev to nil)

okyeron commented 6 years ago

(I'm message justmat regarding the above)

MIDI thing number 2 for today:

playfair using BeatClock sounds just a bit laggy behind my TR-9 (just playing a 4x4 kick and high-hat)- especially after a few minutes. Latency? Is this due to the external device only getting a clock at the start and then not getting (or taking) updates over time? (I seem to remember this being an issue on Electribe drum machines for example)

Would a timing adjustment parameter be useful?

catfact commented 6 years ago

i don't know but that is definitely a separate issue. as you say, drift between devices is possible. IIRC the TR-8 drifts badly in "song" mode but syncs more often in "pattern" mode. dunno about tr-9.

anyways this issue is about midi device management, mostly the C side of it. i know nothing about the BeatClock abstraction.

justmat commented 5 years ago

Hello everyone. I will be looking into the foulplay issues today, but anything lower level than that is going to be over my head. My plan is to follow fairplay and switch over to BeatClock. My apologies if foulplay problems have muddied the midi discussion.

Dewb commented 5 years ago

@okyeron when playfair was laggy behind your TR-9, was the TR-9 the clock master, or playfair?

okyeron commented 5 years ago

Playfair was master.

I tried but was unsuccessful getting clock-in working with the BeatClock abstraction.

Dewb commented 5 years ago

Hmm, interesting. I ran sync in both directions for a long time with an elektron box, I can't think of any reason Roland devices would behave differently. I have a TB-3 I can try. I haven't tested since the update, though; was your test pre- or post-180707?

edit: dug the TB-3 out of the closet. By ear, syncing with playfair in either direction seems pretty tight, even after a few minutes. This is with the 180707 update. I'm using USB midi directly from norns to the TB-3 with a USB A-B cable, how are you connecting the TR-09? (And wow, the combo of playfair and the TB-3 is pretty fun, thanks for the nudge.)

Dewb commented 5 years ago

(your ears may be better than mine, or the TB-3 and TR-09 might not have much in common after all.)

Dewb commented 5 years ago

The troubleshooting talk is a bit orthogonal to the original issue of midi callback design. Some stream-of-consciousness thoughts on that...

I think the ideal system would have these properties:

user scripts can attach callbacks to different kinds of midi events
user scripts can decide, at script start or device insertion, whether the script wants to attach its callbacks to that device or not, and scripts could choose to attach different callbacks to different devices
user script callbacks can coexist next to library callbacks, system menu callbacks, or even other callbacks in the same script, in the case of someone cut-and-pasting multiple musical concepts together from separate scripts
the system should clean up callbacks when a script is killed or re-executed, or a device is removed, so script authors don't have to remember to do this & implement it correctly. (Looks like this was just added in #470, awesome.)

If, instead of assigning device.event = cb, scripts instead had to call something like device.subscribe(cb) which added the callback to a collection, and calls to .event triggered every callback in the collection, that gets you most of the way there. I think this is compatible with the approaches in #444, as long as you can stack several callbacks on the same device/event. And making them consistent across midi/hid/etc is great.

A second optional argument to .subscribe that lists which types of events (cc, note, clock, sysex, etc.) the script wants to subscribe to (or even separate .subscribe_cc, .subscribe_note) might be helpful in saving script authors from writing boilerplate event switch statements in their callbacks, but I'm not sure the tradeoff of API complexity for script complexity is worth it.

tehn commented 5 years ago

@dewb i like this idea a lot. subscription would be a versatile approach.

i'll have a go at implementing this for midi.

tehn commented 5 years ago

sorry for the delay. design proposal:

-- callback for all devices
function in_from_all_devices(data)
  tab.print(data)
end

-- set up new midi object
all_devices = midi.subscribe(in_from_all_devices)

-- send to all devices
all_devices:send(data)

----

-- callback for named device
function in_from_one_device(data)
  tab.print(data)
end

-- set up new midi object
one_device = midi.subscribe(in_from_one_device, "Roland 1x1")

-- send to one device
one_device:send(data)

the idea is that both of these device objects could co-exist.

thoughts?

catfact commented 5 years ago

subscription / responder model seems good. but also complicated and maybe error prone to implement in a useful way.

by "useful" i mean with a mechanism to filter against things like channel number and to have a hierarchy of callbacks. since norns scripts are monolithic, it's a little hard for me to think of situations where it would be really necessary to dynamically define multiple responders for midi.

if we do want that though, i'd consider looking at the supercollider MIDIResponder API for inspiration: http://doc.sccode.org/Classes/MIDIResponder.html

pq commented 5 years ago

a few quickies:

where/when is registration expected to happen? in init() ?
on first read i imagined that in_from_all_devices was a global callback that you were overriding but i think its actually just a "local" function. if so, maybe clarify by adding the local keyword?
is midi still a global?
thinking ahead could grid registrations look the same?
and arc too?

antonhornquist commented 5 years ago

I’m late for this discussion (sorry) but just wanted to chime in that I like subscriber/responder approaches. I’m biased towards SC and am very fond of its modern MIDIdef/MIDIFunc responders which are akin to hooking up callback funcd in the subscribe approach described above but with additional args for filtering on low granularity, like ”i want this callback to handle note ons 60-71 on channel 10 when velocity is over 50)”. I made equivalent responders (GridKeydef/GridKeyFunc/EndDeltadef/EncDeltaFunc/et cetera) in my SerialOSCClient library.

One gripe I have in the example above is that sending data requires attaching a callback. SC MIDI responders (modern referred above as well as old MIDIResponders) have nothing to do with MIDI output, and I believe thats a good thing, output is better handled separately.

tehn commented 5 years ago

@antonhornquist you can still use midi.send and midi.send_all of course, and throw away the return. i simply thought it would be convenient that the user wouldn't need to track an id/name of a midi device multiple times?

i like the idea of granularity in the midi lib, ie, filter for cc's as @Dewb suggested. i'm not going to implement this myself presently, but we should design for its potential inclusion later.

also FYI all of the previously existing midi functionality is there. this PR breaks nothing. but overriding midi.add and then keeping track of device pointers does not feel like a friendly approach to scripting... most users will use the blanket "all devices" method, and sorting by name seems a reasonable approach if one wants specificity. ie, i'd like to be able to specify in-menu which midi device does parameter-cc-mapping, so another device could be freed for script-only assignment, etc.

a few more issues:

i'm not sure linux reports duplicate device names with a appended number... i don't have multiple of the same midi device. ie, op-1 op-1 2 etc. so referencing devices my name has this drawback.
there's really no specific use for unsubscribe beyond realtime moving around of callbacks, which will be incredibly rare (but should exist). we'll just use clear in system-cleanup

okyeron commented 5 years ago

For reference - plugging in 2 teensy's setup as midi devices:

we@norns:~/norns $ lsusb
Bus 001 Device 010: ID 16c0:0485 Van Ooijen Technische Informatica Teensyduino MIDI
Bus 001 Device 009: ID 16c0:0485 Van Ooijen Technische Informatica Teensyduino MIDI

we@norns:~/norns $ amidi -l
Dir Device    Name
IO  hw:1,0,0  Teensy MIDI MIDI 1
IO  hw:2,0,0  Teensy MIDI MIDI 1

and from maiden (with foulplay running)

foulplay: midi device added 1   Teensy MIDI
foulplay: midi device added 2   Teensy MIDI

EDIT: /dev lists the teensy's as incrementing

ttyACM0
ttyACM1

monome / norns

MIDI management #454