tebjan / Sanford.Multimedia.Midi

Full sources of the famous C# MIDI toolkit on CodeProject by Leslie Sanford
http://www.codeproject.com/Articles/6228/C-MIDI-Toolkit
MIT License
169 stars 62 forks source link

'FatalExecutionEngineError receiving a sysex message "F0 00 01 00 03 01 01 00 05 F7" #4

Closed ghost closed 7 years ago

ghost commented 8 years ago

I get this exception when running C# VS2015, .NET 4.0 (Win 7 support). I downloaded and built the lib and also just tried NuGet, both have the same problem.

If I run the App outside VS using either Release or Debug builds everything is working?

InpuDevice.Messaging.cs - PtrToStrucure line:

 private void HandleSysExMessage(object state)
        {
            lock(lockObject)
            {
                IntPtr headerPtr = (IntPtr)state;

                MidiHeader header = (MidiHeader)Marshal.PtrToStructure(headerPtr, typeof(MidiHeader));

Install-Package Sanford.Multimedia.Midi

PM> Install-Package Sanford.Multimedia.Midi Attempting to gather dependency information for package 'Sanford.Multimedia.Midi.6.1.1' with respect to project 'MKConnectUI', targeting '.NETFramework,Version=v4.0' Attempting to resolve dependencies for package 'Sanford.Multimedia.Midi.6.1.1' with DependencyBehavior 'Lowest' Resolving actions to install package 'Sanford.Multimedia.Midi.6.1.1' Resolved actions to install package 'Sanford.Multimedia.Midi.6.1.1' Adding package 'Sanford.Multimedia.Midi.6.1.1' to folder '\vmware-host\Shared Folders\Desktop\MKConnect2 - Copy\packages' Added package 'Sanford.Multimedia.Midi.6.1.1' to folder '\vmware-host\Shared Folders\Desktop\MKConnect2 - Copy\packages' Added package 'Sanford.Multimedia.Midi.6.1.1' to 'packages.config' Successfully installed 'Sanford.Multimedia.Midi 6.1.1' to MKConnectUI PM>

ghost commented 8 years ago

The only .NET I can get this working with is 3.5. $ and up are not working with the error about MIDI DOT NET also seems to have a similar problem. No C# expert here.

tebjan commented 8 years ago

does your app receive proper sysex data? the exception sounds like there is no actual midi header at the pointer location. you can check the midi data with something like MIDI-OX...

tebjan commented 8 years ago

had a quick look at the code... MidiHeader.cs might be the problem. The fields in the struct may not match the native types in MIDIHDR: https://msdn.microsoft.com/en-us/library/windows/desktop/dd798449(v=vs.85).aspx

you could try to change all DWORD_PTR to IntPtr... that might fix it. not sure about the first one with LPSTR... would be interesting whether that is the problem.

ghost commented 8 years ago

That makes all kinds of sense!

uckuper commented 8 years ago

I am experiencing this same issue, but I don't think it is the Midi header that is the issue. It seems to be somehow triggering HandleSysxMessage when there is no message at all after doing a inDevice Reset(), so there must be some other thing that is maybe corrupting some callback pointer. I used MIDI OX to verify there are not Sysx messages being sent. Here's the development of this problem. Maybe it will shed light on what is the cause.

I was using VS 2013 for the project using the 5.1 library, and it started exhibiting this issue after installing Visual Studio 2015. Nothing else had changed so I could not find a cause for the issue. Debugging the issue did not make any sense because it kept executing HandleSysxMessage (in InputDevice.Messaging.cs) when there was no such message. So I started running the project in 2015 and the issue went away. Everything was running fine. I figured it was VS2013 getting corrupted after the VS 2015 installation. Now after installing Windows 10, I am seeing this same problem. The compiled exe will also produce the error when not running in the VS debugger. I just upgraded to the latest version of the Midi library (6.1) hoping it would correct the problem but it did not.

There is definitely something funky going on because of the following.

The problem occurs after closing an input device. Once it hits the Monitor.Wait(lockObject); in the Reset() function something is making it think it needs to handle a SysxMessage received and it goes to HandleSysExMessage(object state) where it fails. Yet there should not be ANY sysx messages coming from anywhere. When the reset process hits Monitor.Wait(lockObject); at line 170 at InputDevice.PublicMethods,cs, HandleSysExMessage executes for some reason and then it bombs at MidiHeader header = (MidiHeader)Marshal.PtrToStructure(headerPtr, typeof(MidiHeader));.

Any insight into this would be much appreciated. Thanks.

uckuper commented 8 years ago

Just one more point just in case. I also reinstalled VS 2015 from scratch after the Windows 10 installation to see if something had corrupted the CLR in the process. No luck,

tebjan commented 8 years ago

ok it seems that the code should not be called at all then... hard to find out what went wrong. but i will try to debug it.

uckuper commented 8 years ago

I will try to help Tebjan. Since I can duplicate it reliably, I will see if I can at least shed more light on the issue. Thanks!!

uckuper commented 8 years ago

Hi. I tried to debug this further. I did not have enough time to get in too deep, but I can characterize the problem a bit better. The issue happens on an inDevice.Close() call after the device has been stopped. Close() calls Reset() which in turn calls midiInReset(IntPtr handle) in winmm.dll. At this point, the sequence of events is suspicious and as follows.

The call to midiInReset in InputDevice.PublicMethods.cs blocks and triggers 5 successful calls to HandleMessage() while still blocking. The first comes in with msg == MIM_OPEN, which is a no-op. The next 4 calls come in with message == MIM_LONGDATA (964). These in turn then do a callback to HandleSysExMessage, and they succeed. Strangely these all come in on the Main thread! It must be noted here that there are no REAL sysEx messages occurring. When this used to work fine, I could verify these 4 calls did not happen after the midiInReset(). (After it was fixed by moving from VS2013 to VS2015, which worked the first time).

Only after the 5 calls to HandleMessage happen, the call to midiInReset unblocks and resumes on the next lines in InputDeviceMessaging.cs hitting line 170 Monitor.Wait(lockObject); As soon as this line is executed, a 6th call to HandleMessage happens, calling HandleSysxMessage, which then bombs as previously described at line 152 (InputDevice.Messaging,cs - MidiHeader header = (MidiHeader)Marshal.PtrToStructure(headerPtr, typeof(MidiHeader)); )

Without going into disassembly of the DLL I cannot see what midiInReset is doing to trigger those extra calls to HandleMessage with msg == 964. And certainly it seems that the last call that bombs is happening after the device reset has completed, so it should not happen at all.

The only way I can see this making sense at this point is that there is some pointer that seems to get corrupted that is wrongly calling HandleMessage with bad data. I reviewed the code to see if I could find a mismatch between pointer sizes for x86 vs 64-bit or something of that nature, but did not get anywhere. Maybe this will help you take a closer look.

Thanks!!

tebjan commented 8 years ago

thanks, this will help a lot, hope i find time for it in the near future...

ghost commented 8 years ago

Looks like you just got blown-off, open-source circle-jerking.

If you have a job to do. Everything builds and linked with .NET 3.5 will work fine on Win 7,8,10 and VS 2015. My App is shipped, tested and paid for, super happy customer.

Anything above 3.5 and shit becomes very unstable. Mostly I'm interested only in sysex so you see me filing this bug report. I expected no response due to the antique nature of the codebase, I just try to be professional. I also noticed connection problems on input devices but of

I had to consider what I was going to do if I couldn't get this going. So I took a look at Midi Dot Net, much simpler lib, ONLY other C# lib, with the idea of extending this to handle Midi. Well well well, same problems :)

So in summary it might be easier to get to the bottom of this with midi dot net c# lib extending it to receive sys ex rather than wade thru this codebase

Good luck

On Jul 28, 2016, at 10:45 AM, uckuper notifications@github.com wrote:

Hi. I tried to debug this further. I did not have enough time to get in too deep, but I can characterize the problem a bit better. The issue happens on an inDevice.Close() call after the device has been stopped. Close() calls Reset() which in turn calls midiInReset(IntPtr handle) in winmm.dll. At this point, the sequence of events is suspicious and as follows.

The call to midiInReset in InputDevice.PublicMethods.cs blocks and triggers 5 successful calls to HandleMessage() while still blocking. The first comes in with msg == MIM_OPEN, which is a no-op. The next 4 calls come in with message == MIM_LONGDATA (964). These in turn then do a callback to HandleSysExMessage, and they succeed. Strangely these all come in on the Main thread! It must be noted here that there are no REAL sysEx messages occurring. When this used to work fine, I could verify these 4 calls did not happen after the midiInReset(). (After it was fixed by moving from VS2013 to VS2015, which worked the first time).

Only after the 5 calls to HandleMessage happen, the call to midiInReset unblocks and resumes on the next lines in InputDeviceMessaging.cs hitting line 170 Monitor.Wait(lockObject); As soon as this line is executed, a 6th call to HandleMessage happens, calling HandleSysxMessage, which then bombs as previously described at line 152 (InputDevice.Messaging,cs - MidiHeader header = (MidiHeader)Marshal.PtrToStructure(headerPtr, typeof(MidiHeader)); )

Without going into disassembly of the DLL I cannot see what midiInReset is doing to trigger those extra calls to HandleMessage with msg == 964. And certainly it seems that the last call that bombs is happening after the device reset has completed, so it should not happen at all.

The only way I can see this making sense at this point is that there is some pointer that seems to get corrupted that is wrongly calling HandleMessage with bad data. I reviewed the code to see if I could find a mismatch between pointer sizes for x86 vs 64-bit or something of that nature, but did not get anywhere. Maybe this will help you take a closer look.

Thanks!!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

uckuper commented 8 years ago

Hi Tebjan. I found the cause of the problem and fixed it.

Basically it boiled down to the fact that winmm.dll in a 64-bit system must handle the larger pointers, both for callbacks and pointers to structures, and it does. So in InputDevice.Messaging.cs, private void HandleMessage(...) must be changed to HandleMessage(IntPtr hnd, int msg, long instance, long param1, uint param2) because param1 passes the pointer to the MidiHeader structure, and using an int truncated the pointer to a bad pointer. Same goes for the "instance" parameter.

Accordingly, the declaration of the delegate signature in InputDevice.Win32.cs must be changed to match.

Other changes were necessary like when posting to the delegateQueue, converting to IntPtr also caused the pointer to be truncated. So I changed the calls to read as delegateQueue.Post(HandleSysExMessage, param1); (no recasting of param1) passing the long value as the pointer directly.

Because param1 is also used to pass ints and bytes for other things, the functions using them must also be changed to do the recasting properly.

I would be happy to do a pull request and update the project with my changes or I could send you the changed files (3 total) for you to update. Let me know how you'd like to handle it.

uckuper commented 8 years ago

ClayApps. You may be a little harsh? Actually I have been round and round picking a MIDI library for .NET, and after lots of iterations I conclude this is THE best, most complete and mature library for MIDI that is compatible with older systems and Windows 10. It is really good. If you look at the threading design and other goodies like timers and players and so forth, this does a superb job. It does handle SysEx messages (all messages in fact).

The reason it worked for you (as it did for me) before is that you had built the project as a 32-bit app. With Windows 8 and 10 pointers are all 64 bit so when you build for all environments the bug I just fixed surfaces. Simple problem though... As soon as I or Tebjan incorporate the fix your stuff will work fine.

I actually thank Leslie Sanford infinitely for developing and maintaining this great library for so long and making it available. He has a right to retire from it! He's earned it. And so I also must thank Tebjan for taking on the responsibility to continue to maintain this code.

Now for the future, the Windows 10 SDK has a great new .NET library for MIDI without having to resort to the old WinMM library. Problem is, it only works on Universal Windows environments, so if you develop with this library your programs won't run in older Windows like Windows 7. The good thing is that they will be Microsoft Phone compatible, Surface compatible, etc. if this is a factor for your app.

uckuper commented 8 years ago

Tebjan. One afterthought. I don't know if these changes will work in a 32-bit build. We will need to test that.

tebjan commented 8 years ago

so user IntPtr everywhere, it is different on 64 and 32 bit builds. thats basically why it exists. or are you suggesting that even if you build for 32-bit winmm.dll will work with long?

uckuper commented 8 years ago

Actually, I have changed all the values that can contain pointers to IntPtr and then do the conversion downstream back to Int32 where needed (ToInt32) such as: delegateQueue.Post(HandleShortMessage, param1.ToInt32()); Notice this will throw an exception if param1 is larger than Int32, but if that happens, then something else is wrong and an error is appropriate anyway.

It did work with long instead of InPtr, but that would not work on 32-bit environment I don't think.

in InputDevice.Win32.cs: private delegate void MidiInProc(IntPtr handle, int msg, IntPtr instance, IntPtr param1, IntPtr param2); (matches the documented declaration)

[DllImport("winmm.dll")] private static extern int midiInOpen(out IntPtr handle, int deviceID, MidiInProc proc, IntPtr instance, int flags);

And this required some changes in InputDevice.Construction.cs: int result = midiInOpen(out handle, deviceID, midiInProc, System.IntPtr.Zero, CALLBACK_FUNCTION);

And in InputDevice.Messaging.cs: private void HandleMessage(IntPtr hnd, int msg, IntPtr instance, IntPtr param1, IntPtr param2) { delegateQueue.Post(HandleRawMessage, param1.ToInt64()); ..... }

and private void HandleRawMessage(object state) { long param = (long)state; OnRawMessage(new RawMessageEventArgs(unchecked((int)param))); }

This last thingy is because RawMessageEventArgs expects an Int32, but some of these will pass a 64-bit pointer, so it needs to be truncated. In this case, the value is not used anyway.

I can send you the files if you want.

mweetman2 commented 8 years ago

Hi I have been trying to debug this issue for a while. uckuper - could you send me the files & I'll do some testing. I couldn't work out why the HandleSysExMessage routine was even being called, as no SysEx was received. Thanks Martin

uckuper commented 8 years ago

Hey Martin. I'll be glad to upload the files. I have it working. The trick is in taking care that the COM structure pointer parameters are the correct size and converted appropriately to int or long when used as values. Give me a couple of days and I will do a fork, update the files and do a pull request.

I am not sure why the SysEx messages are being received. In the end, when handled appropriately they work just fine. The only thing I can think of is that some of the hardware drivers send some type of SysEx when initialized, but I have not bothered (yet) to try and figure out what they are saying in the message. I will hopefully have an answer by the time I check in my changes.

mweetman2 commented 8 years ago

Thanks a lot. I have spent days on this, so was so glad to see your post. I'll check out the SysEx messages when I get the code. The only thing I could think of is that they were used as a signaling channel from the driver. Will be interesting to find out. Thanks again. Martin Sent from my BlackBerry® wireless device

-----Original Message----- From: uckuper notifications@github.com Date: Thu, 18 Aug 2016 07:51:38 To: tebjan/Sanford Multimedia MidiSanford.Multimedia.Midi@noreply.github.com Reply-To: tebjan/Sanford.Multimedia.Midi reply@reply.github.com Cc: mweetman2martin@weetman.com; Manualmanual@noreply.github.com Subject: Re: [tebjan/Sanford.Multimedia.Midi] 'FatalExecutionEngineError receiving a sysex message "F0 00 01 00 03 01 01 00 05 F7" (#4)

Hey Martin. I'll be glad to upload the files. I have it working. The trick is in taking care that the COM structure pointer parameters are the correct size and converted appropriately to int or long when used as values. Give me a couple of days and I will do a fork, update the files and do a pull request.

I am not sure why the SysEx messages are being received. In the end, when handled appropriately they work just fine. The only thing I can think of is that some of the hardware drivers send some type of SysEx when initialized, but I have not bothered (yet) to try and figure out what they are saying in the message. I will hopefully have an answer by the time I check in my changes.

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/tebjan/Sanford.Multimedia.Midi/issues/4#issuecomment-240747737

uckuper commented 8 years ago

Hey Martin. I just uploaded my changes to my fork (uckuper/sanford.multimedia.midi) and created a pull request (Tebjan - please take a look and update). The relevant changes are in 3 files (attached). The other change is just the addition of a comment.

uckuper changes.zip

mweetman2 commented 8 years ago

Great. Thanks so much. I'm traveling at moment, but have my PC with me, but no MIDI till I get back. Will test thoroughly then as I have an application already developed. Martin Sent from my BlackBerry® wireless device

-----Original Message----- From: uckuper notifications@github.com Date: Thu, 18 Aug 2016 09:58:04 To: tebjan/Sanford Multimedia MidiSanford.Multimedia.Midi@noreply.github.com Reply-To: tebjan/Sanford.Multimedia.Midi reply@reply.github.com Cc: mweetman2martin@weetman.com; Manualmanual@noreply.github.com Subject: Re: [tebjan/Sanford.Multimedia.Midi] 'FatalExecutionEngineError receiving a sysex message "F0 00 01 00 03 01 01 00 05 F7" (#4)

Hey Martin. I just uploaded my changes to my fork (uckuper/sanford.multimedia.midi) and created a pull request (Tebjan - please take a look and update). The relevant changes are in 3 files (attached). The other change is just the addition of a comment.

uckuper changes.zip

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/tebjan/Sanford.Multimedia.Midi/issues/4#issuecomment-240787425

mweetman2 commented 8 years ago

Hi

Thanks again for the fix. This worked perfectly.

I took a look at the mystery SysEx messages & they seem to be messages containing all zeros. Looks like he may be flushing some buffers used by the driver.

Will do some further testing once I have the MIDI TX side working.

I just worked out the MIDI output device #0 must be an internal Windows “MIDI” port. Need to set the outDeviceID to 1 to send to the first external MIDI port ;-). Couldn’t work out why no MIDI was being generated by my application...

Thanks yet again. You are a STAR!

Regards

Martin Weetman

Phone +44 7798 903766

http://www.weetman.com

From: uckuper [mailto:notifications@github.com] Sent: 18 August 2016 17:58 To: tebjan/Sanford.Multimedia.Midi Cc: mweetman2; Manual Subject: Re: [tebjan/Sanford.Multimedia.Midi] 'FatalExecutionEngineError receiving a sysex message "F0 00 01 00 03 01 01 00 05 F7" (#4)

Hey Martin. I just uploaded my changes to my fork (uckuper/sanford.multimedia.midi) and created a pull request (Tebjan - please take a look and update). The relevant changes are in 3 files (attached). The other change is just the addition of a comment.

uckuper changes.zip https://github.com/tebjan/Sanford.Multimedia.Midi/files/425452/uckuper.changes.zip

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tebjan/Sanford.Multimedia.Midi/issues/4#issuecomment-240787425 , or mute the thread https://github.com/notifications/unsubscribe-auth/ALpxFzPeOR_f3ENYWw1UigMWshbXZzjfks5qhI8cgaJpZM4I8XOL . https://github.com/notifications/beacon/ALpxF85ZP86ZoUYDHYe4bpfEje-BY_abks5qhI8cgaJpZM4I8XOL.gif

uckuper commented 8 years ago

Hi Martin,

Glad to hear! I think you're right that the bogus message seems to be generated by flushing the buffers. Maybe one of these days I will have time to take a look at that.

Regarding the port numbers, unless you are writing an application that is dedicated to your own environment, be very cautious how you use those port numbers. What I found out is that, depending on the order of discovery by the system, these ports will come up with different numbers at different times. So for example, if you have a USB MIDI device that you are used to finding on port #3, but next time you bring your computer up you forgot to plug it in, so you go ahead and plug it in later, this time it could come up as the highest number (total ports - 1) instead of #3, and you will have another port in its stead at 3. I got bitten by this issue big time. It is best to identify the port by its unique name or signature. I think the new Windows Core classes that support MIDI give you some sort of unique identifier for each port. Unfortunately I don't think these Core classes are backward compatible so we are stuck with win32.

Would love to talk to you, but apparently you reside in the UK and I in the States (long distance call). Maybe we can exchange email addresses somehow without publishing them online. You can find me on LinkedIn (Carl Kupersmit)

Cheers.

afdeprado commented 8 years ago

I found several additional P/Invoke signature errors on the source, and have posted a solution as a pull request. Two of the errors, in particular, affect the possibility to retrieve a MIDI in/MIDI out interface description (the code as is will provide bogus data on a 64-bit system, as the call to midiInGetDevCaps() / midiOutGetDevCaps() wasn't getting the correct parameters).

uckuper commented 8 years ago

Hey Andres. Thank you. I have not noticed that error on my end, and I am working in a 64-bit environment, but great that you found it. Can you provide more detail on what data is bogus on the calls to midiInGetDevCaps() / midiOutGetDevCaps() so I can just check why I may not be experiencing any problems? I do use that method quite a bit. Thanks!

afdeprado commented 8 years ago

Hey, uckuper. The midi*GetDevCaps() call takes as its first parameter (deviceID) a handle, which is 32 or 64 bits depending on your system. This is because you can either pass the ordinal number of the device (deviceID) or the Handle to the device you want the data for. If you pass a 32-bit number on a 64-bit system, you might get stack corruption depending on the calling convention (whether parameters are passed in registers or on the stack).

If the calling convention is to pass the first parameters in registers, it will appear that everything is fine (but since you are writing just 32 bits instead of 64 bits, the number you pass might not be the number received by the procedure, as the high bits will not necessarily be cleared). If the calling convention is to pass all parameters in the stack, you will get in serious problems...

Windows system calls use typically stdcall, where the first parameters (up to the first 3, depending on type) are passed in registers and the rest in the stack, therefore, you won't see a problem most of the time, but it's a bug waiting to happen...

uckuper commented 8 years ago

Thank you for the detailed explanation. I did not even notice. Good catch.

webgal commented 8 years ago

Hey @tebjan, just wondering if you had time to consider accepting the pull request from @afdeprado ?

This could most certainly help kickstart things nicely for the future of this great library :)

Cheers!

stashymane commented 7 years ago

This error is also thrown to me upon disposing of an InputDevice, but only in x64.

uckuper commented 7 years ago

Hey StashCat. If you put in the fixes that afdeprado made in his fork you should be OK. Also check the flow with Dispose. There seems to be a bit of a problem there in that it calls other methods like Reset that can throw exceptions. You may want to modify them or use Close (which calls dispose I believe) instead of Dispose.

tebjan commented 7 years ago

i have merged the pull request from @afdeprado