MuseScore
MuseScore copied to clipboard
musescore
Fix multiple MIDI out issues
Resolves: #22354 Resolves: #18382
For a long time the midi output was broken in MuseScore 4. Due to inconsistent use of MIDI 2.0 and MIDI 1.0 data values a midi 1.0 note velocity of 64 would be interpreted in Midi 2.0 as very small and substitued by 1. The pull request fixes this by small changes in fluid synth event generation and event interpretation code. Also handling of Midi event lists on the mac, both output and input was not correct. I fixed it as well. I added some needed utilities and fixed some code in Event type that I noticed was incorrect. I tested Midi Input and output on the Mac.
Another change is usability of MIDI output. Currently the user has no way to assign a port and channel to a staff. As a workaround I suggest using the staff index as the channel number (implemented). When stopping playback, now also All-Note-Off commands are sent to MIDI output.
- [x ] I signed the CLA
- [ x] The title of the PR describes the problem it addresses
- [ x] Each commit's message describes its purpose and effects, and references the issue it resolves
- [ x] If changes are extensive, there is a sequence of easily reviewable commits
- [ x] The code in the PR follows the coding rules
- [ x] There are no unnecessary changes
- [ x] 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)
Wouldn't similar changes as those you did for Mac be needed for Windows and Linux too?
For Windows the MIDI call is midiOutShortMsg, taking a 32 bit Word containing a Midi 1.0 message. It requires no packet length. For Linux, AlsaMidiOutPort also converts the messages to Midi 1.0 and dispatches on message type. Looks good. The other changes should fix the low velocity issues on those platforms as well. But I did not test it.
Velocity is still 1 (lowest) for all notes on Musescore 4.4.2 on a Windows 10 platform (latest version). No progress since the first report of this regression. See also (...)/MuseScore/issues/22354 What about a simple patch to set velocity on 64 instead of 1? At least the music would be audible on the keyboard, waiting for a better fix.
@Flying-Roger Could you please build, run and test if this pull request resolves the MIDI-out velocity issue on Windows 10? @cbjeukendrup When will this pull request eventually get merged? I have not heard anything from you since 3 weeks.
Sorry that I hadn't replied. We're awaiting a review from @RomanPudashkin, because personally I'm not knowledgeable enough about the MIDI specifics to really validate these changes.
@Flying-Roger Could you please build, run and test if this pull request resolves the MIDI-out velocity issue on Windows 10? @cbjeukendrup When will this pull request eventually get merged? I have not heard anything from you since 3 weeks.
Sorry for not being up to speed with github: not yet equiped to build, but I can and am willing to run and test if you send me a link to an exe (Windows 10).
You can find a download link here: https://github.com/musescore/MuseScore/actions/runs/11093391357 See How to download test builds from pull requests for instructions how to run it, and also how to find the latest download link yourself when there are new changes.
You can find a download link here: https://github.com/musescore/MuseScore/actions/runs/11093391357 See How to download test builds from pull requests for instructions how to run it, and also how to find the latest download link yourself when there are new changes.
Thank you for the fix ! I just downloaded the build and I tested it on my Casio keyboard + monitoring with midiview. The velocity is now set correctly and all notes are audible. UPDATE: During my initial test, the nuances were not taken into account, but A new test, creating a new score in MS4 and applying nuances (p, f) worked fine. More tests were also successful, so I have no idea what went wrong for the nuances on my first test.
I just dowloaded the build and I tested it on my Casio keyboard + monitoring with midiview. The velocity is now set to 64 and all notes are audible.
Thanks for testing. Did you use a score with different loudness levels (pianissimo to fortissimo), which is also audible with internal speakers? On my Mac, the monitored midi note velocities change. On Windows they should also not be always 64.
I just dowloaded the build and I tested it on my Casio keyboard + monitoring with midiview. The velocity is now set to 64 and all notes are audible.
Thanks for testing. Did you use a score with different loudness levels (pianissimo to fortissimo), which is also audible with internal speakers? On my Mac, the monitored midi note velocities change. On Windows they should also not be always 64.
Sorry, my first test was probably too quick: I could not reproduce the problem. Today, I tested again on a "fresh" score and it worked. Adding p,f nuances to an old MS3 score worked as well. No ideal what I did wrong initially. I noticed that the mixer has no effect on the midi play; is that normal ?
Hello! I am a midi guy, and just jumping in on this issue because I noticed the problem and found this pull request related to it.
I would love to help out with this issue. I'm honestly very much into the raw midi output of notation software. Is there any specs on what is supposed to output through midi based on the score data? I can create some test files if there aren't any yet.
I tested it too on Windows11 23H2. I used the MSGS sound module while monitoring the MIDI data with a homemade tool. It works fine! Velocity value of note property also reflected output. Very Good!
The dynamics symbol was not reflected to the velocity value in note property. MIDI output value is OK, the value is 127. Is this another issue?
Hello, I did two tests, which I wrote today. I have added them, and the resulting midi files to this repository: https://github.com/chrisroode/MuseScoreMidiTests
I found that velocity does change according to what the velocity is set at in the note properties window. However, I did find another issue...I think it's unrelated, but the pitch output will reveal it.
I honestly do not know what is expected to be output through the MIDI when accents, dynamics, and different instrument implementations...etc are present, and I would like to know what that is, or help work on organizing that. But as for the magic number issue in midievent.h which was setting all the velocities to 1, I believe that has been fixed.
Hello! I am a midi guy, and just jumping in on this issue because I noticed the problem and found this pull request related to it.
I would love to help out with this issue. I'm honestly very much into the raw midi output of notation software. Is there any specs on what is supposed to output through midi based on the score data? I can create some test files if there aren't any yet.
Ideally, the user would expect to get the same contents on on the midi out port as when exporting the score to a midi file.
Hello! I am a midi guy, and just jumping in on this issue because I noticed the problem and found this pull request related to it. I would love to help out with this issue. I'm honestly very much into the raw midi output of notation software. Is there any specs on what is supposed to output through midi based on the score data? I can create some test files if there aren't any yet.
Ideally, the user would expect to get the same contents on on the midi out port as when exporting the score to a midi file.
I just took a look at that. When museScore exports a midi file, it uses Note On messages with velocity of 0 to express note off messages. That's different than playback. Although it's defacto the same in most midi situations, it is a difference. But I think this is a completely separate issue, of which I already created something for that.
@cbjeukendrup @RomanPudashkin This pull request ist 1 month old now. Given that users here are quite happy with the results that they get, would not it be time to make other users happy as well by merging this pull request? I understand that Roman (pending reviewer) may be busy with other issues, but I get the sad feeling that this one-week contribution of a professional music software developer (me) will eventually just be forgotten.
It won't be forgotten; eventually every PR will be considered. Anyway, the situation right now is that Roman is on vacation... so I guess all I can do is try harder to review it myself.
It won't be forgotten; eventually every PR will be considered. Anyway, the situation right now is that Roman is on vacation... so I guess all I can do is try harder to review it myself.
It sure is unfair to press you. I want you to know, that I appreciate your work very much. 😀❤️
For reviewing it might help to look at the commits in https://github.com/SolfaMode/MuseScore/tree/22354-fix-midi-out-velocity-and-midi-mac.
It might be helpful if you could separate the changes related to midiOutPort() and to m_sequencer.lastStaff() into a separate PR. These will need a bit more thought perhaps, and may conflict with other changes that are planned.
I agree, that lastStaff is a temporary solution for choosing a midi destination channel. It should be handled later in a more profound to-be-designed way. Currently the user has no way of specifying staff-specific midi output destinations. As a user I would love to manually assign midi destinations to staffs, comprised of midi output port (there might be multiple), midi 2.0 group, midi channel.
Much more needs to be redesigned: MIDI-output should be independent of the synthesizer choice (fluid-synth vs. Muse Sound) and should eventually be moved from the fluid-synth code to its own sequencer.
So while I could create multiple pull requests, only the whole solution creates a somewhat satisfying midi output. So I would rather not split this up but leave the proposed solution as a starting point for future discussions, designs and more profound implementations.
I and many people are waiting for this to get merged for a long time... .
「Why doesn't MIDI output work on Musescore 4?」 https://musescore.org/en/comment/1272177
I and many people are waiting for this to get merged for a long time... .
「Why doesn't MIDI output work on Musescore 4?」 https://musescore.org/en/comment/1272177
I fully agree: this is a long outstanding issue and the fix is more than welcome, even if more could be done later. Personnally, I considered this bug as a show stopper for moving to Musescore 4. Now, I use regularly MS4 instead of MS3, assuming that the fix will be integrated in a very near future with the other fixes recently implemented (e.g. fixed MS4 crashes). If this is not the case, I would seriously reconsider my choice.
@SolfaMode Sorry to insist, but I'd prefer not merging code that creates technical debt of which it is unclear when and how and by whom it is going to be resolved. So if you could still split off those m_lastStaff and if (port->isConnected()) { … changes to a separate PR, then I can review this one more easily and merge it soon. I know that that does't solve the full problem, but I think that's the best way to make any progress with this.
P.S. the MIDI output port changes to FluidSynth also conflict with the goal to remove MIDI output handling out of FluidSynth completely because it doesn't belong there, so that's another reason why I'm reluctant about merging them now.
@cbjeukendrup I feel quite sad about how this pull request is handled. First, the requested expert reviewer never joined the review. Then everything takes so much time. Then discussion after 2 months again starts at the same point.
What you have to weight is if a huge improvement in user satisfaction is worth a few lines of additional code. I think so and so do @Flying-Roger and @knoike and maybe others in this discussion. It is a very small technical dept.
Democracy is not always good, as we know from social media, but I think it should be part of Open Source decisions. It does not help this discussion to mark comments as off-topic. (I do not know who did it.)
If you feel overwhelmed by deciding about the pull request, then please ask another staff member for help. I there is no one You can ask, You may also pull the code and then do the changes yourself that you feel are necessary to avoid the technical dept. Sorry, I do not have time to do this.
The fact that I didn't join the conversation in this PR doesn't mean we didn't discuss it internally (which resulted in Casper's comment). We've planned this fix for 4.5, so we'll have to do something about it before the release (which will happen pretty soon). Please refrain from any non-tech related discussions in this PR. We have a pretty small team and we're all overloaded with lots of different tasks, so sometimes, unfortunately, we can lose some community PRs out of focus (this is actually a pretty common problem in open-source projects, so we're not unique in this regard)
I'm going to assign a QA to internally test your PR before merging it. If all good, we'll merge it and raise a tech debt issue. But anyway, thanks for your contribution, and sorry it's taking so long
@SolfaMode Could we possibly get this rebased and fresh builds for testing?
