Alsa midi input support
Moderators: MattKingUSA, khz, muldjord, deva
Alsa midi input support
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.
- 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
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
command to create the patch.
If you have more than one commit in your changeset just use the corresponding -<n> number instead of -1
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
If you have more than one commit in your changeset just use the corresponding -<n> number instead of -1
Re: Alsa midi input support
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...
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...
- 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
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.
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.
Re: Alsa midi input support
I thought that I have already done it like jackmidi. If I use the cli withWhat 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.
./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...
- 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
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
I have now created a branch with the changes called "alsamidi":
http://cgit.drumgizmo.org/drumgizmo.git ... 042a41b274
- 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
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?
Is the author field as you would like it to be? or do you want your email address in there as well?
Re: Alsa midi input support
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.
Maybe I'll add the link to my Github page, too, if that makes sense...Is the author field as you would like it to be? or do you want your email address in there as well?
Re: Alsa midi input support
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.
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.
- 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
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:
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.
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)
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.
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.
Re: Alsa midi input support
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.
BTW: Is it possible to get developer access to the Drumgizmo git repo? That would be much easier than to work with patches.
- 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
Sure, you can just send me an email with your ssh public key, and I can grant you access
Re: Alsa midi input support
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.
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.
- 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
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?
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?
Re: Alsa midi input support
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...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.