[PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Clemens Ladisch]

There are several devices that expect to receive MIDI data only in the
first eight data blocks of a packet.  If the driver restricts the data
rate to the allowed rate (as mandated by the specification, but not yet
implemented by this driver), this happens naturally.  Therefore, there
is no reason to ever try to use more data packets with any device.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/firewire/amdtp.c                      |   10 +++++++---
 sound/firewire/amdtp.h                      |    3 ---
 sound/firewire/bebob/bebob_stream.c         |    7 -------
 sound/firewire/fireworks/fireworks_stream.c |    5 -----
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 3badc70..17548d8 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -21,6 +21,12 @@
 #define CYCLES_PER_SECOND	8000
 #define TICKS_PER_SECOND	(TICKS_PER_CYCLE * CYCLES_PER_SECOND)

+/*
+ * Several devices look only at the first eight data blocks.
+ * In any case, this is more than enough for the MIDI data rate.
+ */
+#define MAX_MIDI_RX_BLOCKS	8
+
 #define TRANSFER_DELAY_TICKS	0x2e00 /* 479.17 µs */

 /* isochronous header parameters */
@@ -78,8 +84,6 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
 	s->callbacked = false;
 	s->sync_slave = NULL;

-	s->rx_blocks_for_midi = UINT_MAX;
-
 	return 0;
 }
 EXPORT_SYMBOL(amdtp_stream_init);
@@ -474,7 +478,7 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
 		b = (u8 *)&buffer[s->midi_position];

 		port = (s->data_block_counter + f) % 8;
-		if ((f >= s->rx_blocks_for_midi) ||
+		if ((f >= MAX_MIDI_RX_BLOCKS) ||
 		    (s->midi[port] == NULL) ||
 		    (snd_rawmidi_transmit(s->midi[port], b + 1, 1) <= 0))
 			b[0] = 0x80;
diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
index e6e8926..cd4c4df 100644
--- a/sound/firewire/amdtp.h
+++ b/sound/firewire/amdtp.h
@@ -152,9 +152,6 @@ struct amdtp_stream {
 	/* quirk: fixed interval of dbc between previos/current packets. */
 	unsigned int tx_dbc_interval;

-	/* quirk: the first count of data blocks in an rx packet for MIDI */
-	unsigned int rx_blocks_for_midi;
-
 	bool callbacked;
 	wait_queue_head_t callback_wait;
 	struct amdtp_stream *sync_slave;
diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 1aab0a32..0ebcabf 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -484,13 +484,6 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob)
 		amdtp_stream_destroy(&bebob->rx_stream);
 		destroy_both_connections(bebob);
 	}
-	/*
-	 * The firmware for these devices ignore MIDI messages in more than
-	 * first 8 data blocks of an received AMDTP packet.
-	 */
-	if (bebob->spec == &maudio_fw410_spec ||
-	    bebob->spec == &maudio_special_spec)
-		bebob->rx_stream.rx_blocks_for_midi = 8;
 end:
 	return err;
 }
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
index b985fc5..4f440e1 100644
--- a/sound/firewire/fireworks/fireworks_stream.c
+++ b/sound/firewire/fireworks/fireworks_stream.c
@@ -179,11 +179,6 @@ int snd_efw_stream_init_duplex(struct snd_efw *efw)
 		destroy_stream(efw, &efw->tx_stream);
 		goto end;
 	}
-	/*
-	 * Fireworks ignores MIDI messages in more than first 8 data
-	 * blocks of an received AMDTP packet.
-	 */
-	efw->rx_stream.rx_blocks_for_midi = 8;

 	/* set IEC61883 compliant mode (actually not fully compliant...) */
 	err = snd_efw_command_set_tx_mode(efw, SND_EFW_TRANSPORT_MODE_IEC61883);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/2] ALSA: firewire-lib: limit the MIDI data rate

[Clemens Ladisch]

