fix #64251: crash on playback of ornament on tab staff

[MarcSabatella]

The crash was from trying to calculate an appropriate accidental so we can decide whether to go a half step or whole step (or something else potentially). Neither accidental nor key calculations make sense on tab staves, however. I'm not sure what makes sense to do instead, so I'm preventing the crash by always going with half steps for tab.

@jimka2001 : feel free to try to come up with something more clever.

[mgavioli]

@MarcSabatella : I know I am not objective on this, but this is another example of why all this juggling with ornamentation playback is opening one can of worms after the other, forcing to invent 'abstract' solutions with little or none relationship to actual practice.

I do not have a better solution for this specific problem and anyone complaining (in future) is also welcome to come up with something more clever. I even believe a better solution is likely not to exist.

So, thanks for fixing this issue. As I said for other topics, I think to have expressed my opinion clearly enough and I will now shut up.

[lasconic]

@jimka2001 Any opinion on this?

I'm a bit concerned that scores will standard staff + tab staff will have weird playback while we have the info. We do render only one staff to midi but we don't necessarily choose the standard staff as the one we render. If we don't have a clue about the right behavior for tab staff, then maybe we shouldn't render ornaments at all on these and make sure the standard staff is used. Any thought?

[MarcSabatella]

I'm certainly OK with just not rendering ornaments on tab staves at all, or at least not doing to by default. It just turned out to be about as easy to make this fix as to disable it for tab.

One thing to keep in mind: currently, Staff::primaryStaff() is used to decide which staff should be used for playback. Right now, it's just picks the first staff it finds. That same function has been used elsewhere in the code to identify the first staff of the group for other purposes as well. Currently, I see it used calculating barline spans while generating parts, but if was formerly used while filling staves with rests during score creation, and while that call was removed recently, there was a problem that actually just proposed fixing by adding it back: #2084. I was aware while doing this that there could be an issue down the road if primaryStaff was ever rewritten to not choose the first staff of the set but to somehow identify the playback staff some other way.

If we do extended primaryStaff() to prefer standard staves, maybe we could add a parameter to specify which behavior to use. Or have two separate functions.

[lasconic]

This commit can hurt, so I merge it. In a follow up I will make sure that primaryStaff() send a non tab staff if there are linked staves and ornaments will not be played in standalone tab since we lack info.

[jimka2001]

Hi Nicolas, what's a tab staff and why does it have ornaments?
On Jun 22, 2015 5:46 PM, "Nicolas Froment" notifications@github.com wrote:

This commit can hurt, so I merge it. In a follow up I will make sure that
primaryStaff() send a non tab staff if there are linked staves and
ornaments will not be played in standalone tab since we lack info.

—
Reply to this email directly or view it on GitHub
#2075 (comment).

[lasconic]

A tab staff == a tablature staff.

[jimka2001]

I’d like to see an example score to help me understand this issue. I feel a little lost in the discussion.

On 22 Jun 2015, at 17:11, Marc Sabatella notifications@github.com wrote:

I'm certainly OK with just not rendering ornaments on tab staves at all, or at least not doing to by default. It just turned out to be about as easy to make this fix as to disable it for tab.

One thing to keep in mind: currently, Staff::primaryStaff() is used to decide which staff should be used for playback. Right now, it's just picks the first staff it finds. That same function has been used elsewhere in the code to identify the first staff of the group for other purposes as well. Currently, I see it used calculating barline spans while generating parts, but if was formerly used while filling staves with rests during score creation, and while that call was removed recently, there was a problem that actually just proposed fixing by adding it back: #2084 #2084. I was aware while doing this that there could be an issue down the road if primaryStaff was ever rewritten to not choose the first staff of the set but to somehow identify the playback staff some other way.

If we do extended primaryStaff() to prefer standard staves, maybe we could add a parameter to specify which behavior to use. Or have two separate functions.

—
Reply to this email directly or view it on GitHub #2075 (comment).

[MarcSabatella]

First, if you are not familiar with tablature, look it up, presumably there is info on Wikipedia or elsewhere. But it's a way of notating music that represents notes for stringed instruments not in terms of their pitches, butin terms of which string is pressed at which fret. There is no concept of a key signature or of accidentals - telling the reader to press the 3rd string at the 2nd fret is always completely unambiguous.

So the issue is, there is simply no useful information available to the code that tries to calculate how many half steps up or down to play for a trill etc. We know 3rd string / 2nd fret works out to an "A", but we have absolutely no idea if it makes sense to trill up to a Bb or a B - there is no key signature, there are no accidentals.