fix #148311 by adding dummy note events to end of loop during playback #5719
Conversation
krkartikay
force-pushed
the
148311-loop-rests-last-measure
branch
from
5979ac3 to
17a7582
Compare
mscore/seq.cpp
Outdated
| // this is to let the playback reach the end completely before starting again | ||
| if (!events.count(cs->loopOutTick().ticks())) { | ||
| NPlayEvent ev; | ||
| ev.setVelo(0); |
There was a problem hiding this comment.
I would do ev.setValue(META_EOT), since that is what this event is (end of track). 0 is ME_INVALID or META_SEQUENCE_NUMBER, neither of which this event is, really.
There was a problem hiding this comment.
Well, it's not really an end-of-track event either, because the user can select a portion in the middle of the score to be looped. I did a bit of research about MIDI events, but I'm still not sure about which event exactly should be used here. I thought that using an invalid event is the best option because this isn't really a MIDI event, it's just a dummy object, and is never written to a MIDI file. Maybe being more explicit about it could be better?
| ev.setVelo(0); | |
| ev.setValue(ME_INVALID); |
There was a problem hiding this comment.
Oh, right... I was under the impression that this happened when the loop was right at the end of the score, no? If not, then you probably don't need to change anything.
There was a problem hiding this comment.
No, actually this happens whenever there is a gap between the last event and loop endpoint. Now, because users normally select loop endpoints which happen to align with metronome ticks, so this issue was only noticed at the last measure where there was no metronome tick at the end. However this fix is more general and adds an event at any loop endpoint.
The issue is fixed by adding dummy note events to end of loop before playback, because the sequencer only looks for the last event in the events map before looping back.
krkartikay
force-pushed
the
148311-loop-rests-last-measure
branch
from
17a7582 to
ca83e84
Compare
|
But this would put an invalid MIDI event into the render result, wouldn't it? |
Yeah, it does add an invalid midi event into the event list, but that seems to get ignored by the sequencer. Should I do something else in its place? |
|
I don't know what the MIDI spec says nor do I have any insight into how different implementations might react. But I'd say, if it works for our own internal playback in both Fluid and Zerberus, I won't worry about if there is some case where MIDI output or JACK or whatever doesn't work right when loop is enabled. |
Resolves: https://musescore.org/en/node/148311
The playback in MuseScore relies on midi events to know when to stop playing / loop back to start. By default, metronome ticks are added to the events map at every beat, but if there is a note after the last tick, playback will loop back simply after playing the last note and will not reach the end of the measure.
One possible solution was to add an end-of-track (or similar) midi event to the last measure, however, that won't fix the issue because the user may try to loop a portion of the score, which might not end on a measure boundary. In this commit, I have added a dummy NPlayEvent at the end whenever the user selects a loop. This works even for loops within measures. As I have not modified libmscore/rendermidi.cpp, the exported MIDI files are not affected.