Do no send MIDI bytes at the full rate at which FireWire packets happen
to be sent, but restrict them to the actual rate of a real MIDI port.
This is required by the specification, and prevents data loss when the
device's buffer overruns.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/firewire/amdtp.c |   61 +++++++++++++++++++++++++++++++++++++++++++-----
 sound/firewire/amdtp.h |    2 ++
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index 17548d8..c0c0c5e 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -22,6 +22,12 @@
 #define TICKS_PER_SECOND	(TICKS_PER_CYCLE * CYCLES_PER_SECOND)

 /*
+ * Nominally 3125 bytes/second, but the MIDI port's clock might be
+ * 1% too slow, and the bus clock 100 ppm too fast.
+ */
+#define MIDI_BYTES_PER_SECOND	3093
+
+/*
  * Several devices look only at the first eight data blocks.
  * In any case, this is more than enough for the MIDI data rate.
  */
@@ -226,6 +232,14 @@ sfc_found:
 	for (i = 0; i < pcm_channels; i++)
 		s->pcm_positions[i] = i;
 	s->midi_position = s->pcm_channels;
+
+	/*
+	 * We do not know the actual MIDI FIFO size of most devices.  Just
+	 * assume two bytes, i.e., one byte can be received over the bus while
+	 * the previous one is transmitted over MIDI.
+	 * (The value here is adjusted for midi_ratelimit_per_packet().)
+	 */
+	s->midi_fifo_limit = rate - MIDI_BYTES_PER_SECOND * s->syt_interval + 1;
 }
 EXPORT_SYMBOL(amdtp_stream_set_parameters);

@@ -467,6 +481,36 @@ static void amdtp_fill_pcm_silence(struct amdtp_stream *s,
 	}
 }

