micdah / RtMidi.Core

RtMidi for .Net Standard
https://rtmidicore.micdah.dk/
Other
36 stars 12 forks source link

SysEx messages limited to an unknown buffer size #32

Closed MacroDX closed 1 year ago

MacroDX commented 1 year ago

Hi there, I tried to receive a SysEx data dump with a size of 18108 bytes. Waiting for the callback always results in a timeout which suggests that the message is swallowed in an undelaying layer. On the one hand I have not found an option to set a buffer size and on the one hand I would have expected for the callback to be invoked multiple times until all packages have been received. Is that a known issue? And is there anything that you or I can do? Thanks

micdah commented 1 year ago

Can you provide some code to reproduce the issue or similar?

I believe I should get entire MIDI messages from the underlying rtmidi library, and when I decode the data I look for the status byte and react to a SysExStart:

            internal const byte SysExStart = 0b1111_0000;
            internal const byte SysExEnd = 0b1111_0111;

In which case we:

                        case Midi.Status.SysExStart:
                            if (SysExMessage.TryDecode(message, out var sysExMessage))
                                SysEx?.Invoke(this, in sysExMessage);
                            break;

Which simply does

        internal static bool TryDecode(byte[] message, out SysExMessage msg)
        {
            if (message.Length <= 1)
            {
                Log.Error("Incorrect number of bytes ({Length}) received for SysEx message", message.Length);
                msg = default;
                return false;
            }

            if (message[0] != Midi.Status.SysExStart || message[message.Length - 1] != Midi.Status.SysExEnd)
            {
                Log.Error("Not a valid SysEx message received for SysEx message", message.Length);
                msg = default;
                return false;
            }

            msg = new SysExMessage
            (
                message
            );
            return true;
        }

So if the messages was truncated, I expect there should be an error log as the last byte wasn't the expected SysExEnd, or if only the start byte was received we should have seen an error on receiving only 1 byte.

It uses Serilog, so you can enable output by configuring Serilog in your code.

MacroDX commented 1 year ago

Hi and thanks for taking care of this.

I configured Serilog für my test output and got this, which is exactly the error message you listed above plus consecutive outputs of 'unknown message type' errors (see below). This strongly supports my suspicion that SysEx messages are not treated as packages until receipt of F7. But this obviously stems from the rtMIDI layer. Since you are using a fork of it, maybe there´s a way you could fix it.

[18:17:06 ERR] Not a valid SysEx message received for SysEx message [18:17:07 ERR] Unknown message type 00 [18:17:07 ERR] Unknown message type 00 [18:17:07 ERR] Unknown message type 00 [18:17:08 ERR] Unknown message type 00 [18:17:08 ERR] Unknown message type 00 [18:17:08 ERR] Unknown message type 00 [18:17:09 ERR] Unknown message type 00 [18:17:09 ERR] Unknown message type 00 [18:17:09 ERR] Unknown message type 00 [18:17:09 ERR] Unknown message type 00 [18:17:10 ERR] Unknown message type 00 [18:17:10 ERR] Unknown message type 00 [18:17:10 ERR] Unknown message type 00 [18:17:11 ERR] Unknown message type 00 [18:17:11 ERR] Unknown message type 00 [18:17:11 ERR] Unknown message type 00 [18:17:12 ERR] Unknown message type 00

micdah commented 1 year ago

I see, well yes it does indeed sound very much like that is what's going on.

I think I might be able to handle it in RtMidi.Core, by recognising the SysExStart and then keep consuming consecutive messages until I get the expected SysExEnd and then reconstruct all those into a single SysExMessage.

I'll have a look at it and keep you updated.

micdah commented 1 year ago

Sorry, didn't mean to close the issue from PR merge

Could you test this pre-release version of the package and see whether the truncated messages are now picked up correctly?

https://www.nuget.org/packages/RtMidi.Core/1.0.53-beta-1

MacroDX commented 1 year ago

I just did and it worked :)

micdah commented 1 year ago

I just did and it worked :)

Great, I'll make a release version later then 🤟

micdah commented 1 year ago

Excellent, just pushed for a release version 1.0.53 to be build and published.

Should be out shortly

MacroDX commented 1 year ago

Thanks again for fixing that so quickly :) What I forgot to mention though this morning (before the first coffee) : The 'Unknown message type 00' is still put out by the logger, which isn´t an issue per se, but might be a bit confusing.