Broken visual tracking of colliding notes

• Mar 16, 2018 - 23:17
Reported version
2.2
Type
Functional
Severity
S3 - Major
Status
closed
Project

After my MIDI fixes patches have been applied, we have an issue during in-application playback of scores containing colliding notes, made even better visible due to the fact that 2.2 also tracks them on the on-screen piano: Colliding notes are not properly cleaned up, making them stay blue (in the score and on the piano) even after they have been silenced.

The good message is that I’m working on a patch, the first edition of which is currently compiling and ready to be tested. I’ll update here once I know more.


Comments

Status (old) active patch (code needs review)
Status active  

I’ve added a possible fix at https://github.com/musescore/MuseScore/pull/3551 but it’s still WIP (first one didn’t even compile and was missing a small part, let’s see how the second one goes).

Code review is already welcome (again: first, please, whether the proposed way of fixing it goes into the right direction; style nits second after it, we can polish the patch once it’s decided that the idea is good, as much as we want).

I’ll put a notice when I think it’s ready.

In reply to by lasconic

No, that’s 100% correct.

With colliding notes, in MIDI, they are all collapsed. For example, in the screencast I posted (measures 28 and 29), look at the second and third staff:

The third staff has an F4 for four fourths (let’s call them beat 1 to 4).
The second staff has an F4 starting at beat 4 but going to beat 7. In beat 7.5 we get another F4, but a “new one”, which lasts until the end of beat 8:


beat |1   2   3   4  |5   6   7   8  |
     |---|---|---|---|---|---|---|---|
st.2             «F4----------»«F4--»
st.3 «F4------------»

Now, with the collision fixes, the first two on the timeline collide and are merged AND hoisted to the staff on which they first begun, that means:


beat |1   2   3   4  |5   6   7   8  |
     |---|---|---|---|---|---|---|---|
st.2                           «F4--»
st.3 «F4----------------------»

However, for the duration this whole+half+eigth note is played, all corresponding notes in the score should be highlighted, and, more importantly, afterwards, none of them. That is what this patch achieves.

Yes, strictly speaking the whole note F4 in the third stave is no longer “playing” when we arrive in beat 5, but technically it is, because of the collision merger.

The same happens in your example. You have a really long C4 in voice 2 which starts (beat 1) before the first C4 in voice 1 (beat 4). That means all the C4 in voice 1 in your example are removed from the score during playing (because they are already playing). However, they still sound, so they are still highlighted… for the duration of their sound, which is not their notated duration but the one from the held C4.

This precisely matches what’s be happening on a piano, or (perhaps better) on one manual of an organ. If you notate them in the same “channel” there is only one “key” that can be pressed, so it’s held throughout the duration of the longer note (tied wholes).

If you notate different instruments in one stave, however, you will have to make sure the voices in the stave are assigned separate MIDI channels; I think there’s an issue here somewhere, but AIUI this functionality does not exist yet. Iff they have different MIDI channels, the visual representation would be what one’d normally expect here. (In fact, if you look at the F4 in the first stave of my score, which is the soprano part and therefore has a different MIDI port:channel, you will see its duration is as expected.)

(To be honest, I was surprised for about a second as well when I first saw it, but it is correct, after all.)

In reply to by lasconic

Maybe. We could invent a new ME_* type which would not end up in the MIDI… but on the other hand, then you’ll have people writing bugreports because they don’t hear the C4 from your first voice (e.g. if your stave has piano as instrument).

The way I implemented it, the highlight completely matches what’s actually played.

Furthermore, I see a problem with keeping the events: you could fix the in-score highlights that way, but the highlights on the Piano widget would be completely broken again.

Also, given the short time until the 2.2 release, I’d rather have this than “stuck” keys…

I understand it is technically correct and reflects the inner midi structure, but as you said, it's rather unexpected. In the score, I would really prefer to see the notes highlighted when they are "sung". Of course for a piano it doesn't really matter but for singers, I guess it does (?)