+/*
+ * To avoid sending MIDI bytes at too high a rate, assume that the receiving
+ * device has a FIFO, and track how much it is filled.  This values increases
+ * by one whenever we send one byte in a packet, but the FIFO empties at
+ * a constant rate independent of our packet rate.  One packet has syt_interval
+ * samples, so the number of bytes that empty out of the FIFO, per packet(!),
+ * is MIDI_BYTES_PER_SECOND * syt_interval / sample_rate.  To avoid storing
+ * fractional values, the values in midi_fifo_used[] are measured in bytes
+ * multiplied by the sample rate.
+ */
+static bool midi_ratelimit_per_packet(struct amdtp_stream *s, unsigned int port)
+{
+	int used;
+
+	used = s->midi_fifo_used[port];
+	if (used == 0) /* common shortcut */
+		return true;
+
+	used -= MIDI_BYTES_PER_SECOND * s->syt_interval;
+	used = max(used, 0);
+	s->midi_fifo_used[port] = used;
+
+	return used < s->midi_fifo_limit;
+}
+
+static void midi_rate_use_one_byte(struct amdtp_stream *s, unsigned int port)
+{
+	s->midi_fifo_used[port] += amdtp_rate_table[s->sfc];
+}
+
 static void amdtp_fill_midi(struct amdtp_stream *s,
 			    __be32 *buffer, unsigned int frames)
 {
@@ -474,16 +518,21 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
 	u8 *b;

 	for (f = 0; f < frames; f++) {
-		buffer[s->midi_position] = 0;
 		b = (u8 *)&buffer[s->midi_position];

 		port = (s->data_block_counter + f) % 8;
-		if ((f >= MAX_MIDI_RX_BLOCKS) ||
-		    (s->midi[port] == NULL) ||
-		    (snd_rawmidi_transmit(s->midi[port], b + 1, 1) <= 0))
-			b[0] = 0x80;
-		else
+		if (f < MAX_MIDI_RX_BLOCKS &&
+		    midi_ratelimit_per_packet(s, port) &&
+		    s->midi[port] != NULL &&
+		    snd_rawmidi_transmit(s->midi[port], &b[1], 1) == 1) {
+			midi_rate_use_one_byte(s, port);
 			b[0] = 0x81;
+		} else {
+			b[0] = 0x80;
+			b[1] = 0;
+		}
+		b[2] = 0;
+		b[3] = 0;

 		buffer += s->data_block_quadlets;
 	}
diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
index cd4c4df..8a03a91 100644
--- a/sound/firewire/amdtp.h
+++ b/sound/firewire/amdtp.h
@@ -148,6 +148,8 @@ struct amdtp_stream {
 	bool double_pcm_frames;

 	struct snd_rawmidi_substream *midi[AMDTP_MAX_CHANNELS_FOR_MIDI * 8];
+	int midi_fifo_limit;
+	int midi_fifo_used[AMDTP_MAX_CHANNELS_FOR_MIDI * 8];

 	/* quirk: fixed interval of dbc between previos/current packets. */
 	unsigned int tx_dbc_interval;

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Takashi Iwai]

At Tue, 25 Nov 2014 22:52:24 +0100,
Clemens Ladisch wrote:
> 
> There are several devices that expect to receive MIDI data only in the
> first eight data blocks of a packet.  If the driver restricts the data
> rate to the allowed rate (as mandated by the specification, but not yet
> implemented by this driver), this happens naturally.  Therefore, there
> is no reason to ever try to use more data packets with any device.
> 
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

To which branch is the patch applied?


Takashi

> ---
>  sound/firewire/amdtp.c                      |   10 +++++++---
>  sound/firewire/amdtp.h                      |    3 ---
>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>  4 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
> index 3badc70..17548d8 100644
> --- a/sound/firewire/amdtp.c
> +++ b/sound/firewire/amdtp.c
> @@ -21,6 +21,12 @@
>  #define CYCLES_PER_SECOND	8000
>  #define TICKS_PER_SECOND	(TICKS_PER_CYCLE * CYCLES_PER_SECOND)
> 
> +/*
> + * Several devices look only at the first eight data blocks.
> + * In any case, this is more than enough for the MIDI data rate.
> + */
> +#define MAX_MIDI_RX_BLOCKS	8
> +
>  #define TRANSFER_DELAY_TICKS	0x2e00 /* 479.17 µs */
> 
>  /* isochronous header parameters */
> @@ -78,8 +84,6 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
>  	s->callbacked = false;
>  	s->sync_slave = NULL;
> 
> -	s->rx_blocks_for_midi = UINT_MAX;
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL(amdtp_stream_init);
> @@ -474,7 +478,7 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
>  		b = (u8 *)&buffer[s->midi_position];
> 
>  		port = (s->data_block_counter + f) % 8;
> -		if ((f >= s->rx_blocks_for_midi) ||
> +		if ((f >= MAX_MIDI_RX_BLOCKS) ||
>  		    (s->midi[port] == NULL) ||
>  		    (snd_rawmidi_transmit(s->midi[port], b + 1, 1) <= 0))
>  			b[0] = 0x80;
> diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
> index e6e8926..cd4c4df 100644
> --- a/sound/firewire/amdtp.h
> +++ b/sound/firewire/amdtp.h
> @@ -152,9 +152,6 @@ struct amdtp_stream {
>  	/* quirk: fixed interval of dbc between previos/current packets. */
>  	unsigned int tx_dbc_interval;
> 
> -	/* quirk: the first count of data blocks in an rx packet for MIDI */
> -	unsigned int rx_blocks_for_midi;
> -
>  	bool callbacked;
>  	wait_queue_head_t callback_wait;
>  	struct amdtp_stream *sync_slave;
> diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> index 1aab0a32..0ebcabf 100644
> --- a/sound/firewire/bebob/bebob_stream.c
> +++ b/sound/firewire/bebob/bebob_stream.c
> @@ -484,13 +484,6 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob)
>  		amdtp_stream_destroy(&bebob->rx_stream);
>  		destroy_both_connections(bebob);
>  	}
> -	/*
> -	 * The firmware for these devices ignore MIDI messages in more than
> -	 * first 8 data blocks of an received AMDTP packet.
> -	 */
> -	if (bebob->spec == &maudio_fw410_spec ||
> -	    bebob->spec == &maudio_special_spec)
> -		bebob->rx_stream.rx_blocks_for_midi = 8;
>  end:
>  	return err;
>  }
> diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
> index b985fc5..4f440e1 100644
> --- a/sound/firewire/fireworks/fireworks_stream.c
> +++ b/sound/firewire/fireworks/fireworks_stream.c
> @@ -179,11 +179,6 @@ int snd_efw_stream_init_duplex(struct snd_efw *efw)
>  		destroy_stream(efw, &efw->tx_stream);
>  		goto end;
>  	}
> -	/*
> -	 * Fireworks ignores MIDI messages in more than first 8 data
> -	 * blocks of an received AMDTP packet.
> -	 */
> -	efw->rx_stream.rx_blocks_for_midi = 8;
> 
>  	/* set IEC61883 compliant mode (actually not fully compliant...) */
>  	err = snd_efw_command_set_tx_mode(efw, SND_EFW_TRANSPORT_MODE_IEC61883);
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Clemens Ladisch]

