linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
@ 2007-10-12 12:25 Kai Vehmanen
  2007-10-14  3:36 ` Luiz Augusto von Dentz
  2007-10-14 11:19 ` Fabien Chevalier
  0 siblings, 2 replies; 10+ messages in thread
From: Kai Vehmanen @ 2007-10-12 12:25 UTC (permalink / raw)
  To: bluez-devel

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

Hello,

I've been having some problems in using the Bluez ALSA/A2DP plugin
with applications that use the ALSA API in event driven fashion (so
'write samples at last possible moment' instead of 'write samples when 
there is free room in the buffer'). One test application I've used is
Pulseaudio. 

Here're couple of patches (against CVS HEAD) that solve the 
problems for me (for instance pulseaudio starts working with
the plugin, less CPU usage with other apps). If you think these
changes are ok, please include them to the upstream codebase.

This first modifies the buffering constraints as exposed by
the A2DP ALSA plugin:
  - increased max overall buffer size to 16384 bytes (~93msecs
    of CD quality audio, so a reasonable max size)
  - allow applications to specify buffer size
  - allow applications to use bigger periods (less wakeup
    load -> the previous limit of 128 is heavy for apps)

I'll submit the other patch as a separate mail, so it's easier
to discuss about the patch contents separately.

This is my first patch to bluez-utils, so please be kind. :)

Br,
-- 
first.surname@nokia.com (Kai Vehmanen)

[-- Attachment #2: patch-kv-bluez-utils-fix-a2dp-buffer-constraints-20071012.txt --]
[-- Type: text/plain, Size: 2252 bytes --]

diff -ruN bluez-utils.orig/audio/pcm_bluetooth.c bluez-utils/audio/pcm_bluetooth.c
--- bluez-utils.orig/audio/pcm_bluetooth.c	2007-10-12 12:57:28.000000000 +0300
+++ bluez-utils/audio/pcm_bluetooth.c	2007-10-12 15:04:49.000000000 +0300
@@ -45,7 +45,8 @@
 
 #define MIN_PERIOD_TIME 1
 
-#define BUFFER_SIZE 2048
+#define MIN_BUFFER_SIZE 256    /* minimum size of buffer */
+#define MAX_BUFFER_SIZE 16384  /* allocated RAM for buffer */
 
 #ifdef ENABLE_DEBUG
 #define DBG(fmt, arg...)  printf("DEBUG: %s: " fmt "\n" , __FUNCTION__ , ## arg)
@@ -123,7 +124,7 @@
 	sbc_t sbc;			/* Codec data */
 	int codesize;			/* SBC codesize */
 	int samples;			/* Number of encoded samples */
-	uint8_t buffer[BUFFER_SIZE];	/* Codec transfer buffer */
+	uint8_t buffer[MAX_BUFFER_SIZE];/* Codec transfer buffer */
 	int count;			/* Codec transfer buffer counter */
 
 	int nsamples;			/* Cumulative number of codec samples */
@@ -137,7 +138,7 @@
 	struct ipc_data_cfg cfg;	/* Bluetooth device config */
 	struct pollfd stream;		/* Audio stream filedescriptor */
 	struct pollfd server;		/* Audio daemon filedescriptor */
-	uint8_t buffer[BUFFER_SIZE];	/* Encoded transfer buffer */
+	uint8_t buffer[MAX_BUFFER_SIZE];/* Encoded transfer buffer */
 	int count;			/* Transfer buffer counter */
 	struct bluetooth_a2dp a2dp;	/* A2DP data */
 
@@ -932,14 +933,24 @@
 	if (err < 0)
 		return err;
 
-	/* supported block size */
+	/* supported block sizes:
+	 *   - lower limit is A2DP codec size
+	 *   - total buffer size is the upper limit (with two periods) */
 	err = snd_pcm_ioplug_set_param_minmax(io, SND_PCM_IOPLUG_HW_PERIOD_BYTES,
-						a2dp->codesize, a2dp->codesize);
+					      a2dp->codesize, MAX_BUFFER_SIZE / 2);
+	if (err < 0)
+		return err;
+
+	/* supported buffer sizes */
+	err = snd_pcm_ioplug_set_param_minmax(io, SND_PCM_IOPLUG_HW_BUFFER_BYTES,
+					      MIN_BUFFER_SIZE, MAX_BUFFER_SIZE);
 	if (err < 0)
 		return err;
 