In reply to by lasconic

Hmyes. I guess we really need a way to assign different MIDI channels to different voices in the same staff then.

With MuseScore before the MIDI fixes, you only heard the beginning of voice2 in your example and then the C4s in voice 1.

Now, after the fixes, you only head the C4 in voice 2, not the one in voice 1.

I think it’s okay to have the visual feedback of playback reflect this in playback. If you really have two independent instruments, you can notate them in different staves, or (since MuseScore is mostly about the notation) ignore the audible and visual artefacts of collision and care about the PDF.

With MuseScore 2.2 as it is right now, there are stuck highlights, which is even worse than with this patch applied.

In the original bugreport’s discussion at https://musescore.org/en/node/12971 it was mentioned that we need different fixes for the re-striking instruments vs. the not restriking ones. Currently, we only have implemented a fix for the latter — things like the piano or one organ manual.

I postulate that “fixing” the visual playback here will just make a different class of users unhappy, and that we’d get bugreports either way, and propose to write a FAQ entry addressing this.

I also postulate that, with this patch as-is, things will magically be correct once MuseScore supports assignment of different MIDI channels (port:channel in the MIDI file) to different voices in the same staff, which is the proper fix for the other instruments, at no further cost related to this code.

That being said, the violin only has three channels. We will need an instrument with four channels.

How about we change the generic Voice instrument in the Vocals group to have four channels with generic names (V1, V2, V3, V4 perhaps) and use it for this?

OK. I coded the solution with keeping the events but mark them. It's not better indeed... it feels weird to see the note turn highlighted but not hear it. Does someone else have an opinion about that ? I will do a PR with the code, just in case.

In reply to by lasconic

I think we definitively need either of the two fixes, and I think we definitely need a FAQ entry either way.

I have a slight preference for my solution if only for the fact that it matches MIDI output and sound and will automatically be correct once we get differing channels for voices.

I’ve thought a bit about the instruments, and I think we optimally need three more general instruments, one with two, one with three, one with four channels, to keep mixer confusion to a minimum. If we keep the names as generic as possible, the string freeze won’t matter for them anyway. Perhaps that is better than to hack an existing instrument.

I’ll experiment with it a bit. Can we have top-level instruments or do we need a new InstrumentGroup named workarounds or hacks?

Or do you prefer just hacking voice.vocals to have four channels and using that as a generic instrument for 2.2 and working on proper channel assignment for 3.0?

Edit: we can't have top-level instruments, and MusicXML export etc. all use the metadata of the actually chosen instrument, so either way it’s going to suck. Adding channels to all instruments, or even just a few, would be possible, but then, the mixer control would show “too many” lines for them. Ideally, it would just show channels that are in use.

I think we should have at least one instrument with four channels in instruments.xml, but vocal closed scores only need two… hm.

Perhaps the best solution is to document in the FAQ entry how to add another channel to an instrument in a score (edit the XML file, ugh… perhaps we should offer some example files ready-to-download for not so advanced users) and not do anything about the shipped instruments, and hope the problem goes away in the next release…

… or a quick code hack to add the ability to add channels to an instrument on the fly. But at this point in time, I’m wary of “quick code hacks”. Maybe the FAQ entry should just be linked from the release notes as known deficiency.

Edit 2: maybe offer something like the attached file for downloading, and perhaps not only generic voice but also other instruments that could be affected like this (usually notated as different voices in the same stave), and then document how to switch instruments to them and change channels in the FAQ.

Could also use the <init> tag to initially populate “voice2”, but that does not allow me to rename the first voice from “normal” to “channel 1”, so… (but perhaps better?)

Attachment Size
i.xml 1.12 KB

FWIW, unless I am totally misunderstanding this report, to me this is minor or normal at worst, nowhere near major. Is there really something that isn't working, as opposed to just a temporary visual glitch? As far as I can tell the highlighting is fixed when playback stops?