Takashi Iwai wrote:
> Clemens Ladisch wrote:
>>
>> There are several devices that expect to receive MIDI data only in the
>> first eight data blocks of a packet.  If the driver restricts the data
>> rate to the allowed rate (as mandated by the specification, but not yet
>> implemented by this driver), this happens naturally.  Therefore, there
>> is no reason to ever try to use more data packets with any device.
>>
>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>
> To which branch is the patch applied?

Sorry; this is intended for 3.19.


Regards,
Clemens

>
>
> Takashi
>
>> ---
>>  sound/firewire/amdtp.c                      |   10 +++++++---
>>  sound/firewire/amdtp.h                      |    3 ---
>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>>  4 files changed, 7 insertions(+), 18 deletions(-)
>>
>> diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
>> index 3badc70..17548d8 100644
>> --- a/sound/firewire/amdtp.c
>> +++ b/sound/firewire/amdtp.c
>> @@ -21,6 +21,12 @@
>>  #define CYCLES_PER_SECOND	8000
>>  #define TICKS_PER_SECOND	(TICKS_PER_CYCLE * CYCLES_PER_SECOND)
>>
>> +/*
>> + * Several devices look only at the first eight data blocks.
>> + * In any case, this is more than enough for the MIDI data rate.
>> + */
>> +#define MAX_MIDI_RX_BLOCKS	8
>> +
>>  #define TRANSFER_DELAY_TICKS	0x2e00 /* 479.17 µs */
>>
>>  /* isochronous header parameters */
>> @@ -78,8 +84,6 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
>>  	s->callbacked = false;
>>  	s->sync_slave = NULL;
>>
>> -	s->rx_blocks_for_midi = UINT_MAX;
>> -
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(amdtp_stream_init);
>> @@ -474,7 +478,7 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
>>  		b = (u8 *)&buffer[s->midi_position];
>>
>>  		port = (s->data_block_counter + f) % 8;
>> -		if ((f >= s->rx_blocks_for_midi) ||
>> +		if ((f >= MAX_MIDI_RX_BLOCKS) ||
>>  		    (s->midi[port] == NULL) ||
>>  		    (snd_rawmidi_transmit(s->midi[port], b + 1, 1) <= 0))
>>  			b[0] = 0x80;
>> diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
>> index e6e8926..cd4c4df 100644
>> --- a/sound/firewire/amdtp.h
>> +++ b/sound/firewire/amdtp.h
>> @@ -152,9 +152,6 @@ struct amdtp_stream {
>>  	/* quirk: fixed interval of dbc between previos/current packets. */
>>  	unsigned int tx_dbc_interval;
>>
>> -	/* quirk: the first count of data blocks in an rx packet for MIDI */
>> -	unsigned int rx_blocks_for_midi;
>> -
>>  	bool callbacked;
>>  	wait_queue_head_t callback_wait;
>>  	struct amdtp_stream *sync_slave;
>> diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
>> index 1aab0a32..0ebcabf 100644
>> --- a/sound/firewire/bebob/bebob_stream.c
>> +++ b/sound/firewire/bebob/bebob_stream.c
>> @@ -484,13 +484,6 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob)
>>  		amdtp_stream_destroy(&bebob->rx_stream);
>>  		destroy_both_connections(bebob);
>>  	}
>> -	/*
>> -	 * The firmware for these devices ignore MIDI messages in more than
>> -	 * first 8 data blocks of an received AMDTP packet.
>> -	 */
>> -	if (bebob->spec == &maudio_fw410_spec ||
>> -	    bebob->spec == &maudio_special_spec)
>> -		bebob->rx_stream.rx_blocks_for_midi = 8;
>>  end:
>>  	return err;
>>  }
>> diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c
>> index b985fc5..4f440e1 100644
>> --- a/sound/firewire/fireworks/fireworks_stream.c
>> +++ b/sound/firewire/fireworks/fireworks_stream.c
>> @@ -179,11 +179,6 @@ int snd_efw_stream_init_duplex(struct snd_efw *efw)
>>  		destroy_stream(efw, &efw->tx_stream);
>>  		goto end;
>>  	}
>> -	/*
>> -	 * Fireworks ignores MIDI messages in more than first 8 data
>> -	 * blocks of an received AMDTP packet.
>> -	 */
>> -	efw->rx_stream.rx_blocks_for_midi = 8;
>>
>>  	/* set IEC61883 compliant mode (actually not fully compliant...) */
>>  	err = snd_efw_command_set_tx_mode(efw, SND_EFW_TRANSPORT_MODE_IEC61883);
>>

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Takashi Sakamoto]

