Re: [v7 PATCH] amidi: add sysex-interval option

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Sep 13 2016 01:41, Felipe Ferreri Tonello wrote:
> ping?

Sorry to be late. I've already read this patch, then consider about 
whether this patch is for general purporse or not.

The basic design of 'amidi' tool is somehow generic purpose; 
send/receive MIDI messages to/from ALSA rawmidi devices. In my opinion, 
model-specific features is not so preferable to this tool.

The system exclusive messages are for the purpose to implement 
vendor-dependent something. So if this patch is quite unique for a small 
part of devices, I think it better to add isolated tools with this 
feature from amidi.

For my information and help of my review, please show which devices do 
you want to set up this tool?

Another my concern is a case that MIDI messages in a given file includes 
some messages except for system exclusives.

> On 30/08/16 17:02, Felipe F. Tonello wrote:
>> This patch adds a new option to amidi tool: sysex-interval.
>>
>> It adds a delay (in milliseconds) in between each SysEx message - it searches
>> for a 0xF7 byte.
>>
>> This is very useful when sending firmware updates to a remote device via SysEx
>> or any other use that requires this delay in between SysEx messages.
>>
>> `amidi' manual was updated with an example usage as well.
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>>  amidi/amidi.1 | 16 ++++++++++-
>>  amidi/amidi.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++------------
>>  2 files changed, 86 insertions(+), 18 deletions(-)
>>
>> diff --git a/amidi/amidi.1 b/amidi/amidi.1
>> index 86beb27a1e3b..5bc24ba3ad54 100644
>> --- a/amidi/amidi.1
>> +++ b/amidi/amidi.1
>> @@ -1,4 +1,4 @@
>> -.TH AMIDI 1 "16 Apr 2016"
>> +.TH AMIDI 1 "30 Aug 2016"
>>
>>  .SH NAME
>>  amidi \- read from and write to ALSA RawMIDI ports
>> @@ -120,6 +120,12 @@ received MIDI commands.
>>  Does not ignore Clock bytes (F8h) when saving or printing received
>>  MIDI commands.
>>
>> +.TP
>> +.I \-i, \-\-sysex-interval=mseconds
>> +Adds a delay in between each SysEx message sent to a device. It is
>> +useful when sending firmware updates via SysEx messages to a remote
>> +device.
>> +
>>  .SH EXAMPLES
>>
>>  .TP
>> @@ -130,6 +136,14 @@ to port
>>  .I hw:0.
>>
>>  .TP
>> +.B amidi \-p hw:1,0,0 -s firmware.syx \-i 100
>> +will send the MIDI commands in
>> +.I firmware.syx
>> +to port
>> +.I hw:1,0,0
>> +with 100 milliseconds delay in between each SysEx message.
>> +
>> +.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 c20512cc96a7..a8264f181cf3 100644
>> --- a/amidi/amidi.c
>> +++ b/amidi/amidi.c
>> @@ -52,6 +52,7 @@ static int receive_file;
>>  static int dump;
>>  static float timeout;
>>  static int stop;
>> +static int sysex_interval;
>>  static snd_rawmidi_t *input, **inputp;
>>  static snd_rawmidi_t *output, **outputp;
>>
>> @@ -70,19 +71,20 @@ static void usage(void)
>>  	printf(
>>  		"Usage: amidi options\n"
>>  		"\n"
>> -		"-h, --help             this help\n"
>> -		"-V, --version          print current version\n"
>> -		"-l, --list-devices     list all hardware ports\n"
>> -		"-L, --list-rawmidis    list all RawMIDI definitions\n"
>> -		"-p, --port=name        select port by name\n"
>> -		"-s, --send=file        send the contents of a (.syx) file\n"
>> -		"-r, --receive=file     write received data into a file\n"
>> -		"-S, --send-hex=\"...\"   send hexadecimal bytes\n"
>> -		"-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   include active sensing bytes\n"
>> -		"-c, --clock            include clock bytes\n");
>> +		"-h, --help                      this help\n"
>> +		"-V, --version                   print current version\n"
>> +		"-l, --list-devices              list all hardware ports\n"
>> +		"-L, --list-rawmidis             list all RawMIDI definitions\n"
>> +		"-p, --port=name                 select port by name\n"
>> +		"-s, --send=file                 send the contents of a (.syx) file\n"
>> +		"-r, --receive=file              write received data into a file\n"
>> +		"-S, --send-hex=\"...\"            send hexadecimal bytes\n"
>> +		"-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            include active sensing bytes\n"
>> +		"-c, --clock                     include clock bytes\n"
>> +		"-i, --sysex-interval=mseconds   delay in between each SysEx message\n");
>>  }
>>
>>  static void version(void)
>> @@ -230,6 +232,47 @@ static void rawmidi_list(void)
>>  	snd_output_close(output);
>>  }
>>
>> +static int send_midi_interleaved(void)
>> +{
>> +	int err;
>> +	char *data = send_data;
>> +	size_t buffer_size;
>> +	snd_rawmidi_params_t *param;
>> +	snd_rawmidi_status_t *st;
>> +
>> +	snd_rawmidi_status_alloca(&st);
>> +
>> +	snd_rawmidi_params_alloca(&param);
>> +	snd_rawmidi_params_current(output, param);
>> +	buffer_size = snd_rawmidi_params_get_buffer_size(param);
>> +
>> +	while (data < (send_data + send_data_length)) {
>> +		int len = send_data + send_data_length - data;
>> +		char *temp;
>> +
>> +		if (data > send_data) {
>> +			snd_rawmidi_status(output, st);
>> +			do {
>> +				/* 320 µs per byte as noted in Page 1 of MIDI spec */
>> +				usleep((buffer_size - snd_rawmidi_status_get_avail(st)) * 320);
>> +				snd_rawmidi_status(output, st);
>> +			} while(snd_rawmidi_status_get_avail(st) < buffer_size);
>> +			usleep(sysex_interval * 1000);
>> +		}
>> +
>> +		/* find end of SysEx */
>> +		if ((temp = memchr(data, 0xf7, len)) != NULL)
>> +			len = temp - data + 1;
>> +
>> +		if ((err = snd_rawmidi_write(output, data, len)) < 0)
>> +			return err;
>> +
>> +		data += len;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static void load_file(void)
>>  {
>>  	int fd;
>> @@ -411,7 +454,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:ac";
>> +	static const char short_options[] = "hVlLp:s:r:S::dt:aci:";
>>  	static const struct option long_options[] = {
>>  		{"help", 0, NULL, 'h'},
>>  		{"version", 0, NULL, 'V'},
>> @@ -425,6 +468,7 @@ int main(int argc, char *argv[])
>>  		{"timeout", 1, NULL, 't'},
>>  		{"active-sensing", 0, NULL, 'a'},
>>  		{"clock", 0, NULL, 'c'},
>> +		{"sysex-interval", 1, NULL, 'i'},
>>  		{ }
>>  	};
>>  	int c, err, ok = 0;
>> @@ -474,6 +518,9 @@ int main(int argc, char *argv[])
>>  		case 'c':
>>  			ignore_clock = 0;
>>  			break;
>> +		case 'i':
>> +			sysex_interval = atoi(optarg);
>> +			break;
>>  		default:
>>  			error("Try `amidi --help' for more information.");
>>  			return 1;
>> @@ -549,9 +596,16 @@ 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 (!sysex_interval) {
>> +			if ((err = snd_rawmidi_write(output, send_data, send_data_length)) < 0) {
>> +				error("cannot send data: %s", snd_strerror(err));
>> +				return err;
>> +			}
>> +		} else {
>> +			if ((err = send_midi_interleaved()) < 0) {
>> +				error("cannot send data: %s", snd_strerror(err));
>> +				return err;
>> +			}
>>  		}
>>  	}

Regards

Takashi Sakamoto