[RFC,06/37] ALSA: firewire-lib: add throttle for MIDI data rate
diff mbox

Message ID 1436623968-10780-7-git-send-email-o-takashi@sakamocchi.jp
State New
Headers show

Commit Message

Takashi Sakamoto July 11, 2015, 2:12 p.m. UTC
Typically, the target devices has internal buffer to adjust output of
received MIDI messages for MIDI serial bus. The capacity of the buffer
is limited, while IEEE 1394 transactions can transfer more MIDI messages
than MIDI serial bus can. For this reason, the MIDI transmission rate
should be constrained.

This commit adds throttle to limit MIDI data rate as 31,250 bits per
seconds. The data rate is enough slower than jiffies granuarity, thus
this commit utilize jiffies count to calculate data rate.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 16 +++++++++++++++-
 sound/firewire/lib.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Jonathan Woithe July 12, 2015, 8:31 a.m. UTC | #1
Hi again Takashi

On Sat, Jul 11, 2015 at 11:12:17PM +0900, Takashi Sakamoto wrote:
> Typically, the target devices has internal buffer to adjust output of
> received MIDI messages for MIDI serial bus. The capacity of the buffer
> is limited, while IEEE 1394 transactions can transfer more MIDI messages
> than MIDI serial bus can. For this reason, the MIDI transmission rate
> should be constrained.

It is good that the core has rate throttling.  I haven't looked at the code
in enough detail but it is important for MOTU devices that it is possible to
make the throttling code do the right thing on the assumption that the
firewire device does not actually have a buffer at all.  Experiments done in
the past have shown that at least some MOTU models are not able to cope with
MIDI data that is sent any faster than the literal line rate of 31250 bps. 
That is, if the bytes of even a single MIDI message are sent faster than
31250 bps then the MIDI bitstream output by the device will be corrupted due
to the dropping of one or more bytes from the message (or other random
creative changes to the data bytes).

In other words, it will need to be possible to tell the throttling code that
the buffer size in the device is 1 byte long and have the code only send to
the MOTU at 31250 bps.  MOTUs cannot be assumed to have any internal buffer
for MIDI bytes.

Obviously software is very likely to send MIDI bytes faster than 31250 bps,
so in the FFADO MOTU MIDI code I needed to implement a short circular buffer
in FFADO in order to match these two rates.  In FFADO this buffer is sized
at 2^10 bytes and this seems to work reliably: so long as the average rate
sent by software stays below 31250 bps, everything is ok.  The relatively
large size is required in order to deal with sysex messages.  I imagine it
will be necessary to provide similar functionality in this implementation of
that driver in ALSA.

Regards
  jonathan

Patch
diff mbox

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index e309b9b..f9c1873 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -76,6 +76,9 @@  static void async_midi_port_callback(struct fw_card *card, int rcode,
 
 	if (rcode == RCODE_COMPLETE && substream != NULL)
 		snd_rawmidi_transmit_ack(substream, port->consume_bytes);
+	else if (!rcode_is_permanent_error(rcode))
+		/* To start next transaction immediately for recovery. */
+		port->next_tick = 0;
 
 	port->idling = true;
 
@@ -99,6 +102,10 @@  static void midi_port_tasklet(unsigned long data)
 	if (substream == NULL || snd_rawmidi_transmit_empty(substream))
 		return;
 
+	/* Do it in next chance. */
+	if (time_is_after_jiffies(port->next_tick))
+		tasklet_schedule(&port->tasklet);
+
 	/*
 	 * Fill the buffer. The callee must use snd_rawmidi_transmit_peek().
 	 * Later, snd_rawmidi_transmit_ack() may be called.
@@ -107,8 +114,10 @@  static void midi_port_tasklet(unsigned long data)
 	port->consume_bytes = port->packetize(substream, port->buf);
 	if (port->consume_bytes <= 0) {
 		/* Do it in next chance, immediately. */
-		if (port->consume_bytes == 0)
+		if (port->consume_bytes == 0) {
+			port->next_tick = 0;
 			tasklet_schedule(&port->tasklet);
+		}
 		return;
 	}
 
@@ -118,6 +127,10 @@  static void midi_port_tasklet(unsigned long data)
 	else
 		type = TCODE_WRITE_BLOCK_REQUEST;
 
+	/* Set interval to next transaction. */
+	port->next_tick =
+		jiffies_64 + msecs_to_jiffies(port->consume_bytes * 8 / 31250);
+
 	/* Start this transaction. */
 	port->idling = false;
 	generation = port->parent->generation;
@@ -153,6 +166,7 @@  int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 	port->addr = addr;
 	port->packetize = packetize;
 	port->idling = true;
+	port->next_tick = 0;
 
 	tasklet_init(&port->tasklet, midi_port_tasklet, (unsigned long)port);
 
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 9d76f5c..e4b00e2 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -26,6 +26,7 @@  struct snd_fw_async_midi_port {
 	struct fw_device *parent;
 	struct tasklet_struct tasklet;
 	bool idling;
+	unsigned long next_tick;
 
 	__u64 addr;
 	struct fw_transaction transaction;