(Nov 26 2013 00:27), Clemens Ladisch wrote:
+++ b/sound/firewire/amdtp.h +/*
- This module supports maximum 2 MIDI channels.
- Then AMDTP packets include maximum 16 MIDI streams multiplexed.
- This is for our convinience.
- */
+#define AMDTP_MAX_CHANNELS_FOR_MIDI 2
This does not look like 16 ports ...
- unsigned int midi_ports;
- unsigned int midi_channels;
... and "MIDI channels" means something different. The RP-027 document defines things like "MIDI data stream", "MIDI Conformant Data Channel", and "MPX-MIDI Data Channel"; please clarify which one you mean.
In my understanding of RP-027: 'MIDI data stream' is a data stream to/from each MIDI ports 'MPX-MIDI data channel' is a data channel for each 'MIDI data stream' 'MIDI comformant data channel' has multiplexed 8 'MPX-MIDI data channel's
So I should write:
/* * This module supports maximum 2 MIDI comformant data channels. * Each MIDI comformant data channels include 8 MPX-MIDI data stream. * Each MPX-MIDI data streams includes data stream from/to MIDI ports. * Then AMDTP packets include maximum 16 MIDI streams. * This limitation is for our convinience. */
Is this OK?
IEC 61883-6 refers to AMEI/MMA RP-027 for implementation of MIDI conformant data. This module implement 'MIDI1.0-1x-SPEED' mode. In the specification, 'MIDI1.0-2x/3x-SPEED' mode is defined with 'negotiation procedure' and 'encapsulation details'. But I cannot find specifications about them.
The 2x/3x speed modes are not specified.
OK. I'll add it in comment.
static void amdtp_fill_midi(struct amdtp_stream *s, __be32 *buffer, unsigned int frames)
if ((s->midi[port] != NULL) &&test_bit(port, &s->midi_triggered)) {The stream pointer (s->midi[]) is not needed when the stream is not active (triggered). So you could drop the midi_triggered field and just update the stream pointer when triggering (like s->pcm).
Oh. That's better.
buffer[s->pcm_channels + c] =be32_to_cpu((b[0] << 24) | (b[1] << 16));Endian conversions are required only for values that occupy more than one byte. You should make 'buffer' a byte pointer so that you can copy b[] directly.
OK.
Thank you
Takashi Sakamoto