+	/* supported period count: 
+	 *   - derived from max buffer size and minimum period size */
 	err = snd_pcm_ioplug_set_param_minmax(io, SND_PCM_IOPLUG_HW_PERIODS,
-									2, 50);
+					      2, MAX_BUFFER_SIZE / a2dp->codesize);
 	if (err < 0)
 		return err;
 

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- 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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
@ 2007-10-17 16:31 Kai.Vehmanen
  2007-10-18  8:42 ` Johan Hedberg
  2007-10-18 10:08 ` Fabien Chevalier
  0 siblings, 2 replies; 10+ messages in thread
From: Kai.Vehmanen @ 2007-10-17 16:31 UTC (permalink / raw)
  To: bluez-devel

Hi,

On 12 Oct 2007, Kai Vehmanen wrote: 
>Here're couple of patches (against CVS HEAD) that solve the 
>problems for me (for instance pulseaudio starts working with 
>the plugin, less CPU usage with other apps). If you think 
[...]
>This first modifies the buffering constraints as exposed by 
>the A2DP ALSA plugin:
>  - increased max overall buffer size to 16384 bytes (~93msecs
>    of CD quality audio, so a reasonable max size)

hmm, looking at this more deeply, I realized this doesn't quite 
work. I originally thought there is a real ringbuffer (imitating 
the usual audio hardware ringbuffer) between the worker
thread and the application, but this appears not to be the
case. Current code seems to gather samples-to-be-sent in a buffer,
and when a threshold (a2dp->codesize or cfg.pkt_len) is reached,
a packet is then sent right away. So in other words, application
cannot really prefill more data than one packet worth, and most of 
the allocated buffer space gets never used.

So I guess this specific patch should be dropped (it might fix
some issues, but at least the assumptions I used to pick the 
values are wrong ;)). The other patch should still be valid.

What I intended with the patch was that you could...:
  - prefill N*a2dp_codesize (less than buffersize) worth of 
    PCM frames
  - trigger playback 
      -> oldest queued a2dp_codesize worth of samples is encoded 
        and sent
  - application is woken up every M*a2dp_codesize
      - in case application is occasionally late, the worker
        thread would still be encoding and sending packets
        at a steady rate using the buffered samples
	- with M>1, application can reduce the amount of wakeups
        needed, but will increase the playout latency (but
        note: the HW thread still needs to wake up for every 
        a2dp_codesize)
  - an XRUN occurs only if application is stalled for 
    duration of N*a2dp_codesize
        
But yeah, that would seem to be a much bigger task to implement (and
something you may have already discussed earlier).

br,
-- 
first.surname@nokia.com (Kai Vehmanen)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-10-18 17:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-12 12:25 [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints Kai Vehmanen
2007-10-14  3:36 ` Luiz Augusto von Dentz
2007-10-14 11:19 ` Fabien Chevalier
2007-10-15 10:54   ` [Bluez-devel] PATCH1/2: bluez-utils -fix-a2dp-buffer-constraints Kai Vehmanen
2007-10-17 10:23     ` Fabien Chevalier
2007-10-17 14:36       ` Kai.Vehmanen
  -- strict thread matches above, loose matches on Subject: below --
2007-10-17 16:31 [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints Kai.Vehmanen
2007-10-18  8:42 ` Johan Hedberg
2007-10-18 10:08 ` Fabien Chevalier
2007-10-18 17:33   ` Jim Carter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).