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

Midi/track input #170

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Midi/track input #170

merged 1 commit into from
Oct 22, 2024

Conversation

qm210
Copy link
Contributor

@qm210 qm210 commented Oct 18, 2024

I just rebased your current upstream master and I think I made it work again, but...

Expecting your feedback ;)

Copy link
Owner

@vsariola vsariola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool stuff, accepting notes through the MIDI while jamming seems to be the point of this? There was so much changes to many different files, so it's a bit difficult to review what it actually does.

  1. Please properly rebase to the master. You have essentially just reverted all the changes there :) In particular, the refactorings e.g. MIDIContexter -> MIDIContext, m.d.Loop -> m.Loop and you removed again the fixUnitParams...
  2. There's a ton of minor style changes (e.g. renaming method receivers), documentation (comments), and changing .gitignore. These probably should go to separate commits, with style:, doc: and chore: type indicators.
  3. I'd appreciate if you can already squash the commits, because it seems that there's more different types of changes than one feat: commit here. I recommend you making a backup branch of your current commit history, if you start removing some of those unrelated style/doc/chore changes from the PR.
  4. Please add a message in the CHANGELOG.md (on the first line after "Added") to describe what feature you actually added.

.gitignore Show resolved Hide resolved
tracker/action.go Show resolved Hide resolved
tracker/gomidi/midi.go Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
cmd/sointu-track/main.go Outdated Show resolved Hide resolved
tracker/model.go Outdated Show resolved Hide resolved
tracker/processor.go Outdated Show resolved Hide resolved
tracker/table.go Outdated Show resolved Hide resolved
tracker/table.go Outdated Show resolved Hide resolved
tracker/table.go Outdated Show resolved Hide resolved
@vsariola
Copy link
Owner

Oh, and all the VST binaries don't seem to build, that would have to be sorted out also.

@vsariola
Copy link
Owner

vsariola commented Oct 19, 2024

So, I got to experiment with this properly. I have a question: normally, when recording and jamming, we interpret the midi channel to correspond to the instrument that is being played. But now you input whatever MIDI notes have arrived at the current cursor position. Little bit confusingly, the player still respects the channel instrument as the instrument being played, so even if you input a note on track corresponding to instrument Foo, but the channel of the MIDI message corresponds to instrument Bar, the player will play instrument Bar. I guess at least we should at least somehow trigger the right instrument to not confuse the user?

In your use case, I think you intentionally do not want to respect the channel information of the MIDI message, right? So you want to have a MIDI keyboard near by, and place the cursor where-ever, and input a note at that position, so it's almost similar as pressing a note on keyboard?

@qm210 qm210 force-pushed the midi/track-input branch 2 times, most recently from b920c95 to 37dab09 Compare October 21, 2024 19:58
@vsariola vsariola merged commit 8dfadac into vsariola:master Oct 22, 2024
18 checks passed
@vsariola
Copy link
Owner

Merged, thank you for your contribution!

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

Successfully merging this pull request may close these issues.

2 participants