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.

Sign up for GitHub

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

Jump to bottom

Don't merge ties and rests due to conflicts with quantization on past Addmusics #350

Open
KungFuFurby opened this issue Jan 25, 2023 · 0 comments · May be fixed by #351
Open

Don't merge ties and rests due to conflicts with quantization on past Addmusics #350

KungFuFurby opened this issue Jan 25, 2023 · 0 comments · May be fixed by #351
Labels
addmusic405 Involves an adaptation that originates from Addmusic405. addmusicm Involves an adaptation that originates from AddMusicM. bug Something isn't working c++-side Involves the AddMusicK program itself in some way.

Comments

@KungFuFurby
Copy link
Owner

Spun off from #349. Turns out that the test case I used was actually failing for a different reason (though it would still save memory): namely, it is caused by a bad optimization by AddmusicK. Namely, it merges ties and rests together. The problem occurs when merging ties attached to a note (not a rest) due to a conflict with quantization.

Specifically, quantization resets the duration prior to keying off a note when processing a new VCMD regardless of whether it is a tie or a rest. This special case that I found relies on the tie VCMD causing the key off to occur later than what AddmusicK ends up doing by mistake since said duration gets recalculated.

In order to eliminate this bug, I will have to bypass this optimization.
@KungFuFurby KungFuFurby added bug Something isn't working c++-side Involves the AddMusicK program itself in some way. addmusicm Involves an adaptation that originates from AddMusicM. addmusic405 Involves an adaptation that originates from Addmusic405. labels Jan 25, 2023
@KungFuFurby KungFuFurby changed the title Don't merge ties and rests due to conflicts with quantization Don't merge ties and rests due to conflicts with quantization on past Addmusics Jan 25, 2023
KungFuFurby added a commit that referenced this issue Jan 26, 2023
@KungFuFurby
Skip merging notes and ties on past Addmusics
2352bfe 
Rests are allowed to be merged together, but notes and ties are not. This is
because of a conflict with quantization: quantization acts relative to the
length of the last VCMD executed, and ties are separated from normal notes in
this specific situation. Without doing this, notes may key off sooner than
expected compared to the way they were originally created. Rests can still be
merged together because it is already keyed off to begin with.

This commit closes #350.
@KungFuFurby KungFuFurby linked a pull request Jan 26, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
addmusic405 Involves an adaptation that originates from Addmusic405. addmusicm Involves an adaptation that originates from AddMusicM. bug Something isn't working c++-side Involves the AddMusicK program itself in some way.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Skip merging notes and ties on past Addmusics KungFuFurby/AddMusicKFF
1 participant
@KungFuFurby