Clemens,

On Nov 26 2014 06:52, Clemens Ladisch wrote:
> There are several devices that expect to receive MIDI data only in the
> first eight data blocks of a packet.  If the driver restricts the data
> rate to the allowed rate (as mandated by the specification, but not yet
> implemented by this driver), this happens naturally.  Therefore, there
> is no reason to ever try to use more data packets with any device.
> 
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> ---
>  sound/firewire/amdtp.c                      |   10 +++++++---
>  sound/firewire/amdtp.h                      |    3 ---
>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>  4 files changed, 7 insertions(+), 18 deletions(-)

Can I ask your opinion about applying this patch to devices with
non-blocking mode? At least, your comment of this patch is for blocking
mode (the fixed number of data blocks in a packet).


Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Clemens Ladisch]

Takashi Sakamoto wrote:
> On Nov 26 2014 06:52, Clemens Ladisch wrote:
>> There are several devices that expect to receive MIDI data only in the
>> first eight data blocks of a packet.  If the driver restricts the data
>> rate to the allowed rate (as mandated by the specification, but not yet
>> implemented by this driver), this happens naturally.  Therefore, there
>> is no reason to ever try to use more data packets with any device.
>>
>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>> ---
>>  sound/firewire/amdtp.c                      |   10 +++++++---
>>  sound/firewire/amdtp.h                      |    3 ---
>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>>  4 files changed, 7 insertions(+), 18 deletions(-)
>
> Can I ask your opinion about applying this patch to devices with
> non-blocking mode? At least, your comment of this patch is for blocking
> mode (the fixed number of data blocks in a packet).

This patch does not assume that there is a fixed number of data blocks.

It does not matter whether a stream is (non-)blocking: eight data blocks
per packet (i.e., one MIDI byte per MPX-MIDI data channel per packet) is
always enough for MIDI.  Even at 32 kHz, there are about 4000 packets
per seconds in blocking mode.  In non-blocking mode, the packets are
smaller (so not ecery MPX-MIDI data channels gets sent in every packet),
but there are exactly 8000 packets per second, so the overall number of
samples (= data blocks) does not change.


