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

fix #25010: added midi generation for guitar bends #25080

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

alexpavlov96
Copy link
Contributor

@alexpavlov96 alexpavlov96 commented Oct 8, 2024

Resolves: #25010

changes that affect other things in midi export than only GuitarBend:

  1. Duration of note with slide after grace note:
    el->push_back(NoteEvent(0, onTime, slideOn - onTime, velocity, !note->tieBack()));
    example:
    grace+slide.gp.zip

  2. Resetting pitchwheel to 0 as close to next note as possible.

if (tick - specs.mStep > pwEvent->first) {
    tickForPwReset = tick - specs.mStep;
} else {
    tickForPwReset = (tick + pwEvent->first) / 2;
}

With new implementation of bends we have 2 notes (instead of 1), so we skip sound ON of second note (adding more pitchbends to keep sound on the needed pitch), so if pitchbend should be dropped as close as possible to next note - not to have sound artifacts

Screenshot 2024-10-08 at 15 05 49

example:
bend+note.mscz.zip

@@ -932,7 +945,7 @@ void CompatMidiRender::createSlideOutNotePlayEvents(Note* note, NoteEventList* e
int slideOn = NoteEvent::NOTE_LENGTH - totalSlideDuration;
double velocity = !note->ghost() ? NoteEvent::DEFAULT_VELOCITY_MULTIPLIER : NoteEvent::GHOST_VELOCITY_MULTIPLIER;
if (!hasTremolo) {
el->push_back(NoteEvent(0, onTime, slideOn, velocity, !note->tieBack()));
el->push_back(NoteEvent(0, onTime, slideOn - onTime, velocity, !note->tieBack()));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure, but it seems NoteEvent m_len will always be 0 (slideOn-onTime). So this event makes no sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

slideOn - time when slide should start, it's calculated from the end of the duration. See the file in description, there was bug, maybe I've also missed smth

Copy link

@abariska abariska left a comment

Choose a reason for hiding this comment

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

  1. Grace-bend should take its duration from previous not (not from the target one).
    In this case it should be part of A note. In properties of the grace-bend diagram should reproduce actual previous not (A), which allows to make the grace-bend note as long as we want.
    abariska 2024-10-11 at 17 08 22
    gracebend+bend+release.mscz.zip

  2. Bend after prebend ends on one point earlier that it should.
    Last pitch change should be at the begin of the beat.
    abariska 2024-10-10 at 17 53 08
    prebend+bend.mscz.zip

  3. Pitch of bends changes to less of pitch points (e.g. 8-9 for full bend instead of 10)

abariska.2024-10-10.at.18.08.21.mp4
  1. 3 pitch points change instead 2 to be used for slight-bend.

} else {
GuitarBend* bendFor = note->bendFor();
if (bendFor) {
collectGuitarBend(note, noteChannel, tick1, noteParams.graceOffsetOn, noteParams.previousChordTicks, pitchWheelRenderer,
Copy link
Contributor

Choose a reason for hiding this comment

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

How exactly will this work if the bend starts from a tied note? Right now bends don't work at all during normal playback if they start from tied notes (#21345). So I'm wondering if we have the same problem with MIDI export...

Copy link
Contributor Author

@alexpavlov96 alexpavlov96 Oct 25, 2024

Choose a reason for hiding this comment

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

any consequent combination of ties/bends is treated in 1 pass, and ignored if called again from another note of combination.
Rechecked, this combination also works. @abariska FYI

@abariska abariska self-requested a review October 29, 2024 19:21
Copy link

@abariska abariska left a comment

Choose a reason for hiding this comment

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

There're some minor issues but they will be reported separately

@alexpavlov96 alexpavlov96 merged commit 8eb42bd into musescore:master Oct 29, 2024
11 checks passed
@alexpavlov96 alexpavlov96 deleted the guitarbend_midi branch October 29, 2024 19:22
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.

Bends are not exported in MIDI
4 participants