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.

Sign up for GitHub

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

Jump to bottom

Show playing notes on the piano keyboard #21874

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Show playing notes on the piano keyboard #21874

wants to merge 7 commits into from
+602 −7

Conversation

adazem009
Copy link
Contributor

@adazem009 adazem009 commented Mar 10, 2024 •
edited
Loading

Resolves: #14428
Resolves: #23054

The current implementation of MIDI output isn't very elegant. FluidSynth, which is used only with MS Basic, is used to send MIDI events to MIDI output devices. That means using e. g. Muse Sounds will disable MIDI output completely.

It was therefore necessary to add a special synthesizer which handles MIDI output and nothing else (no audio playback). This synthesizer is always active, even if Muse Sounds or VST is used. The MIDI events aren't, however, sent to MIDI devices from the synthesizer, but instead sent through a channel in PlayerHandler so that any module outside framework can listen to note MIDI events.

There's a MidiInputOutputController class in the notation module which currently only handles MIDI input. Because of the class name, it makes sense to handle MIDI output there too. To avoid sending MIDI events of muted mixer channels (#18389), it was necessary to make synthesizers aware of track ID so that the special MIDI output synthesizer can send MIDI events with the track ID. The track ID is then used to get the mute state of the channel.

The visualization of notes on the piano keyboard is implemented using the MIDI event channels too.

Please note:

  • It probably isn't the best idea to have 2 synthesizers running simultaneously because it might slow things down, but maybe it'll give you some ideas for a better solution.
  • I didn't add a sequencer for the MIDI output synthesizer because FluidSequencer is sufficient. Maybe FluidSequencer could be moved out of FluidSynth to be a common sequencer for these two synthesizers.
  • I also handled edge cases for the piano keyboard such as when an instrument is removed during playback.
  • Issue MIDI Note Off not sent on pause #19254 can be easily fixed after these changes because the MIDI output handling can be extended with a list of played keys just like in the piano keyboard controller (and the playingNotesRevoked() channel could be used).
  • Notes coming from muted channels won't show. This is something that wasn't in MuseScore 3.
  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@adazem009 adazem009 force-pushed the piano_keyboard_playing_notes branch from 52998ac to 5ea4a62 Compare March 10, 2024 11:56
@adazem009 adazem009 mentioned this pull request Mar 10, 2024
Closed
8 tasks
@Jojo-Schmitz
Copy link
Contributor

rebase needed

@ShawkMusic
Copy link

Any progress on getting this merged? I really want this feature. As a pianist, it would really help me to visualize and understand the music written.

@adazem009 adazem009 force-pushed the piano_keyboard_playing_notes branch from 5ea4a62 to c293fed Compare April 26, 2024 10:31
@adazem009
Copy link
Contributor Author

rebase needed

Done. I've also changed the source file headers of new files to MuseScore Studio (20dd58e). Please check if it's correct.

@adazem009
Copy link
Contributor Author

Any progress on getting this merged? I really want this feature. As a pianist, it would really help me to visualize and understand the music written.

I guess the MuseScore team is busy with more important issues, so it may take some time until they get into this one. In the meantime, you can test this PR using these builds:

Windows
Linux
macOS

@ShawkMusic

This comment was marked as resolved.

Sign in to view
@adazem009

This comment was marked as resolved.

Sign in to view
@ShawkMusic
Copy link

@ShawkMusic Looks like the application forces the wayland platform plugin, but it's broken (probably after switching to Qt 6 in master while 4.3 still uses Qt 5 if I remember correctly). To force the xcb platform plugin, use:

export QT_QPA_PLATFORM=xcb

Then run the AppImage.

Yes, now it works! I can see the notes in the score being played on the keyboard now! Thank you! I can finally see my beautiful harmonies! Hopefully this gets added to the real app soon...

