[Bluez-devel] [PATCH] a2dp_write patch

[Bluez-devel] [PATCH] a2dp_write patch

[keith preston]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

This is a patch to rewrite a2dp_write to avoid memory copies.   It
does this by only using the temporary buffer if we are dealing with
partial buffers.  I see a little bit of a speedup with my embedded
device, however sbc encoding usually takes the most time.

This also fixes a small bug in pcm_bluetooth.c.   Typically in an alsa
write call you return the number of samples(frames) that you have
processed.  However in this case where we don't have a full block to
encode in SBC, we return basically return (incoming_buffer_size) %
(SBC_Encode_Block_size).   This is correct in the case where
(incoming_buffer_size) < (SBC_Encode_Block_size), however is wrong
when (incoming_buffer_size) > (SBC_Encode_Block_size), because this
statement ignores previously processed blocks.

Keith Preston

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: a2dp_write.patch --]
[-- Type: text/x-patch; name=a2dp_write.patch, Size: 4838 bytes --]

--- bluez-utils-3.36.orig/audio/pcm_bluetooth.c	2008-07-23 17:13:13.000000000 -0500
+++ bluez-utils-3.36/audio/pcm_bluetooth.c	2008-07-24 13:50:31.000000000 -0500
@@ -974,21 +974,25 @@
 	struct bluetooth_data *data = io->private_data;
 	struct bluetooth_a2dp *a2dp = &data->a2dp;
 	snd_pcm_sframes_t ret = 0;
-	snd_pcm_uframes_t frames_to_read, frames_left = size;
-	int frame_size, encoded, written;
+	int frame_size, encoded, written, bytes_left;
 	uint8_t *buff;
-
 	DBG("areas->step=%u areas->first=%u offset=%lu size=%lu",
 				areas->step, areas->first, offset, size);
 	DBG("hw_ptr=%lu appl_ptr=%lu diff=%lu", io->hw_ptr, io->appl_ptr,
 			io->appl_ptr - io->hw_ptr);
 
+	/* Calutate starting pointers */
+	frame_size = areas->step / 8;
+	bytes_left = size * frame_size;
+	buff = (uint8_t *) areas->addr +
+		(areas->first + areas->step * (offset)) / 8;
+	/* Check for underrun */
 	if (io->hw_ptr > io->appl_ptr) {
 		ret = bluetooth_playback_stop(io);
 		if (ret == 0)
 			ret = -EPIPE;
 		data->reset = 1;
-		goto done;
+		return ret;
 	}
 
 	/* Check if we should autostart */
@@ -1003,39 +1007,18 @@
 			if (io->appl_ptr >= threshold) {
 				ret = snd_pcm_start(io->pcm);
 				if (ret != 0)
-					goto done;
+					return ret;
 			}
 		}
 
 		snd_pcm_sw_params_free(swparams);
 	}
 
-	while (frames_left > 0) {
-		frame_size = areas->step / 8;
-
-		if ((data->count + frames_left * frame_size) <= a2dp->codesize)
-			frames_to_read = frames_left;
-		else
-			frames_to_read = (a2dp->codesize - data->count) / frame_size;
-
-		DBG("count=%d frames_to_read=%lu", data->count, frames_to_read);
-		DBG("a2dp.count=%d data.link_mtu=%d", a2dp->count, data->link_mtu);
-
-		/* FIXME: If state is not streaming then return */
-
-		/* Ready for more data */
-		buff = (uint8_t *) areas->addr +
-			(areas->first + areas->step * (offset + ret)) / 8;
+	/* Check if we have any left over data from the last write */
+	if(data->count > 0 && ((bytes_left - data->count) >= a2dp->codesize)) {
+		int additional_bytes_needed = a2dp->codesize - data->count;
 		memcpy(data->buffer + data->count, buff,
-				frame_size * frames_to_read);
-
-		/* Remember we have some frames in the pipe now */
-		data->count += frames_to_read * frame_size;
-		if (data->count != a2dp->codesize) {
-			ret = frames_to_read;
-			goto done;
-		}
-
+				additional_bytes_needed);
 		/* Enough data to encode (sbc wants 1k blocks) */
 		encoded = sbc_encode(&(a2dp->sbc), data->buffer, a2dp->codesize,
 					a2dp->buffer + a2dp->count,
@@ -1045,35 +1028,65 @@
 			DBG("Encoding error %d", encoded);
 			goto done;
 		}
-
-		data->count -= encoded;
+		/* Increment a2dp buffers */
 		a2dp->count += written;
 		a2dp->frame_count++;
 		a2dp->samples += encoded / frame_size;
 		a2dp->nsamples += encoded / frame_size;
-
-		DBG("encoded=%d  written=%d count=%d", encoded,
-				written, a2dp->count);
-
 		/* No space left for another frame then send */
 		if (a2dp->count + written >= data->link_mtu) {
 			avdtp_write(data);
 			DBG("sending packet %d, count %d, link_mtu %u",
 					a2dp->seq_num, a2dp->count,
 					data->link_mtu);
-		}
-
-		ret += frames_to_read;
-		frames_left -= frames_to_read;
+		}		
+		/* Increment up buff pointer to take into account the data processed */
+		buff += additional_bytes_needed;
+		bytes_left -= additional_bytes_needed;
+		/* Since data has been process mark it as zero */
+		data->count = 0;
 	}
 
