Opened 5 years ago

Closed 19 months ago

Last modified 6 months ago

#6579 closed defect (fixed)

OS X - only first CoreMIDI endpoint used

Reported by: SF/dominus Owned by: criezy
Priority: normal Component: Port: Mac OS X
Keywords: Cc:
Game:

Description

Hi,

when you have more than one CoreMIDI device "attached" (software devices like the Munt QT Gui count as well) only the first one gets used by ScummVM.

An example is having a real MT32 device attached via USB adapters and running Munt's QT gui (http://sourceforge.net/projects/munt/files/munt/1.3.0/mt32emu-qt-1.1.0.dmg/download - you will need the MT32 ROMs) (or other MIDI devices via USB).
ScummVM will only use the first device (first CoreMIDI endpoint).

DOSBox "recently" added code to detect the other endpoints, enumerate and get their names and then use the endpoint number in the cfg file.

Adapting this to ScummVM one could make ScummVM grab the names as well and make them selectable in the Audio and MIDI tabs of the ScummVM GUI.
So if it is more than one endpoint, don't list CoreMIDI but
"CoreMIDI - endpointname1"
"CoreMIDI - endpointname2"
"CoreMIDI - endpointname3"

This will probably end up as a feature request, though :)

Ticket imported from: bugs/6579.

Attachments (1)

0001-Implement-mixer-listmidi-for-coremidi.-Thanks-for-th.patch (2.1 KB) - added by digitall 5 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 5 years ago by SF/dominus

sorry, meant to post the DOSBox revision but forgot due to SF eating my first try at reporting because I wasn't logged in...

So, the DOSBox revision is 3838, the diff https://sourceforge.net/p/dosbox/code-0/3838

comment:2 Changed 5 years ago by digitall

Dominik: Thank you for the bug report. Our CoreMidi code can be found here and already has a TODO to deal with this:
https://github.com/scummvm/scummvm/blob/master/backends/midi/coremidi.cpp#L203

The required changes are likely to be around line 93 there, where only endpoint 0 is used currently:
https://github.com/scummvm/scummvm/blob/master/backends/midi/coremidi.cpp#L93

As this is a relatively simple code change to add, I'll take a look at making the required changes, but I do not have an OSX box to test on and thus will need to make the required changes, then make a build for one of the OSX using developers to test with...

If you know C++ and are capable, then a Pull Request on Github of a code patch for this from you would likely be faster... otherwise, this may take me some time. Thanks.

comment:3 Changed 5 years ago by digitall

Have attached the dosbox patch for reference.

comment:4 Changed 5 years ago by digitall

Sigh... Sorry, sf.net is malfunctioning this evening. Will sort in a sec.

comment:5 Changed 5 years ago by digitall

Here we are.

comment:6 Changed 5 years ago by digitall

Right. Should have a working solution for this. If you can compile yourself, try this:
https://github.com/digitall/scummvm/tree/coremidiDeviceSelection

The code makes it clear what the changes are:
https://github.com/digitall/scummvm/commit/3c02272d71441fd0c510b8d75fdc221e0c3b6e66

Please let me know about any issues.

I will look at compiling a "special" build for you to test with, in case you can not compile yourself... which I admit can be an issue on OSX, especially if you try to use XCode, rather than "bare" ./configure && make in the command line.

comment:7 Changed 5 years ago by SF/dominus

Hey, wow, great, didn't expect something so soon. I'm good at finding things and connecting it but bad at actually coding ;)
I'll give compiling a try this evening. Last time I managed to compile ScummVM was when it was still on SVN, so I should be able to figure it out again ;)
I'll report back then (got two usb adapters and the munt software device, so I should have some test cases).

comment:8 Changed 5 years ago by digitall

It should be fairly easy. See the following for help:
http://wiki.scummvm.org/index.php/Developer_Central
http://wiki.scummvm.org/index.php/Git#Retrieving_the_code
http://wiki.scummvm.org/index.php/Compiling_ScummVM

You will need to do something like this:
#!/bin/sh
# Getting the code
git clone https://github.com/digitall/scummvm.git scummvm-digitall
cd scummvm-digitall
# switch to the correct branch
git checkout coremidiDeviceSelection
# build the codebase... for help, call "./configure --help"
./configure && make clean && make

comment:9 Changed 5 years ago by digitall

Have just compiled a test build on our buildbot for OSX Intel. This required a couple of minor fixes which I have pushed to that branch. Will put a tarball up for this test build in a bit.

Unfortunately, this code does not compile cleanly for OSX PPC, which blocks committing this to the scummvm master repository as-is.

It appears that the PPC SDK API for 10.2 to 10.5 is missing a symbol used here:
../scummvm/backends/midi/coremidi.cpp: In member function 'virtual int MidiDriver_CoreMIDI::open()':
../scummvm/backends/midi/coremidi.cpp:103: error: 'kMIDIPropertyDisplayName' was not declared in this scope

Will need to take a look at the earlier coremidi API and see if we can deal with this.

comment:10 Changed 5 years ago by digitall

OK. Precompiled test build for OSX Intel is here:
http://buildbot.scummvm.org/snapshots/other/osx_intel-coremidiDeviceSelection-9442b62e.tar.xz