Here's some little nitpicks:

  • Selected notes shouldn't be highlighted on the keyboard during playback, only when the player is paused.
  • Unpitched percussion shouldn't display notes during playback.
  • It would be nice to be able to distinguish notes from different instruments, maybe by coloring the notes following a color code (maybe something like this), (I imagine this might be hard to do since I don't think the keyboard even supports different colors, but I figured I should write this down somewhere)

Either way, overall in it's current form it's still very good and much, much better then not being able to see anything at all. I think it's in about the same state that I remember it being in Musescore 3.

@adazem009
Copy link
Contributor Author

@ShawkMusic Looks like the application forces the wayland platform plugin, but it's broken (probably after switching to Qt 6 in master while 4.3 still uses Qt 5 if I remember correctly). To force the xcb platform plugin, use:

export QT_QPA_PLATFORM=xcb

Then run the AppImage.

Yes, now it works! I can see the notes in the score being played on the keyboard now! Thank you! I can finally see my beautiful harmonies! Hopefully this gets added to the real app soon...

Here's some little nitpicks:

  • Selected notes shouldn't be highlighted on the keyboard during playback, only when the player is paused.
  • Unpitched percussion shouldn't display notes during playback.
  • It would be nice to be able to distinguish notes from different instruments, maybe by coloring the notes following a color code (maybe something like this), (I imagine this might be hard to do since I don't think the keyboard even supports different colors, but I figured I should write this down somewhere)

Either way, overall in it's current form it's still very good and much, much better then not being able to see anything at all. I think it's in about the same state that I remember it being in Musescore 3.

  1. I agree with this too because it would make the keyboard more clear during playback. And it isn't difficult to implement.

  2. MuseScore 3 behaves the same if I remember correctly. I'm not going to resolve this in this PR, but the MuseScore team should discuss the design after this is merged.

  3. The piano keyboard currently uses the accent color and I don't think it's difficult to implement because the piano keyboard already has the information about instruments.

@adazem009
Make synthesizers aware of track ID
a528470 
MidiInputOutputController in the notation module will listen to events coming from the synthesizers and knowing the track ID is necessary to check whether it's the metronome or a chord track.
@adazem009
Add channels for note MIDI events
f05c379 
Modules outside framework can listen to these channels to receive note MIDI events, for example to output them to MIDI devices or to show the playing notes on the piano keyboard.
@adazem009
Add MIDI output synthesizer
9b15553 
There must be a special synthesizer to handle note MIDI events so that they can be sent to MIDI devices even when using synths such as MuseSamplerWrapper.
The MIDI events are sent through the channels in PlayerHandler because they're going to be sent to devices by the notation module which has access to track information.
@adazem009
Add MIDI output synth resolver
44fc4a0
@adazem009
Use MidiOutputSynth in EventAudioSource
fca8f28 
Use the MIDI output synthesizer together with the synthesizer which is used to play audio.
@adazem009
fix musescore#14428: Show playing notes on the piano keyboard
ef79184 
The note MIDI event channels can now be used to easily implement this feature.
Edge cases such as removing an instrument while playing are handled too.
@adazem009 adazem009 force-pushed the piano_keyboard_playing_notes branch from c293fed to ef79184 Compare June 1, 2024 16:28
@adazem009 adazem009 mentioned this pull request Jun 1, 2024
Open
@bkunda bkunda requested a review from cbjeukendrup June 4, 2024 13:36
@adazem009
Copy link
Contributor Author

I can split the changes into multiple PRs to fix #23054 first, if needed.

@bkunda
Copy link

bkunda commented Jun 6, 2024

This PR needs a proper review, and potentially some further investigation, by @RomanPudashkin, because it makes changes to the audio engine that may affect performance. Unfortunately this will likely take some time due to other competing demands on his work slate at the moment. Thank you for your understanding and patience!

@sneakysushi

This comment was marked as off-topic.

Sign in to view
@cbjeukendrup

This comment was marked as off-topic.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RomanPudashkin RomanPudashkin Awaiting requested review from RomanPudashkin

@cbjeukendrup cbjeukendrup Awaiting requested review from cbjeukendrup

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@adazem009 @Jojo-Schmitz @ShawkMusic @bkunda @sneakysushi @cbjeukendrup