-	/* note: some ALSA apps will get confused otherwise */
-	if (ret > size)
-		ret = size;
+
+	/* Process this buffer in full chunks */
+	while( bytes_left >= a2dp->codesize)
+	{
+		/* Enough data to encode (sbc wants 1k blocks) */
+		encoded = sbc_encode(&(a2dp->sbc), buff, a2dp->codesize,
+					a2dp->buffer + a2dp->count,
+					sizeof(a2dp->buffer) - a2dp->count,
+					&written);
+		if (encoded <= 0) {
+			DBG("Encoding error %d", encoded);
+			goto done;
+		}
+		/* Increment up buff pointer to take into account the data processed */
+		buff += a2dp->codesize;
+		bytes_left -= a2dp->codesize;
+		/* Increment a2dp buffers */
+		a2dp->count += written;
+		a2dp->frame_count++;
+		a2dp->samples += encoded / frame_size;
+		a2dp->nsamples += encoded / frame_size;
+		/* No space left for another frame then send */
+		if (a2dp->count + written >= data->link_mtu) {
+			avdtp_write(data);
+			printf("sending packet %d, count %d, link_mtu %u",
+					a2dp->seq_num, a2dp->count,
+					data->link_mtu);
+		}		
+	}
+	/* Copy the extra to our temp buffer for the next write */
+	if(bytes_left > 0) {
+		memcpy(data->buffer + data->count, buff,
+				bytes_left);
+		data->count += bytes_left;
+		bytes_left = 0;
+	}
 
 done:
-	DBG("returning %ld", ret);
-	return ret;
+	DBG("returning %ld", size - bytes_left / frame_size);
+	return size - bytes_left / frame_size;
 }
 
 static int bluetooth_playback_delay(snd_pcm_ioplug_t *io,

[-- Attachment #3: Type: text/plain, Size: 363 bytes --]

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] a2dp_write patch

[Luiz Augusto von Dentz]

Hello Keith,

Well done, now Im wondering if it isn't possible to use alsa buffer
all the time instead of copying to a temporary buffer? I mean can't we
just point to the first pcm frame processed but not encoded? (If not
freed already by alsa)
Also there is some code duplication, but still the idea is very good.
If you could please check the performance with oprofile.


-- =

Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great priz=
es
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=3D100&url=3D/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] a2dp_write patch

[keith preston]

> Well done, now Im wondering if it isn't possible to use alsa buffer
> all the time instead of copying to a temporary buffer?

You definitely can't depend on the buffer still being there.   There
are two options, you can just refuse to process any part of the buffer
that is not a certain size, or you can do what we are trying to do and
store it off temporarily.   I think the last is the best solution,
because it makes it easier on the our clients,  we have to deal with
the extra instead of an application needed to deal with half processed
buffers.

 I mean can't we
> just point to the first pcm frame processed but not encoded? (If not
> freed already by alsa)
> Also there is some code duplication, but still the idea is very good.
> If you could please check the performance with oprofile.
>
>
> --

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] a2dp_write patch

[Marcel Holtmann]

Hi Keith,

> > Well done, now Im wondering if it isn't possible to use alsa buffer
> > all the time instead of copying to a temporary buffer?
> 
> You definitely can't depend on the buffer still being there.   There
> are two options, you can just refuse to process any part of the buffer
> that is not a certain size, or you can do what we are trying to do and
> store it off temporarily.   I think the last is the best solution,
> because it makes it easier on the our clients,  we have to deal with
> the extra instead of an application needed to deal with half processed
> buffers.

I applied your patch to the BlueZ 4.x repository. Thanks.

Regards

Marcel



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel