Album segfault {2parts-excerpted}.append( {1part-notexcerpted} );
I'm trying to diagnose exactly, so bear with my temporary title. The issue is something to do with a particular mismatch number excerpts and number staves...
If make album of:
1. flute-piano w/excerpts
2. flute-piano w/excerpts
3. flute
and create joined score, then there is a segfault with stack trace:
Score::appendScore line 2112
Score::addRest line 162
Score::undoAddCR line 1321
Measure::undoGetSegment line 748
Measure::findSegment line 733
Measure::first line 215
SegmentLine::first line 40
Issue occurs on latest b376184 ...I believe it was introduced with https://github.com/musescore/MuseScore/commit/6da9a6242d0e2d9bea5081267… which allows scores with different excerpt counts to be joined together by first removing excerpts (so just appends base scores, not children scores). The segfault happens when filling in full measure rests, right after finished cloning measures of 3rd score, because the 3rd score has fewer staves.
Attachment | Size |
---|---|
flute-piano-one-meas-parts.mscx | 14.25 KB |
flute-one-meas.mscx | 3.54 KB |
Comments
i'll investigate some more tomorrow. For now, here is the test case filled as a tst_album function:
https://github.com/musescore/MuseScore/commit/fccce6cb5b0e97b3fb2efc5c9…
it suspect has something to do with doing undoAddCR when there is no actual CR segment present initially.
Hello eric from "yesterday". That was a long night.
Anyway, I'm going to tackle now...
But, I'm a bit perplexed how my commit fccce6c (which was a test case in tst_album) is somehow in musescore/MuseScore.git because I don't believe I have commit access and didn't include it in any PR. Did I somehow unintentionally commit? It doesn't seem to be in "master", but somehow it is here:
https://github.com/musescore/MuseScore/commit/fccce6cb5b0e97b3fb2efc5c9…
I'm afraid I don't understand git enough to know what is going on. Could someone enlighten me?
fccce6c indeed is not in master (and that wrong indentation from album.cpp line 152 is still in, now in line 175) and I can't find it anywhere else either ?!?
OK. I wonder if it a github bug?
I'll be redoing my album test, so I will commit what needs to be correct.
regarding that fccce6c magical commit issue, I'm suspecting I had a "Detached Head" such that that commit was never connected to any branch, and therefore somehow managed to find its way inside the official musescore/MuseScore.git repository.
Anyway, I'm returning to this bug...got latest git master (a6ec8aa) & followed my replication steps, and I get the segfault (am on win 8.1 x86-64 now) on line 40 of segmentlist.h:
and with the debugger evaluator saying _first is "", I believe because the this pointer (of type Ms::SegmentList*) has address 0xc8, which clearly is an invalid address for a pointer because regular pointers shouldn't be so small.
Here's stacktrace:
and here is console output:
So clearly my debug statment "Debug: Will not join parts." at https://github.com/musescore/MuseScore/blob/d8e437343562c47561d4a14cafd… is warning me that mscore will not attempt to join parts... But it still should be able to continue on and just join the parent scores...
note: Album::createScore() is able to append the 2nd flute-piano movement, but fails when appending the flute-only movement, during the time when filling with rests, specifically when filling the 2nd staff (the 1st piano staff) of measure 1.
I've discovered that issue doesn't have anything to do with the number of staves in each part. Since if I do {flute part & recorder part} + {flute part & recorder part} + {flute}, then I get same error, with identical stacktrace.
things go wrong in this line:
https://github.com/musescore/MuseScore/blob/master/libmscore/undo.cpp#L…
Since tick2measure returns NULL, because no measure with tick=3840 is found in the 1 measure score we're appending. The next line dereferences the NULL Measure*, which clearly causes the rest of the problems, I believe.
tick 3840 corresponds to 3rd measure. That would be the 1st measure of the final appeneded score (the plain flute score), since all my test scores are 1 meas each. But when I was looking in the tick2measure, the score only had 1 measure. Maybe it should be looking for tick 0 then, not tick 3840. There are simply no measures for tick 3840 on that flute-only score...
I believe {2inst}w/excerpts + {1inst} successes by luck because line tick2measure considers the tick 1920 to belong to the first measure of a 1-measure score, since uses "tick <= lm->endTick()" https://github.com/musescore/MuseScore/blob/ad302568e4b216cb1b69cf70845…
so problem is before that tick2measure is called.
so in Album::createScore when try to append root score, then in Score::appendScore, when cloning the measures, if the initial score has excerpt while 2nd score has no excerpt, it is somehow not actually cloning enough measures...
I've managed to simplify the test case down to: {2parts w/excertps} + {1 part no excerpts, 2measures}
so what is happening is Score::Append when cloning measures is only appending one stave, so what happens is the resultant score Measure list end up like:
but should really have blank measures for the 2nd staff like:
I'm going to take a break now, but I know what needs to be done now...those extra blank measures need to be put in. (The code that puts rests in those staves will work correctly, once they have measures).
I'm going to make an additional test which does nothing more than:
since that is basically where the fault lies. Once that is fixed, then the album will work...
I'm going to take a break now, but I should be able to fix this fairly quickly.
I've deteremined the issues occurs because one movement has excerpts & one doesn't. Although we are only appending the root scores, however when adding rests into the blank measure, we are adding a ChrordRest element which is still linked with the excerpt staff. So I need to figure out the most appropraite way to only add a rest which is for the single staff, and not it's linked cousin.
I belive the removeExcerpts section:
https://github.com/musescore/MuseScore/blob/master/mscore/album.cpp#L17…
Does not properly completely remove the excerpts, since the individual elements of the staff still have all their elements pointing to their linked cousins. I believe I will have to do something similiar to what lasconic did with https://github.com/musescore/MuseScore/commit/8bf08c232818e04150481ceb2… but I'm trying to figure out how to properly unlink, and maybe if their is some of lasconic's code I should make a function out of to reuse.
There is also another thing I need to consider now: How should albums behave when a score has a linked staff to another staff in that score that is not a child score? Also I'm thinking I need to put more strict rules on what will are the permitted ways that scores can be joined together into an album. Currently there is just a weak test, which just checks if number of excerpts are equal between scores. But I think that will need to place some more strict rules, that basically say that if want to include linked parts and linked scores in resultant album, that entire organization of each score must be indentical. (And the best way for user to do this is to use the same tempate for each movement). Otherwise, if have different organization of child parts, then child parts are not transfered to resultant joined alubm. And if have different layout of linked staffs, then will simply prohibit joining.
how about just joining the scores and ignoring the parts?
Maybe just in case of mismatches and after an appropriate warning?
Joining the scores and ignoring the parts is what is currently done...BUT there is some bug in implementation, because there are stray linked elements in the staffs. What is done here: https://github.com/musescore/MuseScore/blob/d8e437343562c47561d4a14cafd… doesn't seem to be sufficient.
I will give an apporpriate warning in case of mismatches.
just making a note to myself that my test case still produces a fault (https://github.com/ericfont/MuseScore/commit/35af05d42526a9057ab1bda9d1…)
Relates to #292005: [EPIC] ALBUMS issues.
Even if currently disabled, relates to #303762: [EPIC] Collection of Undo issues