feat: input midi velocity into a separate track#182
feat: input midi velocity into a separate track#182qm210 wants to merge 6 commits intovsariola:masterfrom
Conversation
| trackerUi := gioui.NewTracker(model) | ||
| audioCloser := audioContext.Play(func(buf sointu.AudioBuffer) error { | ||
| player.Process(buf, midiContext, trackerUi) | ||
| player.Process(buf, midiContext) |
There was a problem hiding this comment.
this third argument was a relic from my old solution, which is why you introduced the broker. ergo, I got rid of it
| func (m *Model) CurrentPlayerConstraints() PlayerProcessConstraints { | ||
| d, ok := m.currentDerivedForTrack() | ||
| if !ok { | ||
| return PlayerProcessConstraints{IsConstrained: false} | ||
| } | ||
| return PlayerProcessConstraints{ | ||
| IsConstrained: m.trackMidiIn, | ||
| MaxPolyphony: len(d.tracksWithSameInstrument), | ||
| InstrumentIndex: d.instrumentRange[0], | ||
| } | ||
| } |
There was a problem hiding this comment.
PlayerProcessConstraints are a new concept - depending on what some settings in the Model are, the Player, via the PlayerProcessContext, might need to take care of some constraining conditions
|
|
||
| MenuClickable struct { | ||
| Clickable Clickable | ||
| menu Menu | ||
| Selected tracker.OptionalInt | ||
| TipArea component.TipArea | ||
| Tooltip component.Tooltip | ||
| } |
There was a problem hiding this comment.
the new "when putting MIDI into tracks, you can, optionally, select another track where the velocity byte goes" feature required a new type of button (that opens a popup menu)
| if b.Hidden { | ||
| return layout.Dimensions{} | ||
| } |
There was a problem hiding this comment.
also; I considered it better UX to make the MIDI button hidden if there is no input device available (this is checked once on startup, so if I plug a device in later, I need to restart the tracker - might still be better than checking all devices every single frame)
| if icon == nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Menu Items were forced to have an icon until now
tracker/gioui/menu.go
Outdated
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *widget.Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { | ||
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { |
There was a problem hiding this comment.
this corresponds to the change we introduced with the buttons-that-do-not-react-to-spacebar; we have our own Clickables which were not referenced consistently yet (probably still not everywhere, but progress is done in small steps ;) )
| in := layout.UniformInset(unit.Dp(1)) | ||
| voiceUpDown := func(gtx C) D { | ||
| numStyle := NumericUpDown(t.Theme, te.TrackVoices, "Number of voices for this track") | ||
| return in.Layout(gtx, numStyle.Layout) | ||
| } | ||
| voiceUpDown := NumericUpDownPadded(t.Theme, te.TrackVoices, "Number of voices for this track", 1) |
There was a problem hiding this comment.
ok, this was a change that I turned out not to need later on. improves readability imo
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA) { | ||
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA, ignoreEffect bool) { | ||
| cw := gtx.Constraints.Min.X | ||
| cx := 0 | ||
| if t.Model.Notes().Effect(x) { | ||
| if t.Model.Notes().Effect(x) && !ignoreEffect { |
There was a problem hiding this comment.
because our MIDI input does some coloring of other tracks, it might need to ignore these other tracks "Effect" property (otherwise only one Nibble is colored)
|
Uh oh :D Another one of "23 files changed". While in principle I agree with the goal (getting note velocities), reviewing so many small refactorings on top of adding a new feature is a bit exhausting. I'll do my best, but I've just learned that I will be quite busy in work until January. But I'll try to squeeze in some time to review this. |
|
If there was some clear refactorings that needed to be done to make adding that feature easier, I'd appreciate if you can break these down into a bit more manageable chunks, like "refactor: X" "refactor: Y" and then "feat: Z". This was we can get the easy refactorings merged, while the new feature can take a bit more time and discussion. |
I am aware of the problem, and I know that this takes time to review. And it might very well be that some changes in there are not required for this issue (e.g. were part of some changes that turned out to be not the right path), but I wouldn't know how some of them are possibly separated into smaller requests (e.g. widget.Clickable -> Clickable, or the optional integer stuff). hope we'll manage. I'll go work on something else for now ;) |
|
dammit. didnt want to close. |
yes, I can see working on that |
|
Maybe you enjoy reviewing #183 more, this appears rather straightforward and might reduce the number of files to about a half :D |
…di/improve-midi-input # Conflicts: # tracker/gioui/tracker.go
|
Ah. This is still open. Might prepare these changes rebased on the current, if still relevant... |
|
So, I thought about this, and I think there's a bigger prize hiding in it: ability to "split keyboard", so that e.g. MIDI channel 1 notes 0-63 are mapped to instrument 1 and MIDI channel 1 notes 64-127 are mapped to instrument 2. Both potentially with some transpose. This splitting could go to very fine granularity: for example, I'd imagine one could want to even make "drum kits" where note 64 is mapped to kick instrument, 65 to hihat, 66 to crash etc. etc. This would be important, because one of the glaring limitations of Sointu is that there will always be only 16 instruments max. when working with VST, because of the limitations of MIDI channels. And for reasons, I think it will be a even bigger headache to have two Sointus and then try to merge two songs later during compiling. So, having one Sointu, possible with up any number of instruments, triggered by split keyboard. The current rather artificial limitation of 32 voices in the VM can be increased; I think there's just a few places where we use 32-bit integers to store some bit patterns, where this limitation comes from, but it is not hard to change so that there is no real limitation. And once #199 is implemented, it will become realistic to even render something like 64 instruments at least during authoring. Similar multithreaded strategy could be used during playback, but that'll take a bit more effort. Now, why this is related to the note velocity: I think Song struct should have some kind of member like "MIDI mapping", which should tell a) split points (if any) for each MIDI channel; b) transposes of each part of the keyboard; c) if this part of keyboard also send velocity events. This MIDI mapping is not used for anything during compiling or rendering, it's just there to map the MIDI events from DAW or external MIDI ports to Sointu instruments. Thus, if for example MIDI channel 1 is splitted at points 40 and 60, and the transposes for the three parts are +12, +24, and -12, and whether these three parts send velocity events is set to false, true, false, then MIDI channel 1 maps to four different instruments in Sointu: How would this sound to you? This would of course need some sort of popup GUI, probably from the MIDI menu, to define the MIDI mapping (how each MIDI channel is splitted and whether each part also sends velocity events). |
2 participants
Hi pestis,
this is probably large, but I do not know whether overly-large.
I noticed that my polyphonic midi input from a few weeks back (direct track input via my button, not the recording function) was not actually doing its thing as intended, and I wanted to add a way to also write the velocity of the midi input into another chosen track.
doing so included a lot of thoughts, discussion about structure with @LeStahL , and finally this solution which does its job, but might be a lot to review. anticipating your comments! 😬