* [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-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
1 sibling, 0 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2007-10-14 3:36 UTC (permalink / raw)
To: BlueZ development
Hi Kai,
On 10/12/07, Kai Vehmanen <kai.vehmanen@nokia.com> wrote:
> 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)
All seems to be good improvements, perhaps brad can comment
about it because he was working with pulse before.
-- =
Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o
-------------------------------------------------------------------------
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
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
1 sibling, 1 reply; 10+ messages in thread
From: Fabien Chevalier @ 2007-10-14 11:19 UTC (permalink / raw)
To: BlueZ development
Hello Kai,
Please find a few comments below
> 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).
Could you provide some details on the problems you experienced, as well
the hardware platform you experienced them on ?
> 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;
This part of the code is useless, you don't need to set
SND_PCM_IOPLUG_HW_BUFFER_BYTES if you set SND_PCM_IOPLUG_HW_PERIODS, you
just have to choose one of the two alternatives :-)
>
> + /* 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;
>
>
Apart from that i'd say this is a foot step in the right direction :-)
Cheers,
Fabien
-------------------------------------------------------------------------
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils -fix-a2dp-buffer-constraints
2007-10-14 11:19 ` Fabien Chevalier
@ 2007-10-15 10:54 ` Kai Vehmanen
2007-10-17 10:23 ` Fabien Chevalier
0 siblings, 1 reply; 10+ messages in thread
From: Kai Vehmanen @ 2007-10-15 10:54 UTC (permalink / raw)
To: 'BlueZ development'
Hi,
On 14 Oct 2007, Fabien Chevalier wrote:
>Could you provide some details on the problems you
>experienced, as well the hardware platform you experienced them on ?
I've been testing with a low spec GNU/Debian desktop, and the problem has
been too high CPU load leading to a continuous stream of XRUNs in
the pcm_bluetooth.c code (i.e. no audio output at all). With the patch,
on the same exact system, playback is flawless. And the problem only
occurs with apps that always poll/select before writing to the device (i.e.
not aplay). Relaxing the buffering constraints allows apps to feed audio
to A2DP with less context switches and less system calls (bigger buffer
fragments at a time basicly).
>> - /* 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,
[...]
>> + /* 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;
>
>
>This part of the code is useless, you don't need to set
>SND_PCM_IOPLUG_HW_BUFFER_BYTES if you set
>SND_PCM_IOPLUG_HW_PERIODS, you just have to choose one of the
>two alternatives :-)
But if we'd drop setting mimax for BUFFER_BYTES, the application could
request for 64 periods of size 'MAX_BUFFER_SIZE / 2' -> 524288 bytes.
Of course we could fix this by limiting the max period size, but then
the application couldn't use a buffering setup of two max-sized periods
(good for robust playback). If we'd limit the max count of periods,
application couldn't choose a buffering setup of lots of small
buffer fragments (good for robust recording with minimal latency).
So defining a minmax range for all three params gives more
flexibility without requiring any added logic to the plugin
code.
--
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils -fix-a2dp-buffer-constraints
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
0 siblings, 1 reply; 10+ messages in thread
From: Fabien Chevalier @ 2007-10-17 10:23 UTC (permalink / raw)
To: BlueZ development
Hello Kai,
> I've been testing with a low spec GNU/Debian desktop, and the problem has
> been too high CPU load leading to a continuous stream of XRUNs in
> the pcm_bluetooth.c code (i.e. no audio output at all). With the patch,
> on the same exact system, playback is flawless. And the problem only
> occurs with apps that always poll/select before writing to the device (i.e.
> not aplay). Relaxing the buffering constraints allows apps to feed audio
> to A2DP with less context switches and less system calls (bigger buffer
> fragments at a time basicly).
I'm quite interested in optimizations for low end ARM boxes :-)
Could you provide CPU details and the player you used to performed your
testing ? I'd like to be able to reproduce those on my Neo1973 box :-)
>
>>> - /* 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,
> [...]
>>> + /* 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;
>>
>> This part of the code is useless, you don't need to set
>> SND_PCM_IOPLUG_HW_BUFFER_BYTES if you set
>> SND_PCM_IOPLUG_HW_PERIODS, you just have to choose one of the
>> two alternatives :-)
>
> But if we'd drop setting mimax for BUFFER_BYTES, the application could
> request for 64 periods of size 'MAX_BUFFER_SIZE / 2' -> 524288 bytes.
> Of course we could fix this by limiting the max period size, but then
> the application couldn't use a buffering setup of two max-sized periods
> (good for robust playback). If we'd limit the max count of periods,
> application couldn't choose a buffering setup of lots of small
> buffer fragments (good for robust recording with minimal latency).
>
> So defining a minmax range for all three params gives more
> flexibility without requiring any added logic to the plugin
> code.
That's definately a good answer :-).
Maybe we should drop SND_PCM_IOPLUG_HW_PERIODS and keep
SND_PCM_IOPLUG_HW_BUFFER_BYTES only then ?
Cheers,
Fabien
-------------------------------------------------------------------------
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils -fix-a2dp-buffer-constraints
2007-10-17 10:23 ` Fabien Chevalier
@ 2007-10-17 14:36 ` Kai.Vehmanen
0 siblings, 0 replies; 10+ messages in thread
From: Kai.Vehmanen @ 2007-10-17 14:36 UTC (permalink / raw)
To: bluez-devel
Hi Fabien,
On 17 Oct 2007, Fabien Chevalier wrote:
>I'm quite interested in optimizations for low end ARM boxes
>:-) Could you provide CPU details and the player you used to
>performed your testing ? I'd like to be able to reproduce
>those on my Neo1973 box :-)
:) I'm using an x86 system myself (2.66GHz P4). Just try running
pulseaudio plus for instance "aplay -D pulse foo.wav" (or paplay) and
you should see similar results. With the above system and
unpatched 3.19, I get 2x number of gettimeofday()s, plus audio play
out is as described in my earlier mails (choppy, so not really working),
compared to running the same test with patched 3.19. And this with
default settings (i.e. not optimizing for a larger period size which
would further minimize the system call load).
Anyways, this is definitely not specific to any hardware architecture
nor CPU processing power. You can simply calculate the number of
unnecessary
system calls and context switches. Whether those are enough to degrade
audio quality is another question, but nevertheless you are wasting CPU
cycles, using more electricity, accelerating global warming, etc, etc :)
>Maybe we should drop SND_PCM_IOPLUG_HW_PERIODS and keep
>SND_PCM_IOPLUG_HW_BUFFER_BYTES only then ?
Hmm, that should be ok. I guess alsa-lib won't allow applications
to use only one period anyways, so not specifying that in
pcm_bluetooth.c
should be ok.
--
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
2007-10-18 10:08 ` Fabien Chevalier
@ 2007-10-18 17:33 ` Jim Carter
0 siblings, 0 replies; 10+ messages in thread
From: Jim Carter @ 2007-10-18 17:33 UTC (permalink / raw)
To: BlueZ development
On Thu, 18 Oct 2007, Fabien Chevalier wrote:
> > On 12 Oct 2007, Kai Vehmanen wrote:
> > 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.
>
> Well, application can prefill more data, the data gets send immediately
> to the headset, and buffering will happen at headset side. The issue is
> if the buffer size as seen by the application is bigger than the real
> headset buffer size, and the application waits for the buffer to be
> almost empty before to refill it, then audio cuts might happen. :-(
I remember a month or two back, someone said that the A2DP profile was weak
on flow control, requiring the source (ALSA) to send data at an even rate.
If the headset's buffer is full can it cork the stream? Or will the
additional data just overwrite data already at the headset?
With bluez-utils-3.19 (3.20 is latest) I'm having consistently good results
sending to the Bluetooth ALSA device, with neither silent gaps nor lost
chunks. But I'm seeing corruption when going through PulseAudio, which is
attributed to an unknown formatting or parameter issue and is being dealt
with on their mailing list.
James F. Carter Voice 310 825 2897 FAX 310 206 6673
UCLA-Mathnet; 6115 MSA; 405 Hilgard Ave.; Los Angeles, CA, USA 90095-1555
Email: jimc@math.ucla.edu http://www.math.ucla.edu/~jimc (q.v. for PGP key)
-------------------------------------------------------------------------
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
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
1 sibling, 1 reply; 10+ messages in thread
From: Fabien Chevalier @ 2007-10-18 10:08 UTC (permalink / raw)
To: BlueZ development
Hello Kai,
Please find below some comments.
> 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.
No this is not the case. :-)
I implemented it that way so that not to add another buffering layer.
As per ALSA semantic, the buffer is found in the remote device, but we
have no way of knowing how big it is. :-(
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.
Well, application can prefill more data, the data gets send immediately
to the headset, and buffering will happen at headset side. The issue is
if the buffer size as seen by the application is bigger than the real
headset buffer size, and the application waits for the buffer to be
almost empty before to refill it, then audio cuts might happen. :-(
>
> 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 ;)).
Even if your assumptions were wrong, this part of the code is still very
much work in progress, and didn't receive any tuning at all.
So Johan tested your patch, didn't noticed any regression, so we decided
to apply it anyway. :-)
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
>
That is basically another alternative implementation. For now i'm still
not convinced this is the right way to go ;-). This will work but add
another layer of buffering thus a much increased latency :-(
> But yeah, that would seem to be a much bigger task to implement
Yes indeed :-)
(and
> something you may have already discussed earlier).
Well, yes and no. a2dpd implementation was written the way you though we
were working. Audio service ALSA plugin is written another way, that is
known to work. However we didn't tune the implementation to improve its
behaviours regarding the number of application wakeups it generates, nor
the buffer sizes or period sizes :-(
I think you were the first one to run those kind of tests, that's why
i'm still interested in you giving us the details of your tests setup :-)
Cheers,
Fabien
-------------------------------------------------------------------------
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
* Re: [Bluez-devel] PATCH1/2: bluez-utils - fix-a2dp-buffer-constraints
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
1 sibling, 0 replies; 10+ messages in thread
From: Johan Hedberg @ 2007-10-18 8:42 UTC (permalink / raw)
To: BlueZ development
Hi Kai,
Both of your patches are in CVS now since they don't at least seem to
do any harm. Any further patches are very welcome since you seemed to
have a quite good idea of things that need improvement :)
Johan
-------------------------------------------------------------------------
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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.