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

Update midi buffer #18

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

jorshi
Copy link
Collaborator

@jorshi jorshi commented Apr 1, 2020

Thanks so much for your work on this project. I'm working on a master's degree right now and focusing on automatic synthesizer programming - so your work on this and the other work you have published on deep learning for synthesizer programming has been extremely helpful!

I tracked down a bit of an issue with the way that midi notes are being added to the buffer. Timestamps in MidiMessages are ignored in the MidiBuffer.addEvent method (this is true for v5.2.0 as well). This is causing midi notes to be triggered on every buffer iteration. This is also occurring for the note off MidiMessages once they begin to be triggered (less of an issue, but since the timestamp is being used, the message isn't being triggered on the correct sample).

This PR fixes that by ensuring that only one note on MidiMessage is sent during the first buffer, and then the note off MidiMessage is being set to the correct sample in the buffer.

Here is a waveform test rendering of a sine wave with no amplitude envelope:

before:
output_4_1

after:
download

@fedden
Copy link
Owner

fedden commented Jun 5, 2020

Hey @jorshi thanks for the PR. I'm unable to support this project in a professional capacity, so I am wondering if you would like to become a collaborator to get this project in a state that is hopefully usable for yourself and others?

@jorshi
Copy link
Collaborator Author

jorshi commented Jun 8, 2020

Hey @fedden thanks for the response - I would be pleased to become a collaborator on the project. Thank you for offering.

@turian
Copy link

turian commented Dec 21, 2020

@fedden take his offer! Allow him to be a collaborator so he can merge it :)

@fedden
Copy link
Owner

fedden commented Dec 21, 2020

Please excuse my inactivity on this - it's been a weird year - @jorshi you should have an invite. Thank you for your contributions

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.

None yet

3 participants