Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Piano views don't visualize playing of successive same-key notes. #3919

Open
Sawuare opened this issue Oct 27, 2017 · 11 comments
Open

Piano views don't visualize playing of successive same-key notes. #3919

Sawuare opened this issue Oct 27, 2017 · 11 comments

Comments

@Sawuare
Copy link
Member

Sawuare commented Oct 27, 2017

wow

@qnebra
Copy link

qnebra commented Oct 27, 2017

It looks like minor overseight.

@PhysSong
Copy link
Member

Does it happen on piano views on instrument windows?

@Sawuare
Copy link
Member Author

Sawuare commented Oct 27, 2017

Does it happen on piano views on instrument windows?

Yes.

@Sawuare Sawuare changed the title Piano roll doesn't visualize playing of successive same-key notes. Piano views don't visualize playing of successive same-key notes. Oct 27, 2017
@musikBear
Copy link

musikBear commented Oct 27, 2017

@Sawuare if you magnify to max, are the notes

  • overlapping,
  • gapping

-or are they

  • snapped?

@Sawuare
Copy link
Member Author

Sawuare commented Oct 27, 2017

@musikBear The notes are snapped.

@PhysSong
Copy link
Member

I think it's not hard to fix. I think it would be work correctly if we set the state based on the number of running keys instead of MIDI events.

@DomClark
Copy link
Member

DomClark commented Jul 8, 2018

It seems to be sufficient just to move m_piano.setKeyState into the if statement here:

case MidiNoteOff:
m_midiNotesMutex.lock();
m_piano.setKeyState( event.key(), false ); // event.key() = original key
if( key >= 0 && key < NumKeys && --m_runningMidiNotes[key] <= 0 )
{
m_instrument->handleMidiEvent( MidiEvent( MidiNoteOff, midiPort()->realOutputChannel(), key, 0 ), time, offset );
}
m_midiNotesMutex.unlock();
break;

@Sawuare
Copy link
Member Author

Sawuare commented Oct 20, 2019

Seems fixed on master and 1.2.0.

@Sawuare Sawuare closed this as completed Oct 20, 2019
@zonkmachine
Copy link
Member

This was fixed by @DomClark in #4908.

@DomClark
Copy link
Member

The bug certainly seems to have disappeared, but I'd hesitate to call that PR a proper fix for this issue.

Prior to it, note-on messages were sent when NotePlayHandles were constructed during track processing, and then cancelled out by note-off messages sent from old NotePlayHandles during play handle processing. Now that the note-on messages are also sent during play handle processing, they don't get cancelled out since the new NotePlayHandles get put at the end of the list of play handles to process. However, the processing of play handles is handled by multiple threads, so it is technically possible for one of these later handles to send its note-on before the note-off of a previous handle, which would trigger the bug again.

Since this doesn't seem to happen in practise, and this is only a visual bug, I'm happy for this to stay closed, but anyone wanting to fix this properly should look at the suggestions mentioned earlier in this issue.

@Sawuare
Copy link
Member Author

Sawuare commented Jun 11, 2021

Reopening because #6025 recently reported what I think is the same bug.

@Sawuare Sawuare reopened this Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants