Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the use of multiple MIDI devices #9

Closed
xaviergodart opened this issue Nov 6, 2024 · 8 comments
Closed

Allow the use of multiple MIDI devices #9

xaviergodart opened this issue Nov 6, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@xaviergodart
Copy link
Member

Right now, all MIDI message are sent to the configured grid midi device.
We could add a new node parameter that allows to choose the midi device per node, with the grid one being the default.

@xaviergodart xaviergodart added the enhancement New feature or request label Nov 6, 2024
@xaviergodart
Copy link
Member Author

The main problem is: what should happen if a grid is opened in a different midi context, where some midi device are missing?

Should we:

  • display a warning and fallback to an available device?
  • prevent the grid from being opened? (nope)
  • if we fallback to an available device, should we save the grid with the fallback device? Should we handle 2 devices per node? Device + FallbackDevice?

This feature seems to add a lot of complexity.

I'll wait a bit to see if more people are interested.

xaviergodart added a commit that referenced this issue Nov 12, 2024
@xaviergodart
Copy link
Member Author

Most of the work is available in this branch for testing.

@jsmcnair
Copy link

Just had a play with this. I set the device to an external USB device with the channel on a node set to 14 and got a panic.

panic: runtime error: index out of range [13] with length 3                                                                                                                                                       
goroutine 22 [running]:
signls/midi.(*midi).NoteOff(0xc000126840, 0x40b5c0?, 0xd, 0x4c?)
/home/foo/dev/signls/midi/midi.go:122 +0x8a

Here:

signls/midi/midi.go

Lines 116 to 123 in cd9265d

func (m *midi) NoteOn(device int, channel uint8, note uint8, velocity uint8) {
m.outputs[channel] <- gomidi.NoteOn(channel, note, velocity)
}
// NoteOff sends a Note Off midi meessage to the given device.
func (m *midi) NoteOff(device int, channel uint8, note uint8) {
m.outputs[channel] <- gomidi.NoteOff(channel, note)
}

We need to change to:

// NoteOn sends a Note On midi meessage to the given device.
func (m *midi) NoteOn(device int, channel uint8, note uint8, velocity uint8) {
	m.outputs[device] <- gomidi.NoteOn(channel, note, velocity)
}

// NoteOff sends a Note Off midi meessage to the given device.
func (m *midi) NoteOff(device int, channel uint8, note uint8) {
	m.outputs[device] <- gomidi.NoteOff(channel, note)
}

No more panic and the node sends the message on correct device and channel

@jsmcnair
Copy link

Also when cutting and pasting a node:

panic: runtime error: invalid memory address or nil pointer dereference                                                                                                                                   
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x52a7ce]
goroutine 168 [running]:
signls/filesystem.NewNote({{0x63bd08, 0xc00008c880}, 0xc0000a2b10, 0xc0000a2a20, 0x0, 0xc00028c078, 0xc00028c090, 0xc00028c0a8, 0x64, 0x0, ...})
/home/foo/dev/signls/filesystem/bank.go:93 +0x6e

The note copy function does not have the device included:

func (n Note) Copy() *Note {
newKey := *n.Key
newChannel := *n.Channel
newVelocity := *n.Velocity
newLength := *n.Length
source := rand.NewSource(time.Now().UnixNano())
return &Note{
midi: n.midi,
rand: rand.New(source),
Key: &newKey,
Channel: &newChannel,
Velocity: &newVelocity,
Length: &newLength,
Probability: n.Probability,
}
}

func (n Note) Copy() *Note {
	newKey := *n.Key
	newDevice := *n.Device
	newChannel := *n.Channel
	newVelocity := *n.Velocity
	newLength := *n.Length
	source := rand.NewSource(time.Now().UnixNano())
	return &Note{
		midi:        n.midi,
		rand:        rand.New(source),
		Key:         &newKey,
		Device:      &newDevice,
		Channel:     &newChannel,
		Velocity:    &newVelocity,
		Length:      &newLength,
		Probability: n.Probability,
	}
}

@xaviergodart
Copy link
Member Author

Thanks a lot for testing! I just fix theses in the branch.

Do you have an opinion on how you would imagine this to behave when you open a grid in a context where all needed midi devices aren't connected?

Should we fallback on the default device? Should we display an error message?

@jsmcnair
Copy link

My understanding is that currently it's not possible to know whether all MIDI devices are connected as the device is serialised as an index. For example if I unplug a device and plug another in that new device might have the same index as the unplugged device.

If the device were serialised as the string representation (gomidi Port.String()) then you have a solid basis for making decisions about what to do.

When you deserialise and can't map a device name to an existing device then you could select a default. Also there could be a conversion sub(?) command outside of the UI that allows converting devices matching one MIDI device to another.

@xaviergodart
Copy link
Member Author

My understanding is that currently it's not possible to know whether all MIDI devices are connected as the device is serialised as an index. For example if I unplug a device and plug another in that new device might have the same index as the unplugged device.

If the device were serialised as the string representation (gomidi Port.String()) then you have a solid basis for making decisions about what to do.

You're right. Using the string representation is the only way to reliably reference a device.

When you deserialise and can't map a device name to an existing device then you could select a default.

Indeed. I guess the default device value for a new node could be default, which reference to the grid device configuration. But the parameter would allow to select a specific device.
On grid load, if the specified node device is not found, we fallback to the default one, but we keep the reference to previously selected device for future use (displaying it in a disabled state to signify its unavailability).

I think I like that approach. It's simple enough, and the default way is still using only one device (which is my way of using Signls, I map channels to different devices in Carla).

Also there could be a conversion sub(?) command outside of the UI that allows converting devices matching one MIDI device to another.

Not entirely sure what you mean here.

@xaviergodart
Copy link
Member Author

Support for multiple MIDI devices has been added in v0.7.0.
Feel free to reopen or create a new issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants