Date: Tue, 8 Mar 2011 13:23:25 -0500
From: Jay Fenlason <fenlason@redhat.com>
To: Clemens Ladisch <clemens@ladisch.de>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>,
        linux1394-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2 v7] ALSA: add LaCie FireWire Speakers/Griffin
 FireWave Surround driver
Message-ID: <20110308182324.GA26141@redhat.com>
References: <4D5928AA.4060609@ladisch.de>
 <4D6609C2.4070604@ladisch.de>
 <20110224180418.GE31519@redhat.com>
 <4D669F88.5040005@ladisch.de>
 <4D67588D.3000200@ladisch.de>
 <4D6CC2BE.2010506@ladisch.de>
 <20110301192233.GA2844@redhat.com>
 <4D6DF312.6000107@ladisch.de>
 <20110303224612.GA21068@redhat.com>
 <4D709848.4080101@ladisch.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4D709848.4080101@ladisch.de>
User-Agent: Mutt/1.5.20 (2009-12-10)

With the following patch I no longer get glitching on the LFE
channel.  Apparently the device was clipping internally when the
sixteen bit output from ogg123/PulseAudio was shifted into the
high-order bits of the 24-bit space.  Putting it in the middle of the
space prevents glitching while providing reasonably loud output.


I still need to investigate what the Griffin driver on the Mac does
wrt switching in and out of Dolby mode.  It would be good if
snd-firewire-speakers could at least take it out of Dolby mode in case
a Mac left it in that state.

Oh, and snd-firewire-speakers does not coexist well with with a Mac on
the same bus.  It appears to loop forever logging errors that the plug
register is in use, instead of returning EBUSY to userspace and giving
up.  The Mac is apparently not smart enough to only stream to the
device when it has something to play, which does terrible things for
power consumption.

			-- JF

--- amdtp.c.orig	2011-03-07 16:02:25.000000000 -0500
+++ amdtp.c	2011-03-08 10:53:39.000000000 -0500
@@ -271,7 +271,8 @@
 {
 	struct snd_pcm_runtime *runtime = pcm->runtime;
 	unsigned int channels, remaining_frames, frame_step, i, c;
-	const u16 *src;
+	const s16 *src;
+	s32 tmp;
 
 	channels = s->pcm_channels;
 	src = (void *)runtime->dma_area +
@@ -281,7 +282,13 @@
 
 	for (i = 0; i < frames; ++i) {
 		for (c = 0; c < channels; ++c) {
-			*buffer = cpu_to_be32((*src << 8) | 0x40000000);
+			/*
+			 * We need to sign-extend *src, then clip it to
+			 * 24 bits and add the amdtp sample header.
+			 * (ugh)
+			 */
+			tmp = 0x40000000 | ( ( *src << 4 ) & 0x00ffffff );
+			*buffer = cpu_to_be32(tmp);
 			src++;
 			buffer++;
 		}