[alsa-devel] [PATCH 4/4] Add handling AMDTP receive stream

[Clemens Ladisch]

Takashi Sakamoto wrote:
> To handle AMDTP receive stream, this patch adds some codes with condition of its
> direction and new functions. Once amdtp_stream_init() is executed with its
> direction, AMDTP receive and transmit stream can be handles by the same way.
>
> Unfortunatelly Echo Audio's Fireworks(TM) is not fully compliant to IEC 61883-6
> against their guide. This patch include some work arounds for Fireworks.

> +++ b/sound/firewire/amdtp.c

> +static void handle_in_packet_data(struct amdtp_stream *s,
> +						unsigned int data_quadlets)

> +	/* checking CIP headers for AMDTP with restriction of this module */
> +	if (((be32_to_cpu(buffer[0]) & CIP_EOH_MASK) == CIP_EOH) ||
> +	    ((be32_to_cpu(buffer[1]) & CIP_EOH_MASK) != CIP_EOH) ||
> +	    ((be32_to_cpu(buffer[1]) & CIP_FMT_MASK) != CIP_FMT_AM)) {

It would be easier and a little bit more efficient to convert the two
header quadlets at the beginning and to store them in some local
variables.

> +		dev_err(&s->unit->device, "CIP headers error: %08X:%08X\n",
> +			be32_to_cpu(buffer[0]), be32_to_cpu(buffer[1]));
> +		return;

This error should either be ignored (resubmit the packet) or cause the
stream to abort (see "queueing error" below).

But I'm not sure which.

> +		 * Echo Audio's Fireworks reports wrong number of data block
> +		 * counter. Mostly it reports it with increment of 8 blocks
> +		 * but sometimes it increments with NO-DATA packet.

Does this imply that the DBC field could not be used to detect dropped
packets?

> +		 * Handling syt field is related to time stamp,
> +		 * but the cost is bigger than the effect.

Actually, the SYT specifies the presentation timestamp, but when doing
capturing in ALSA, there is no "presentation" in the sense of the spec,
so not handling the SYT field is the correct thing to do regardless of
the cost.  :-)

> +		 * NOTE: this module doesn't check dbc ...

> +		data_block_counter  = (be32_to_cpu(buffer[0]) & AMDTP_DBC_MASK);

Unused.

> +		/* for next packet */
> +		s->data_block_quadlets = data_block_quadlets;
> +		s->data_block_counter  = data_block_counter;

Unused.

> +	/* calcurate packet index */

> +	/* calcurate period and buffer borders */

	   calculate


Regards,
Clemens