[alsa-devel] [PATCH] amidi: add delay option
This patch adds a new options to amidi tool to add a delay (in milliseconds) to each MIDI message.
This is very useful when sending firmware updates to a remote device via SysEx or any other use that requires this delay in between messages.
Signed-off-by: Felipe F. Tonello eu@felipetonello.com --- amidi/amidi.1 | 14 ++++++++++++ amidi/amidi.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 5 deletions(-)
diff --git a/amidi/amidi.1 b/amidi/amidi.1 index 1b4cfb1ea956..40402c17e374 100644 --- a/amidi/amidi.1 +++ b/amidi/amidi.1 @@ -111,6 +111,12 @@ to stop receiving data. Does not ignore Active Sensing bytes (FEh) when saving or printing received MIDI commands.
+.TP +.I -D, --delay=mseconds +Adds a delay in between each MIDI message sent to a device. It is +useful when sending firmware updates via SysEx messages to a remote +device. + .SH EXAMPLES
.TP @@ -121,6 +127,14 @@ to port .I hw:0.
.TP +.B amidi -p hw:1,0,0 -s firmware.syx -D 100 +will send the MIDI commands in +.I firmware.syx +to port +.I hw:1,0,0 +with 100 milliseconds delay in between MIDI messages. + +.TP .B amidi -S 'F0 43 10 4C 00 00 7E 00 F7' sends an XG Reset to the default port.
diff --git a/amidi/amidi.c b/amidi/amidi.c index cedf18c5fd39..f45abfa7eac0 100644 --- a/amidi/amidi.c +++ b/amidi/amidi.c @@ -48,6 +48,7 @@ static int receive_file; static int dump; static int timeout; static int stop; +static int delay; static snd_rawmidi_t *input, **inputp; static snd_rawmidi_t *output, **outputp;
@@ -77,7 +78,8 @@ static void usage(void) "-d, --dump print received data as hexadecimal bytes\n" "-t, --timeout=seconds exits when no data has been received\n" " for the specified duration\n" - "-a, --active-sensing don't ignore active sensing bytes\n"); + "-a, --active-sensing don't ignore active sensing bytes\n" + "-D, --delay=mseconds delay in between each sent MIDI message\n"); }
static void version(void) @@ -305,6 +307,35 @@ static void parse_data(void) send_data_length = i; }
+static int message_length(unsigned char byte) +{ + static int rstate_len = 0; + + switch (byte) { + case 0xf8 ... 0xff: + return 1; + case 0xf0: + return -1; /* variable size */ + case 0xf1: + case 0xf3: + return 2; + case 0xf2: + return 3; + case 0xf4: + case 0xf5: + return 0; /* ignore */ + case 0xf6: + return 1; + case 0xf7: + return 0; /* ignore */ + case 0x80 ... 0xef: + rstate_len = (byte >= 0xc0 && byte <= 0xdf) ? 2 : 3; + return rstate_len; + case 0x00 ... 0x7f: + return rstate_len ?: rstate_len - 1; + } +} + /* * prints MIDI commands, formatting them nicely */ @@ -406,7 +437,7 @@ static void add_send_hex_data(const char *str)
int main(int argc, char *argv[]) { - static const char short_options[] = "hVlLp:s:r:S::dt:a"; + static const char short_options[] = "hVlLp:s:r:S::dt:aD:"; static const struct option long_options[] = { {"help", 0, NULL, 'h'}, {"version", 0, NULL, 'V'}, @@ -419,6 +450,7 @@ int main(int argc, char *argv[]) {"dump", 0, NULL, 'd'}, {"timeout", 1, NULL, 't'}, {"active-sensing", 0, NULL, 'a'}, + {"delay", 1, NULL, 'D'}, { } }; int c, err, ok = 0; @@ -463,6 +495,9 @@ int main(int argc, char *argv[]) case 'a': ignore_active_sensing = 0; break; + case 'D': + delay = atoi(optarg); + break; default: error("Try `amidi --help' for more information."); return 1; @@ -538,9 +573,36 @@ int main(int argc, char *argv[]) error("cannot set blocking mode: %s", snd_strerror(err)); goto _exit; } - if ((err = snd_rawmidi_write(output, send_data, send_data_length)) < 0) { - error("cannot send data: %s", snd_strerror(err)); - goto _exit; + if (!delay) { + if ((err = snd_rawmidi_write(output, send_data, send_data_length)) < 0) { + error("cannot send data: %s", snd_strerror(err)); + return err; + } + } else { + unsigned char *data = (unsigned char *)send_data; + + while (data < ((unsigned char *)send_data + send_data_length)) { + + if (data > (unsigned char *)send_data) + usleep(delay * 1000); + + int len = message_length(*data); + + /* check for variable length */ + if (len == -1) { + int i = 0; + while (data[i++] != 0xf7); + len = i; + } + + if (len > 0 && + (err = snd_rawmidi_write(output, data, len)) < 0) { + error("cannot send data: %s", snd_strerror(err)); + return err; + } + + data += len ?: 1; + } } }
Felipe F. Tonello wrote:
This patch adds a new options to amidi tool to add a delay (in milliseconds) to each MIDI message.
This is very useful when sending firmware updates to a remote device via SysEx or any other use that requires this delay in between messages.
- case 0xf4:
- case 0xf5:
return 0; /* ignore */- case 0xf7:
return 0; /* ignore */
Silent data loss is bad.
- case 0x00 ... 0x7f:
return rstate_len ?: rstate_len - 1;
rstate_len - 1 == -1
int len = message_length(*data);/* check for variable length */if (len == -1) {int i = 0;while (data[i++] != 0xf7);
Possible buffer overflow.
if (len > 0 &&(err = snd_rawmidi_write(output, data, len)) < 0) {
Same here.
The most robust way to find message boundaries probably would be a state machine. However, there is actually no requirement to find all message boundaries; the _actual_ requirement is to insert pauses between SysEx messages. So don't bother to try to parse everything; just search for F0 or F7.
Regards, Clemens
Hi Clemens,
On 12/08/16 08:10, Clemens Ladisch wrote:
Felipe F. Tonello wrote:
This patch adds a new options to amidi tool to add a delay (in milliseconds) to each MIDI message.
This is very useful when sending firmware updates to a remote device via SysEx or any other use that requires this delay in between messages.
- case 0xf4:
- case 0xf5:
return 0; /* ignore */- case 0xf7:
return 0; /* ignore */Silent data loss is bad.
What do you mean by that?
- case 0x00 ... 0x7f:
return rstate_len ?: rstate_len - 1;rstate_len - 1 == -1
?
That's why there is this condition, if rstate_len is zero, than it returns 0.
int len = message_length(*data);/* check for variable length */if (len == -1) {int i = 0;while (data[i++] != 0xf7);Possible buffer overflow.
True. I could check while data + i < send_data + send_data_length.
if (len > 0 &&(err = snd_rawmidi_write(output, data, len)) < 0) {Same here.
This would be solved by previous boundaries check.
The most robust way to find message boundaries probably would be a state machine. However, there is actually no requirement to find all message boundaries; the _actual_ requirement is to insert pauses between SysEx messages. So don't bother to try to parse everything; just search for F0 or F7.
Well, I tough it would be useful to work in any type of MIDI message not just SysEx. So, do you think only SysEx then?
Felipe Ferreri Tonello wrote:
On 12/08/16 08:10, Clemens Ladisch wrote:
Felipe F. Tonello wrote:
- case 0xf4:
- case 0xf5:
return 0; /* ignore */- case 0xf7:
return 0; /* ignore */Silent data loss is bad.
What do you mean by that?
When you want to send the byte F5, this code will silently drop it.
- case 0x00 ... 0x7f:
return rstate_len ?: rstate_len - 1;rstate_len - 1 == -1
?
That's why there is this condition, if rstate_len is zero, than it returns 0.
There is not need to use rstate_len when its value is already known and does not affect the computation; it could just be written as:
return rstate_len ?: -1;
The most robust way to find message boundaries probably would be a state machine. However, there is actually no requirement to find all message boundaries; the _actual_ requirement is to insert pauses between SysEx messages. So don't bother to try to parse everything; just search for F0 or F7.
Well, I tough it would be useful to work in any type of MIDI message not just SysEx.
But there is no _actual_ use case, where somebody actually needed it. And as shown by all those problems, handling all types of messages makes the code too complex.
Regards, Clemens
Hi Clemens,
On 12/08/16 12:38, Clemens Ladisch wrote:
Felipe Ferreri Tonello wrote:
On 12/08/16 08:10, Clemens Ladisch wrote:
Felipe F. Tonello wrote:
- case 0xf4:
- case 0xf5:
return 0; /* ignore */- case 0xf7:
return 0; /* ignore */Silent data loss is bad.
What do you mean by that?
When you want to send the byte F5, this code will silently drop it.
I see your point, you are right. Although, as we know, the underlying driver won't send this byte anyway, but that is not for the application to decide.
- case 0x00 ... 0x7f:
return rstate_len ?: rstate_len - 1;rstate_len - 1 == -1
?
That's why there is this condition, if rstate_len is zero, than it returns 0.
There is not need to use rstate_len when its value is already known and does not affect the computation; it could just be written as:
return rstate_len ?: -1;
Right, that was not the original intent, so this code was actually wrong. But I will remove it anyway based on your comment below.
The most robust way to find message boundaries probably would be a state machine. However, there is actually no requirement to find all message boundaries; the _actual_ requirement is to insert pauses between SysEx messages. So don't bother to try to parse everything; just search for F0 or F7.
Well, I tough it would be useful to work in any type of MIDI message not just SysEx.
But there is no _actual_ use case, where somebody actually needed it. And as shown by all those problems, handling all types of messages makes the code too complex.
True. So I will make sure to add this to the documentation.
On Fri, 12 Aug 2016, Clemens Ladisch wrote:
Off topic here but: what happened to that patch you proposed (requested by me) to enable/disable filtering of MIDI Clock messages, just like already exists for MIDI Active Sensing messages?
participants (4)
-
Clemens Ladisch -
Felipe F. Tonello -
Felipe Ferreri Tonello
-
Martin Tarenskeen