Alsa midi input support

Moderators: MattKingUSA, khz, muldjord, deva

corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Alsa midi input support

Post by corrados »

For learning the code I tried to implement an alsamidi input. I am pretty sure I did a lot of things not "drumgizmo standard-conform" but maybe someone can review my code and give me some feedback. I wanted to have alsamidi to avoid having to run a jack server. Here is my patchfile: https://drive.google.com/file/d/1mmzpjW ... sp=sharing. The patch is against the develop branch.
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

Thank you very much :-)

This particular feature has been on the todo list for years, but keeps on getting bumped down, so perhaps now it can finally make the cut?

The patch you linked only contains the changes to the configure.ac file.
If you use git for creating the patch you can stage and commit all your changes to one commit and then use the

Code: Select all

git format-patch -1
command to create the patch.

If you have more than one commit in your changeset just use the corresponding -<n> number instead of -1
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

I have used

git diff develop corrados_alsamidi > ../patchfile

to generate the patch file. Looking at the patch file, it seems everything is contained, e.g.,

new file mode 100644
index 0000000..a2cbb49
--- /dev/null
+++ b/drumgizmo/input/alsamidi.cc
@@ -0,0 +1,109 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/***************************************************************************
+ * alsamidi.cc

I am usually not using patch files but the Github style with forks and pull requests. Is there something similar available for the Drumgizmo project?

Please note that my code is really a quick and dirty hack. I just took the existing jackmidi file and adapted it with the help of http://fundamental-code.com/midi. So, there is plenty of room for improvements...
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

Sorry - I clearly see the content now. I must have been very tired when I first looked at the patch ;)

Apart from some indentation stuff the code looks good to me.
What is missing is the addition of the input module "alsamidi" in drumgizmo/drumgizmoc.cc and then transferral of command-line arguments, such as interface name, to the actual midi input class.

If you want to do those as well, you should be able to find inspiration from the way the "jackmidi" input module is handled.
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

What is missing is the addition of the input module "alsamidi" in drumgizmo/drumgizmoc.cc and then transferral of command-line arguments, such as interface name, to the actual midi input class.
I thought that I have already done it like jackmidi. If I use the cli with

./drumgizmo/drumgizmo -i alsamidi -I midimap=~/Schreibtisch/corrados_drumgizmo/midimap.xml -o alsa ~/Schreibtisch/corrados_drumgizmo/drumkit.xml

at least it seems to load my module and I can see the ALSA MIDI input if I query the interfaces with aconnect -o.
I think I need someone who points me to the corresponding place in the code...
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

Aah, yes, so you have. I was getting some apply errors due to some local changes that resulted in the patch being only partially applied - sorry about that ( I seem to be saying that alot today :p )

I have now created a branch with the changes called "alsamidi":
http://cgit.drumgizmo.org/drumgizmo.git ... 042a41b274
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

The branch passed the CI tests, and I think it is ready to be merged to the develop branch.
Is the author field as you would like it to be? or do you want your email address in there as well?
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

I just did some more testing with my code. Unfortunately, it does not work as expected. I have the following issues: ev->data.raw8.d[0] seems not to contain the correct 4 high bits. Also, ev->data.time.tick seems not to be the correct value. I have to spend some more time to get this fixed. I'll provide you with a new patch file as soon as I have fixed the outstanding issues. I'll also fix the tabs/spaces on that new patch then.
Is the author field as you would like it to be? or do you want your email address in there as well?
Maybe I'll add the link to my Github page, too, if that makes sense...
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

Here is an updated patch file: https://drive.google.com/file/d/1_PGuQ3 ... sp=sharing

Now my code is actually working. The issue with the previous code was that it was blocking in the run() function which is not allowed.

An open issue is the first byte of the raw8 of the ALSA sequencer. It seems the type information is missing. I now added the missing information for the first byte but this is kind of a hack. BTW: How are the controller messages processed? It seems that only note on/off/aftertouch are used.

Also an open issue is that we may want to check for the required header file asoundlib.h in the configure file. But since we already have alsa audio output, I guess this is not necessary.
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

I have now applied the patch to the branch.

In the code you use += to set the type bits, but wouldn't it be clearer to use |= ?

I think that the check for alsa, ie:

Code: Select all

     dnl ======================
     dnl Check for alsa library
     dnl ======================
     PKG_CHECK_MODULES(ALSA, alsa >= 1.0.18)
should be added in the same way as the alsa output code. Just in case a user decides to compile with alsamidi support but not alsa output.

It could be done in the same way as with the jack modules which are initially just setting a variable "need_jack" and then later in the configure script teh jack dependency is being polled if required.
corrados wrote: Thu Feb 04, 2021 5:10 pm BTW: How are the controller messages processed? It seems that only note on/off/aftertouch are used.
I have had a design discussion with chaot4 about it, and we may have a solution. So now we "just" need to write the code.
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

Thanks for your feedback. I actually thought about not to use the first byte at all but overwrite it (since we do not use the MIDI channel right now anyway) since we do not know what is really mapped to that byte by ALSA. I'll work on the modifications, soon.
BTW: Is it possible to get developer access to the Drumgizmo git repo? That would be much easier than to work with patches.
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

corrados wrote: Fri Feb 05, 2021 5:25 pm BTW: Is it possible to get developer access to the Drumgizmo git repo? That would be much easier than to work with patches.
Sure, you can just send me an email with your ssh public key, and I can grant you access :-)
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

Thank you for adding me to the git repo :-).
I have now done the outstanding work, i.e. fixing the first byte of the MIDI raw message and the check for the ALSA library. My changes are on the new corrados_alsamidi branch.
User avatar
deva
Established Member
Posts: 285
Joined: Sun Oct 23, 2016 10:15 am
Has thanked: 3 times
Been thanked: 31 times
Contact:

Re: Alsa midi input support

Post by deva »

Awesome - and Jenkins is happy: https://jenkins.drumgizmo.org/ :-)
the only thing I can currently think of would be if there are more midi types missing? Ideally the midi data should be "forwarded" raw to the interpretation function inside processNote(...) which is used across all input modules to make sure we only translate/handle midi data in one place in the code.
Is the new (not yet integrated) position CC events supported in the current way of reading the events for example?
corrados
Established Member
Posts: 39
Joined: Sat Jan 02, 2021 4:01 pm

Re: Alsa midi input support

Post by corrados »

Ideally the midi data should be "forwarded" raw to the interpretation function inside processNote(...) which is used across all input modules to make sure we only translate/handle midi data in one place in the code.
I may have used the wrong ALSA MIDI interface. I just found that there is a snd_rawmidi_open function which sounds pretty much like what I need. I'll investigate this now...
Post Reply