Regards,
Clemens

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Takashi Sakamoto]

On Nov 26 2014 23:38, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> On Nov 26 2014 06:52, Clemens Ladisch wrote:
>>> There are several devices that expect to receive MIDI data only in the
>>> first eight data blocks of a packet.  If the driver restricts the data
>>> rate to the allowed rate (as mandated by the specification, but not yet
>>> implemented by this driver), this happens naturally.  Therefore, there
>>> is no reason to ever try to use more data packets with any device.
>>>
>>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>>> ---
>>>  sound/firewire/amdtp.c                      |   10 +++++++---
>>>  sound/firewire/amdtp.h                      |    3 ---
>>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>>>  4 files changed, 7 insertions(+), 18 deletions(-)
>>
>> Can I ask your opinion about applying this patch to devices with
>> non-blocking mode? At least, your comment of this patch is for blocking
>> mode (the fixed number of data blocks in a packet).
> 
> This patch does not assume that there is a fixed number of data blocks.
> 
> It does not matter whether a stream is (non-)blocking: eight data blocks
> per packet (i.e., one MIDI byte per MPX-MIDI data channel per packet) is
> always enough for MIDI.  Even at 32 kHz, there are about 4000 packets
> per seconds in blocking mode.  In non-blocking mode, the packets are
> smaller (so not ecery MPX-MIDI data channels gets sent in every packet),
> but there are exactly 8000 packets per second, so the overall number of
> samples (= data blocks) does not change.

I was suspicious of unfairless for 8 MPX-MIDI data streams to transfer
MIDI messages in non-blocking mode, because .the MPX-MIDI data stream in
the first data block is different per packet.

But this is not matter. At 32.0/44.1/48.0, the number of data blocks in
a packet is lesser than 8 therefore all of MPX-MIDI data stream has the
same chances to transfer MIDI messages. At 88.2 kHz or higher, the
number of data blocks in a packet is bigger than 8 but8 MPX-MIDI data
streams has a chance per packet. As a result, All of MPX-MIDI data
stream has the same opportunity to transfer MIDI messages.

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

I tested these two patch with below devices. All of them can transmit
MIDI messages correctly at any sampling transfer tates.

M-Audio: FIreWire 1814 (snd-bebob)
M-Audio: FireWire 410 (snd-bebob)
M-Audio: FireWire AudioPhile (snd-bebob)
M-Audio: Ozonic (snd-bebob)
Yamaha: GO44 (snd-bebob)
Yamaha: GO46 (snd-bebob)
Echo Audio: AudioFire4 (snd-fireworks)
Echo Audio: AudioFirePre8 (snd-fireworks)
TC Electronic: Impact Twin (snd-dice)

These two patches solves one of issues which I wrote in my report.
9.5 A lack of throttles for MIDI messages in outgoing stream
https://github.com/takaswie/alsa-firewire-report


Thanks

Takashi Sakamoto
o-takashi@sakamocchi.jp

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Takashi Sakamoto]

Iwai-san,

