search

brucelane / Cinder-MIDI2

Alternative approach to interfacing RtMidi lib in Cinder
21 stars 10 forks source link

Proposed solution to platform recognition #12

Closed felixfaire closed 7 years ago

felixfaire commented 7 years ago

Hey.

I don't know if this is the best header structure but I cleaned up a lot of the includes to just use MidiHeader.h where needed as there was lots of duplication. This has also added back the automatic platform recognition (tested on OSX at least) and the sample builds out of the box (which also looks a bit better).

This also switches to cinder/Signals instead of boost/signals2 (so it builds out of the box with the latest cinder branch which doesnt include boost/signals2). HOWEVER, whilst the midi functionality seems to work fine with some minimal tests, the midi messages often come from a separate thread so we may need to investigate how best to handle this with cinder/Signals. And what the worst case scenario would be for race conditions at this point..

Let me know what you think.

brucelane commented 7 years ago

that was quick! I'm going to try it asap

brucelane commented 7 years ago

I didn't dare modifying RtMidi.h to keep it separate, you did it, so I follow!

felixfaire commented 7 years ago

Haha. yes I didnt feel good about that (and i'm sure there is a better way) but it is a fairly minimal intervention and greatly increases the usability of the block so I went ahead... It would be good to get some more experienced eyes on it for sure.

brucelane commented 7 years ago

I agree! +1

brucelane commented 7 years ago

it work fine on Windows (needs some more tests)

brucelane commented 7 years ago

well, only if I declare __WINDOWS_MM__ in the project... CINDER_MSW is not recognized in RtMidi.h Is it working on Mac?

felixfaire commented 7 years ago

Hmm, thats odd. Yeah it is working great on mac. I'm sure we can work it out.

Also I have another little PR coming soon to address some of the threading stuff.