-
-
Notifications
You must be signed in to change notification settings - Fork 291
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
Add check for extra rest duration #462
base: master
Are you sure you want to change the base?
Add check for extra rest duration #462
Conversation
client/transmitter/osc.go
Outdated
lastEvent := event | ||
// Last event offset + duration must equal score.duration | ||
currNoteEnd := int32(math.Round(lastEvent.Duration)) + int32(lastEvent.EventOffset()) | ||
partOffset := int32(score.CurrentParts[len(score.CurrentParts)-1].CurrentOffset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think selecting the last Part in the score is potentially error-prone, as it might not be the Part that the note belongs to. I think lastEvent.Part
is probably what we want here.
Thanks for the PR, Kyle! It looks good overall, apart from the thing I mentioned above. I'll do some manual testing soon when I have time, and merge it if everything looks and sounds good. |
This doesn't appear to fix the issue in #421, unfortunately :( Here's what I did to test:
The "hiccup" is still there where the final rest is skipped. |
@kylewilk567 Sorry for the late reply - I've been on vacation the last couple weeks. This is interesting! I tried it the way you described, and I also heard the last rest being handled correctly. But I tried it again the way I described (running a REPL server in another terminal and using the CLI to send it messages), and the rest is still getting left out when I do it that way. I wonder if there is some important difference in the way the messages are being sent to the REPL server when you do it via the CLI vs. doing it interactively in a REPL client session? |
Patch to fix issue #421 . Checks the ending duration of what is sent to the client matches the PartOffset in the Score object. If there is a mismatch (as in the case with a trailing rest), extra duration is appended to the last note.