Section break causes pause before repeats and jumps during playback
Priority
P1 - High
Type
Functional
Frequency
Many
Severity
S3 - Major
Reproducibility
Always
Status
GitHub issue
Regression
No
Workaround
Yes
Project
- Create a score with a repeat barline at the end
- Add a section break and another section.
- Add some notes
- Save
- Reopen
- Play
Expected result: the repeat is played, a pause is marked and the second section is played
Actual result: the first time is played, a pause is marked, the second time is played, a pause again, and the second section is played.
Example score : http://musescore.org/sites/musescore.org/files/Musiques%20du%20Dauphin%…
Comments
I've noticed this issue as well. It happens not just on repeat signs but jumps as well (e.g. attached test with D.C. and D.S.). I've looked through the code, and discovered that it occurs because the section break pauses are implemented as setPause on the final tick of the measure ending with the section break. Therefore any time that measure is encountered, the pause is incurred.
I don't think that final measure is the entity that needs to be paused. Rather the pause should logically occur after that measure (but before the first measure of the next section).
So my proposed solution is to insert an additional tick in the tempomap inbetween the final tick of the current section and the initial tick of the next section. (I would not be increasing the measure's internal length or otherwise interfere with the measure itself). This extra tick would the one that gets applied with setPause.
I will take care to remove this extra tick whenever the section break is removed (which also occurs when that measure is delted) as well.
Let me know if there is something wrong with this approach.
Sounds good, you can try it and write tests for the different cases.
I don't understand what you mean "insert an extra tick". The final tick of the last measure *is* the first tick of the next. Eg, in 4/4 time, with 480 ticks per quarter note, the last segment of the first measure is at tick 1960, the next measure is *also* at tick 1960. Are you proposing shifting everything so that the next measure is actually at 1961? Actually changing the tick of that and every subsequent measure? I kind of doubt that would work well; there are probably any number of places in the code where it is assumed these values would be equal. Or do you mean something else?
Actually the real problem here is that the tempomap is indexed in ticks and not in uticks currently. If you don't know yet, tick are used for the graphical score, while utick are used for playback when the score is "unrolled". So at one give tick we currently have a pause in the tempo map. There is no way to know if we are going at utick 1920 or 3840 if these two uticks are at the same measure at tick 1920 for exemple...
If tempomap was using utick we could implement "slower the second time" and so on... but the code would be a lot messier...
>> "Are you proposing shifting everything so that the next measure is actually at 1961? Actually changing the tick of that and every subsequent measure?"
Yes, that is what I was saying.
>> "If tempomap was using utick we could implement "slower the second time" and so on... but the code would be a lot messier..."
Ok. Is this something that is on the roadmap that I can try making changes to implement? Or is this something that is wished for but will never be implemented for fear of breaking things? Just asking so I know best way to proceed. It indeed would be nice to have things like tempo changes be on a per repeat basis. (Would indexing by utick's also enable handling dynamic differences be triggered on per repeat basis, e.g. "f-p" for "play forte first time, piano second time?)
Since I don't want to interfere with any hidden assumptions marc brought up about "ticks", I'm going to try modifying the code just so the final "utick" is the entity that gets paused...
I can't speak for roadmap. It's not something I kno w of specific plans for, but the idea of doing this comes up from time to time, and maybe this would be an excuse to provide the framework. Now is as good a timre as any to give it a shot.
As for the tick shift plan, I don't *know* that it won't work, I was just expressing some doubts. The other tricky aspect about it is making sure to update the tick info for all spanners and all the other maps.
I have added a unrolledTempoMap to the root score, which maps unrolled TempoEvents indexed by uticks. Unrolled playback is working as expected, both with the two scores posted here as well as some faily complex tests I've made.
https://github.com/ericfont/MuseScore/tree/32696-unroll-TempoMap-after-…
An overview of changes:
Copying the rest of the tempo events that occur within each RepeatSection is straightforward. Note: if want to trigger tempo events on specific repeats (e.g. "slower the second time"), then that can be handled here by selectively copyig the tempo events that are indicated to trigger on specific repeats. Currently my unrolling will observe tempo changes in order of the time that playback encounter their tempo events, so my code plays a repeat "slower the second time" if playback enounters a slower tempo marking and doesn't enounter another explicit tempo event for the original tempo.
I'm not to worry about performance loss due to regenerating the unrolled tempo map, as it only gets regenerated after the user both makes changes to tempo/form and presses play (or does something else that calls utick2utime or utime2utick). I also don't image the unrolled tempomap to be larger than several hundred events, even on a large score.
I can provide more details if you wan't and can't figure out from reading my code. Note that fermatas will fit nicely in my framework as a "PAUSE_AFTER_TICK", since they would add a delay immediately after the tick of a Chord-Rest containing a fermata.
Notes: I'm not ready for a pull request yet, as I still have some few bugs/crashs, I think related to when I need to invalidate the unrolledTempomap and when I can regenerate it. mp3 export seems to work correclty. But midi file export doesn't hande the breaks, so I need to look into that (my suspicion is that midi export is using a different tempomap class in /miditools/tempomap.cpp, so I might look into merging the two together for consistency). Please take a look and give me any feedback. I'm tyring to follow the code formatting guide, and to write in a similar style as the other code I see, but please let me know if I'm doing any programming styles that are frowned upon and I can fix them.
@ericfonainejazz: thanks for spending time on this! It is appreciated.
For what is worth, from a quick glance at your code, no bell ringed, so please go on!
Existing code base tends to be rather terse in commenting and this makes some spots hard to understand; so, if anything, with respect to existing code, feel free to exceed with comments in your code!
To have a clearer global picture of the changes involved, it could be useful to squash the multiple commits into a single one ( http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.h… ); maybe not right now, but as soon as you will feel confident that the individual steps go in the right direction (the practice is to ask for squashed commits for the PR anyway).
Sorry I'm new to git. I'm using Eclipse egit plugin. I have just tried merging my commits into one using these directions I found onllilne:
"In History tab select all commits you want to squash. Then right-click the selected commits and from the context menu select Modify --> Squash."
So now my history view in git shows them all as one commit. I then pushed that commit to my github. But I don't know if that merged commit is succesfully merged as desired on github...could you check if that latest commit is correctly "merged'?
I've fixed the bugs, I believe. I've sqashed the commits into a new clean branch named "32696-unrolledTempomap" and issued a pull request https://github.com/musescore/MuseScore/pull/2133
Note: There are plently of qDebug messages I've left in my code...those can all be removed, but I'm leaving them in incase any bugs pop up. I think I'm invalidating the unrolledTempomap everytime a new tempoevent is inserted, and only regenerating it when needed (i.e. the play button gets pressed), but I haven't studied the rest of the codebase, so I may be ignorant of something important.
I didn't include any test files. I'm not sure how to create test files in the context of this issue for the auto-tester. Would I check the ending unrolled time of the final tick and compare that to an expected value? Should I make a bunch of smaller tests that only deal with each specific case seperately, or should I try to combine a bunch together into one file?
Note: I do notice some delay almost a second in a huge scores like opengoldberg after adding new tempo events and pressing play for the first time, but I actually think most of that delay is not due my code (I might try running a profiler and see if the delay in my function is significant...like >100ms or so).
(regarding midi file output, I notice that the 2.0.1 release doesn't include any section break delays in the midi file output anyway, so that issue specifically is not a problem with my code. I believe the difference with midi file output occurs because there are two tempomap classes. I might want to file another bug report that says midi file output should have identical pauses as in desktop playback, and fix it by merging the two tempomap classes.)
In light of the revelation that section breaks can occur on non-measure frames, I'm canceling my earlier pull request (https://github.com/musescore/MuseScore/pull/2133). Also another thing I now need to be aware of is: if a section ends on a fine, then it doesn't have a section break immediately after it, and hence would incorrectly not incur a pause when should actually have one. I will open up a new PR, and will probably implement my initially suggested "tick shift" method. Once my fix for #65161: Da Capo/Da Segno al Fine ignore next section on playback (https://github.com/musescore/MuseScore/pull/2172) gets merged, the modified unwind() will only produce RepeatSegments that never cross SECTION_BREAK boundaries. Therefore, I think I just would need to make a note during unwinding about which RepeatSegments start on a section break (could be a boolean variable in RepeatSegment, such as "isAfterSectionBreak"). Those RepeatSegments and their measures will have their initial tick value be increased by one, which could probably be handled when RepeatList::update() is called. This new fix will also fix #73806: SECTION_BREAK on non-measure MEASURE_BASE does not incur pause before starting next section
I will not try to generate an unrolled tempomap for my next PR...I'll leave that for another feature request.
ok, now that my fix for #65161: Da Capo/Da Segno al Fine ignore next section on playback is merged, I'm going to implement what I describe above in #12. I don't see any new comments, so I'm going ahead with adding a boolean to RepeatSegment called "isAfterSectionBreak", and will increase the (graphical) tick by one for those RepeatSegments, and add a pause on that extra (graphical) tick.
I'm following the isAfterSectionBreak but the addition of a graphical tick, I don't get it. If it means that in the scheme measure(4/4), section break, measure, the second measure will have a tick of 1921, I don't think it's a good solution...
OK, then forget about that tick shift idea.
I am now considering a simpler solution. I will add isAfterSectionBreak boolean to RepeatSection. And then in RepeatList::update() if isAfterSectionBreak, then add delay to s->utime. There maybe something else I need to take care of as well, but how does that sound to you?
It sounds better.
I'm marking as "patch code needs review" because I am almost certain that whatever transient error that happened on my first commit to my latest PR (https://github.com/musescore/MuseScore/pull/2188) was not in a result of my code in the PR and is an entirely separate issue.
Regarding my comment in #15, my PR is slightly different since I forwent my suggestion of a boolean "isAfterSectionBreak" in RepeatSegment, and instead only have a qreal "pauseBefore" which equals 0 if there is no pause before the section.
came up again in #97611: Section break interrupts playback in repeats
I should probably reset this status to "active" and unassign myself since my previous attempts to fix this weren't accepted. I'd like to attempt again with a different strategy if no one else takes it.
Reported again: #121566: Repeat sign not working
Came up again in https://musescore.org/en/node/137516
This is still an issue in 2.1
I would recommend to not bother to fix for 2.1, and instead save for 3.0, because I discovered that implementing a proper fix for this would require too many changes to underlying structures.
Came up again in #195976: Playback Repeat at secion end waits as if going to next section
In reply to #25 by Jojo-Schmitz
I think section breaks should be only a sort of "marker", telling simply MS to reset all parameters as if the next bar was the beginning of a score.
So during the playback, the logic would be :
- I'm playing...
- Woaw ! A section break ! Let's have a flag on!
- I'm still playing exactly as if nothing happend...
- Ok, I'm at the end of the playback. Do I have a SB flag on ? Yes : let's go to the 1st bar after the sction break marker.
- Let's reset all parameters
- Let's begin playing with brand new parameters.
If I'm not wrong, this should correct the behaviour, no?
came up again in https://musescore.org/en/node/246551#comment-789246
After having spent some time with the unwind logic, I think the best tracking of when to honor the pause would indeed be on the RepeatSegment.
RepeatList::unwind() already loops over the score on a per-section base, so it is aware when we are moving from one section to the next.
Same pause happens on jumps, like D.C., D.S. etc.
Came up again in https://musescore.org/de/node/273264, here with a jump
Maybe not exactly the right topic here, but wouldn't it be an improvement resp. wouldn't it be more obviously for the user to see/adjust the section breaks properties inside the inspector (especially the settings for "pause")?
In reply to Maybe not exactly the right… by kuwitt
Oh, you mean like on breaths and pauses?
yes, it would.
In reply to Oh, you mean like on breaths… by mike320
@mike320: I mean the settings of section breaks, which are adjustable via right click should also be available inside the inspector, so that's easier to recognize for the user what causes the pause.
Just a thought from user's point of view (aware, that's maybe stupid):
Without being familiar with ticks (only with my personal "Ticks" (https://dict.leo.org/englisch-deutsch/Tick?side=right) ;-), an maybe it's already in a similar way discussed here, but why couldn't it be handled (and implemented in the code) with the same rules like a repeat sign and a jump in the same measure (for example a "repeat sign" and a "d.s.")?:
That means: first play the repeats then jump to segno (anf for the section break: first play the repeats then pause)
In reply to Just a thought from user's… by kuwitt
kuwitt: your idea is not wrong. To me it is quite clear what needs to happen in code, I just need to get around doing it.
Implementation details on (3) are still a bit unresearched on my end:
* does a break influence uticks/tempomap? If not, we should be done fairly straight forward. If it does, we "just" need more time :-p
Came up again in #274313: Section Breaks and "D.C. al Fine" / "Fine" don't work together during playback / audio export
Came up again in https://musescore.org/en/node/280943
Came up again in https://musescore.org/en/node/285306
And again in https://musescore.org/de/node/286257
Came up again in https://musescore.org/en/node/292568
My feeling is that the sensible way to solve this is to separate the section break:
a) either from the measure, as a separate kind of frame which takes time rather than space
b) or from the measure properties into a segment of its own and coming after everything else, so all repeats, da capo etc are executed before the section break is reached (and "played") and execution would resume at it even after an "al Fine".
Relates to #299323: [EPIC] Issues with repeats and jumps
Just adding my vote to getting this fixed, it's probably my main irritation with Musescore and I run into it a lot, with no clear workaround.
https://github.com/musescore/MuseScore/pull/6248 contains some very minor groundwork for this to work, by having the repeatlist be aware of which repeat it is playing
Im not sure if this has already been suggested, but if it hasn't, then i have a viable workaround solution.
1.) Insert a measure after the last measure before the repeat.
2.) Using 'measure properties,' set the actual measure duration to 1/"time signature," and the measure before to "time signature - 1/time signature." Example: if my time signature is in 4/4, i would set the newly added measure to 1/4, and the measure before to 3/4.
3.) Place the repeat barline at the end of the new measure, and a double sided repeat barline between the 2 measures.
4.) Turn the double sided repeat invisible with V
5.) Add a section break to the last measure, 1/2 of the time you want to pause
6.) In the measure before, add pauses or fermatas to compensate for the missing final beat of the measure.
7.) Adjust other elements if necessary to make it look how you want it to.
It may not be perfect, but it works for me. I can get the playback to work properly while maintaining the illusion of a normal score (since the double sided repeat is invisible). I hope this at least helps some of you
Came up once more in https://musescore.org/en/node/309042
Why hasn't this been fixed yet? It's been reported time and time again.
Feel free to pay me for my time I need to spend on implementing it.
More generally, bugs in open source software get fixed when someone who has the right combination of coding expertise + willingness to contribute their efforts to the project + time to do so, decides a given bug rises to the top of their own personal priority list. So far, this bug hasn't yet sufficiently bugged anyone with the necessary skills / willingness / time. No doubt that will change over time as higher priority bugs gets fixed and this one rises to the top of someone's list.
And jeetee specifically has given it a couple of tries but they haven't worked.
Came up again in https://musescore.org/en/node/103256
In reply to Came up again in https:/… by Jojo-Schmitz
I am also complaining about that bug ....
As quick patch attempts came up empty, I'm starting for real on this one at https://github.com/jeetee/MuseScore/tree/section_break_pause_playback. Of course, if anyone else wants to have a go at it, they're welcome to it as always: no guarantees on completion timing from me ;-)
Without any idea of the code. It seems like the proper response to this would be to have the Section Break activate a "Pause_Before" command on the FIRST tick of the following measure. Seems like that would make everything associated with the last measure of the section work normally, including repeats and jumps ... and THEN perform the pause before starting the next section.
Hope I'm making sense :-)
In reply to Without any idea of the code… by TheHutch
Yes; that was sort of what I had in mind!
Workaround is to set the pause property of the section break to 0, or to add an empty measure (and hide it)?
This bug came up again.
In reply to Came up again. by scorster
And again https://musescore.org/en/node/328241
After some conversation with the internal team I'm postponing work on this until the new playback system for MuseScore 4 is in place.
Happens even if the section bresk is not at the (finale) end repeat, but at a following vertical frame. Guess because that doesn't have a tick of its own, still is somewhat surprising
It's indeed because a frame doesn't take ticks
Maybe having just one tick would help here already ;-)
I just came up with a different variant on the empty measure workaround. Mostly differs in how to hide/skip it.
To skip the measure from being considered in playback:
* Add a closed volta over it
* Set the repeatList to "0"
* Mark it invisible as well
To fix the system layout:
* Insert a horizontal frame in front of the extra measure
* Uncheck "create system header"
* Size it negatively to make the visual end barline match up; for a minimum measure width of 5sp and an end repeat barline that should be around
-5.25sp
Example attached.
See https://github.com/musescore/MuseScore/issues/14262