On Nov 27 2014 21:45, Takashi Sakamoto wrote:
> On Nov 26 2014 23:38, Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> On Nov 26 2014 06:52, Clemens Ladisch wrote:
>>>> There are several devices that expect to receive MIDI data only in the
>>>> first eight data blocks of a packet.  If the driver restricts the data
>>>> rate to the allowed rate (as mandated by the specification, but not yet
>>>> implemented by this driver), this happens naturally.  Therefore, there
>>>> is no reason to ever try to use more data packets with any device.
>>>>
>>>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>>>> ---
>>>>  sound/firewire/amdtp.c                      |   10 +++++++---
>>>>  sound/firewire/amdtp.h                      |    3 ---
>>>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>>>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>>>>  4 files changed, 7 insertions(+), 18 deletions(-)
>
> Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> 
> I tested these two patch with below devices. All of them can transmit
> MIDI messages correctly at any sampling transfer tates.
> 
> M-Audio: FIreWire 1814 (snd-bebob)
> M-Audio: FireWire 410 (snd-bebob)
> M-Audio: FireWire AudioPhile (snd-bebob)
> M-Audio: Ozonic (snd-bebob)
> Yamaha: GO44 (snd-bebob)
> Yamaha: GO46 (snd-bebob)
> Echo Audio: AudioFire4 (snd-fireworks)
> Echo Audio: AudioFirePre8 (snd-fireworks)
> TC Electronic: Impact Twin (snd-dice)
> 
> These two patches solves one of issues which I wrote in my report.
> 9.5 A lack of throttles for MIDI messages in outgoing stream
> https://github.com/takaswie/alsa-firewire-report

I think these two patches are deserved to merge. Could you apply these
two patches to linux-next?

Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Clemens Ladisch]

Takashi Sakamoto wrote:
> Iwai-san,
>
> On Nov 27 2014 21:45, Takashi Sakamoto wrote:
>> On Nov 26 2014 23:38, Clemens Ladisch wrote:
>>> Takashi Sakamoto wrote:
>>>> On Nov 26 2014 06:52, Clemens Ladisch wrote:
>>>>> There are several devices that expect to receive MIDI data only in the
>>>>> first eight data blocks of a packet.  If the driver restricts the data
>>>>> rate to the allowed rate (as mandated by the specification, but not yet
>>>>> implemented by this driver), this happens naturally.  Therefore, there
>>>>> is no reason to ever try to use more data packets with any device.
>>>>>
>>>>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>>>>> ---
>>>>>  sound/firewire/amdtp.c                      |   10 +++++++---
>>>>>  sound/firewire/amdtp.h                      |    3 ---
>>>>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
>>>>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
>>>>>  4 files changed, 7 insertions(+), 18 deletions(-)
>>
>> Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>> Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>
> I think these two patches are deserved to merge. Could you apply these
> two patches to linux-next?
>
> Regards
>
> Takashi Sakamoto
> o-takashi@sakamocchi.jp

Ping?

Re: [PATCH 1/2] ALSA: firewire-lib: remove rx_blocks_for_midi quirk

[Takashi Iwai]

At Fri, 16 Jan 2015 22:18:15 +0100,
Clemens Ladisch wrote:
> 
> Takashi Sakamoto wrote:
> > Iwai-san,
> >
> > On Nov 27 2014 21:45, Takashi Sakamoto wrote:
> >> On Nov 26 2014 23:38, Clemens Ladisch wrote:
> >>> Takashi Sakamoto wrote:
> >>>> On Nov 26 2014 06:52, Clemens Ladisch wrote:
> >>>>> There are several devices that expect to receive MIDI data only in the
> >>>>> first eight data blocks of a packet.  If the driver restricts the data
> >>>>> rate to the allowed rate (as mandated by the specification, but not yet
> >>>>> implemented by this driver), this happens naturally.  Therefore, there
> >>>>> is no reason to ever try to use more data packets with any device.
> >>>>>
> >>>>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> >>>>> ---
> >>>>>  sound/firewire/amdtp.c                      |   10 +++++++---
> >>>>>  sound/firewire/amdtp.h                      |    3 ---
> >>>>>  sound/firewire/bebob/bebob_stream.c         |    7 -------
> >>>>>  sound/firewire/fireworks/fireworks_stream.c |    5 -----
> >>>>>  4 files changed, 7 insertions(+), 18 deletions(-)
> >>
> >> Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >> Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >
> > I think these two patches are deserved to merge. Could you apply these
> > two patches to linux-next?
> >
> > Regards
> >
> > Takashi Sakamoto
> > o-takashi@sakamocchi.jp
> 
> Ping?

Sorry, overlooked.  Merged now.


Takashi