As this is a basic improvement, it lacks full GUI integration. You will need to run as "./scummvm -d 1" from a terminal window to see the debug output. This now includes a list of detected CoreMIDI destinations with their id numbers when the CoreMIDI device is initialised. Run it once to get this list. You may need to start a game with MIDI to get this.

Once you have this, set the required output id number in the ScummVM configuration file found at ~/Library/Preferences/ScummVM Preferences.

You will have to add the following to the [global] section, replacing 0 with the correct number for the device you wish to use:
coremidi_device=0

Then restart ScummVM and this should then work. Note that removing or adding devices will change the id numbering and thus require adjustment of the config file setting.

comment:11 Changed 5 years ago by digitall

Ah, I see. kMIDIPropertyDisplayName was only added in OSX 10.4 API. For earlier SDKs, we need to use the code shown here:
https://developer.apple.com/library/mac/qa/qa1374/_index.html

comment:12 Changed 5 years ago by SF/dominus

Ah, yes, probably needs a similar "hack" for recognizing OS X version than what has been done to the coreaudio soundfont selection.

Thanks for the precompiled version, on my first test I didn't get to load a midi game and without that I was stuck on coreaudio ;)
As soon as I don't have an unhappy baby in my arms I'll test further.

comment:13 Changed 5 years ago by SF/dominus

ok, finally found time to test and it works nicely. The only difference to your instructions was that I needed it to put the coremidi_device=0 line to the [scummvm] section not [global] (though that might be an OS X specialty, I don't know).

Regarding your comment in the code, IMO the device should be selectable both by number or name, in case someone has two of the same cable attached (I don't know whether CoreMIDI adds a number to the names of devices that share a name).

Thanks for taking this on and solving it so quickly.

comment:14 Changed 5 years ago by SF/dominus

just noticed that error handling is also not there, in cases one has a device number set which is no longer present.
It should default to 0 ideally, right now it crashes with:
../scummvm/backends/midi/coremidi.cpp:147: failed assertion `isOpen()'

comment:15 Changed 5 years ago by digitall

Ah glad it works. Yes, sorry, I did mean [scummvm], not [global]. I did not check and had forgotten that the "global" section in the configuration file is called [scummvm].

It should not be necessary to select by name or id. kMIDIPropertyDisplayName should be a unique value as should the earlier Device-Entity-Endpoint combination.

Hmm... that shouldn't happen as I did put in a sanity check, but I did say this was a test version to check the basic code change, rather than a fully finished solution. Will look and see why this occurs and as you say, make it default back to 0, rather than crash.

comment:16 Changed 5 years ago by digitall

OK. Have updated my code in that branch. It should now have better sanity checking and I have implemented the interface code for the GUI, so that you can choose the endpoint using the Options->Audio/MIDI/MT32 selections.

The device will be automatically saved by the GUI code, but if it fails to exist later, then the device will fallback to endpoint 0 as before.

I have compiled a build for you to test this with:
http://buildbot.scummvm.org/snapshots/other/osx_intel-coremidiDeviceSelection-99bbb638.tar.xz

Please let me know how this works for you.

Note that this still needs the PPC build fallback code adding before I can get this merged to the scummvm development master branch.

comment:17 Changed 5 years ago by SF/dominus

Works nicely with the new code and choosing the device via the GUI works fine as well.
I noticed two things, though:
- When I open the GUI options, the devices get polled 7 times (terminal debug output shows that) and after selecting a CoreMidi device and applying the options, another two polls
- When you have a coremidi device set that is invalid you get a nice message about this and that ScummVM tries to play via another midi device and then the game starts but you never know which one it now chose. Probably Coreaudio since music plays but none of my CoreMidi devices play. This is not really fault of the CoreMidi code but a ScummVM "problem" (well music plays, that's important :))

Something to note in the code and perhaps documentation is that the first time the CoreMidi devices get polled you will experience some noticeable delay. Nothing one can change but might need to be documented.

see http://genekc07.stowers.org/Users/mec/Library/Developer/Shared/Documentation/DocSets/com.apple.adc.documentation.AppleLion.CoreReference.docset/Contents/Resources/Documents/#qa/qa1374/_index.html it looks as if there was even a change in between 10.2 and 10.3 SDK...

Maybe this new code to get the CoreMIDI endpoints should be limited to 10.4 SDK and higher.

comment:18 Changed 5 years ago by digitall

I can't fix the multiple polling without refactoring all the music/audio GUI code and that is a complex major task which has been open for some time:
http://wiki.scummvm.org/index.php/OpenTasks/Audio/MIDI_Device_Configuration
http://wiki.scummvm.org/index.php/OpenTasks/Audio/Audio_Output_Selection

Patches to fix this are welcome! :)

Apart from that, if this is merged, the GUI team can look at adding a "Please Wait" popup if there is likely to be a significant delay.

Yes, I will do that based on the code for this in the existing CoreAudio driver code which has a similar API difference. This may take a bit of time to sort.

comment:19 Changed 19 months ago by criezy

Owner: set to criezy
Resolution: fixed
Status: newclosed

I didn't see this ticket before, but another user reported this on the forum a few days ago and as a result this has now been fixed.

comment:20 Changed 6 months ago by digitall

Component: Port: Mac OS X
Note: See TracTickets for help on using tickets.