In reply to by Marc Sabatella

The highlight stops when playback stops, yes, but until then, you get stuck keys and notes, which breaks all kinds of scenarios - such as https://twitter.com/musescore/status/955829983552983040 - so it should be fixed, especially when fixes are already available.

(That, plus I’m in a bit of “I broke it, I fix it” mentality. This didn’t happen before, although the general state of before was worse than it is now.)

We’re currently considering the best way to fix it. But even that can be done incrementally… both my patch and lasconic’s are a way up (but we need a FAQ entry either way, as the release is too close). There’s ways to make things even less annoying, but the remaining issue, with my patch for this bug applied, is something that wasn’t even in the scope for the first bugfix in the first place…

We don't modify instruments.xml right now. Too late. And it's just a bad trick.

So we just fix the issue with one of our methods.
When two notes or more notes are colliding we play a sound that start on the very first note and span to the very last note. All other notes in between are not played. That will stay.

The question is as follow: what do we highlight in the score ?
* With mirabilos' solution, all notes are highlighted during the playback.
* With lasconic solution, notes are highlighted when they would be played if they were not silent.

A possible third solution would be to not hightlight them at all since they are silent. Just like muted notes in fact.

What do you prefer ?

In reply to by lasconic

I agree about the bad trick. So perhaps we’d just offer an XML file for download from the FAQ entry, like the one I posted.

I like the third solution slightly more, since it also matches playback, but still prefer mine, because:
• they are not mute
• this matches playback
• this matches what is actually played on a non-restriking instrument (which was the initial scope of the bugreport)
• the highlighting from my fix better demonstrates that, if you want to use the different voices for different instruments, you have to change more (i.e. the voice to MIDI channel assignment)
• with my fix, we won’t have to touch this code later again, as it will Just Work™ right once voice→channel selection code is in place

Consider also scores like the following, where the held note is hoisted to the second stave for the entire duration of its playback, and how the other two proposed solutions work with it (or perhaps not).

Attachment Size
test_voice_collision.mscz 6.22 KB

Summarizing my opinion as given on irc today:
I currently favor the highlight-the-notes-when-the-cursor-passes-it approach (so the lasconic version).

But if (for 3.0?) both this silent-rendering-in-same-channel as well as a "normal user"-friendly implementation of voice->channel mapping are in place; then the latter will automatically match the current highlight behavior. Imho it would then make sense to have the mira highlighting for same-channel playback.

In reply to by jeetee

Heh, we’re getting bugreports already either way, from the playback: https://musescore.org/en/node/270562 where the user complains about the (end of the) short note not being audible. (Still an improvement over before where the long note was cut short, but I guess this is an issue with string instruments…)

(Which is why I think we need visualisation matching playback. Also makes bugreports like this one way easier to debug.)

In reply to by mirabilos

With visual feedback such as the piano keyboard it is impossible to show both long and short notes being played at the same time. One key ALWAYS plays only one note. Since this is the case and there is a definite note being notated, the repetition of the note being displayed is proper. When it comes to playback, I would prefer a realistic sound like most others would. Hopefully one day the playback will be much better, including the sustained notes with the same pitch as a shorter note. As long as you are trying to use a piano keyboard to display multiple notes played on multiple instruments (or strings) you will never get an exact 1 to 1 display for every note.

I can't say I fully understand the nuances of all this, but I fear you may be seriously underestimating the seriousness of this regression. From what is being reported, this is worse than 2.1, at least for this particular score. I'm guessing that's because guitar notes have their own decay that is kind of separate from the duration, so it wasn't so much of a problem that the long notes were cut short. Having the shorter notes not sound at all is much worse. Or maybe something else is going on. All I can say is, I listened to the sample score provided using both 2.1 and 2.2, and can absolutely verify that 2.2 is a step backwards. To the point where if we can't resolve that, I'd advocate reverting the change.