Re: [pulseaudio-discuss] [PATCH] bluetooth: Fix rendering a2dp data
Hi Luiz,
Op 13-12-10 09:55, Luiz Augusto von Dentz schreef:
Hi,
On Sat, Dec 11, 2010 at 1:05 AM, Maarten Lankhorst
m.b.lankho...@gmail.com wrote:
makes my android phone slightly happier
---
src/modules/bluetooth/module-bluetooth-device.c | 11 ---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-device.c
b/src/modules/bluetooth/module-bluetooth-device.c
index 6d31c1e..8664001 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -1387,7 +1387,7 @@ static int a2dp_process_push(struct userdata *u) {
pa_assert(u-source);
pa_assert(u-read_smoother);
-memchunk.memblock = pa_memblock_new(u-core-mempool, u-block_size);
+memchunk.memblock = pa_memblock_new(u-core-mempool, u-block_size * 2);
memchunk.index = memchunk.length = 0;
Im not sure how this would help, we decode sbc frame by frame so
having twice as much memory doesn't really make any sense, there could
be that we should decode the entire buffer read, but that would take
much more memory. What could be the real problem is that the source is
changing the bitpool but that shouldn't be a problem since the
block_size is supposed to be calculated from maximum bitpool range so
the buffer will always be big enough.
Well it seems the block_size is not fixed on my phone, sometimes it is 1
sample more, sometimes 2, the net effect is 1 or 2 frames stay undecoded
leading to massive underruns.
I don't think u-block_size has to be fixed per se, a2dp-code_size
seems to be though.
for (;;) {
@@ -1442,7 +1442,8 @@ static int a2dp_process_push(struct userdata *u) {
to_decode = l - sizeof(*header) - sizeof(*payload);
d = pa_memblock_acquire(memchunk.memblock);
-to_write = memchunk.length = pa_memblock_get_length(memchunk.memblock);
+to_write = pa_memblock_get_length(memchunk.memblock);
+memchunk.length = 0;
while (PA_LIKELY(to_decode 0 to_write 0)) {
size_t written;
@@ -1464,7 +1465,7 @@ static int a2dp_process_push(struct userdata *u) {
/* pa_log_debug(SBC: frame_length: %lu; codesize: %lu, (unsigned
long) a2dp-frame_length, (unsigned long) a2dp-codesize); */
pa_assert_fp((size_t) decoded= to_decode);
-pa_assert_fp((size_t) decoded == a2dp-frame_length);
+pa_assert_fp((size_t) decoded= a2dp-frame_length);
This is the real problem, either we do this if the bitpool is changing
or we have to call sbc_reinit in each frame.
Well, a2dp-frame_length was calculated based on the maximum bitpool
size. The frames decoded have a smaller bitpool size. a2dp-codesize is
still the same though, so I don't think this change matters much.
pa_assert_fp((size_t) written= to_write);
pa_assert_fp((size_t) written == a2dp-codesize);
@@ -1474,10 +1475,14 @@ static int a2dp_process_push(struct userdata *u) {
d = (uint8_t*) d + written;
to_write -= written;
+memchunk.length += written;
frame_count++;
}
+if (to_decode)
+pa_log_error(SBC: %lu bytes not decoded\n, to_decode);
+
pa_memblock_release(memchunk.memblock);
Actually I think we should be solving this in libsbc, both sbc_encode
and sbc_decode should be checking if the bitpool has changed and
reinit automatically, otherwise we gonna get performance penalty doing
sbc_reinit for every frame.
I don't think the bitpool changed, the code just assumes the maximum
bitpool size, while my phone sends data at a lower one.
Of course I still don't know how to handle packet loss correctly, which
is easy to create by moving phone out of the range of my bluetooth device.
Cheers,
Maarten
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulseaudio for two sessions at the same time
On Domingo 12 Diciembre 2010 20:17:47 Colin Guthrie escribió: 'Twas brillig, and Noel David Torres Taño at 10/12/10 08:48 did gyre and gimble: On Jueves 09 Diciembre 2010 10:06:00 Colin Guthrie escribió: HTHs It helped :) Now we both can have half-time sound, and that is better than one getting no sound :) How can this be accomplished using a server-wide PA? (Note: yes, I know WhatIsWrongWithSystemMode but it is not true that either my or my wife's sessions are always open, nor that it is always the same session the first to be opened, so, strage as it can be, I need the equivalent solution using a server wide instance. many many thanks for PA itself and this answer Yeah there are always situations where people want to do things slightly differently and the disadvantages of System Wide are not always problematic, so as always YMMV. For yourself, perhaps system wide is the best option. Take Care. Col Yes, thanks, but... How to do it? Is it just to set a system-wide server or is there something to do in its config and/or in both $HOMEs and/or UNIX groups? Thx again er Envite ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulseaudio for two sessions at the same time
'Twas brillig, and Noel David Torres Taño at 14/12/10 16:41 did gyre and gimble: On Martes 14 Diciembre 2010 12:22:17 Michał Sawicz escribió: Dnia 2010-12-14, wto o godzinie 12:11 +, Noel David Torres Taño pisze: Yes, thanks, but... How to do it? Is it just to set a system-wide server or is there something to do in its config and/or in both $HOMEs and/or UNIX groups? See http://www.pulseaudio.org/wiki/SystemWideInstance First google hit for 'pulseaudio system wide' That does NOT answer my question about audio group access, config on the $HOMEs and the like. Thanks, but I learned to read when I was quite young. No need to be rude to people who post helpful links. That page only says that both users should be on pulse-access group, but does not say if, for example, .pulse-cookie must be the same, or if .pulse/client.conf has to be tweaked. It also doesn't say how much RAM you need nor whether it works if the day has the letter 'T' in it... because they are not relevant :p The fact you know about these additional parts of PA is actually working against you here - you know too much and it means you try to push them into a situation where they are not mentioned because they are not important! But cheekyness aside, the cookie is not used in system wide mode, the access is controlled via the membership of the pulse-access group. There should be no tweaks needed on .pulse/client.conf (the file should not even exist unless something manual has been done). HTHs Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulseaudio for two sessions at the same time
On Martes 14 Diciembre 2010 17:20:37 Colin Guthrie escribió: 'Twas brillig, and Noel David Torres Taño at 14/12/10 16:41 did gyre and gimble: On Martes 14 Diciembre 2010 12:22:17 Michał Sawicz escribió: Dnia 2010-12-14, wto o godzinie 12:11 +, Noel David Torres Taño pisze: Yes, thanks, but... How to do it? Is it just to set a system-wide server or is there something to do in its config and/or in both $HOMEs and/or UNIX groups? See http://www.pulseaudio.org/wiki/SystemWideInstance First google hit for 'pulseaudio system wide' That does NOT answer my question about audio group access, config on the $HOMEs and the like. Thanks, but I learned to read when I was quite young. No need to be rude to people who post helpful links. True, my fault (even when not helpful for me). That page only says that both users should be on pulse-access group, but does not say if, for example, .pulse-cookie must be the same, or if .pulse/client.conf has to be tweaked. It also doesn't say how much RAM you need nor whether it works if the day has the letter 'T' in it... because they are not relevant :p The fact you know about these additional parts of PA is actually working against you here - you know too much and it means you try to push them into a situation where they are not mentioned because they are not important! But cheekyness aside, the cookie is not used in system wide mode, the access is controlled via the membership of the pulse-access group. Ok. Tried but... (continues below) There should be no tweaks needed on .pulse/client.conf (the file should not even exist unless something manual has been done). Ok too HTHs Col (continues here) I've deleted .pulse and .pulse-cookie to be sure, removed both accounts from audio group and set them both in pulse-access group. Still first session opened gets sound (even when changing sessions, so ConsoleKit is not in the way) and secondly opened one gets no sound. May it be that extra PA processes are opened? How to avoid that and get each session connect to the system-wide PA? $ ps -A u | grep pulse pulse12594 0.0 0.1 273444 5132 ?Ssl 16:52 0:01 /usr/bin/pulseaudio --system --daemonize --high-priority --log-target=syslog --disallow-module-loading=1 user115671 0.0 0.1 218140 5144 ?Ssl 17:41 0:00 /usr/bin/pulseaudio --start user216193 0.4 0.0 335412 3868 ?Ssl 17:45 0:00 /usr/bin/pulseaudio --start user1 16306 0.0 0.0 9612 892 pts/1S+ 17:46 0:00 grep pulse Thanks Noel er Envite ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth: Fix rendering a2dp data
Hi,
On Tue, Dec 14, 2010 at 11:10 AM, Maarten Lankhorst
m.b.lankho...@gmail.com wrote:
Hi Luiz,
Op 13-12-10 09:55, Luiz Augusto von Dentz schreef:
Hi,
On Sat, Dec 11, 2010 at 1:05 AM, Maarten Lankhorst
m.b.lankho...@gmail.com wrote:
makes my android phone slightly happier
---
src/modules/bluetooth/module-bluetooth-device.c | 11 ---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-device.c
b/src/modules/bluetooth/module-bluetooth-device.c
index 6d31c1e..8664001 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -1387,7 +1387,7 @@ static int a2dp_process_push(struct userdata *u) {
pa_assert(u-source);
pa_assert(u-read_smoother);
- memchunk.memblock = pa_memblock_new(u-core-mempool,
u-block_size);
+ memchunk.memblock = pa_memblock_new(u-core-mempool, u-block_size
* 2);
memchunk.index = memchunk.length = 0;
Im not sure how this would help, we decode sbc frame by frame so
having twice as much memory doesn't really make any sense, there could
be that we should decode the entire buffer read, but that would take
much more memory. What could be the real problem is that the source is
changing the bitpool but that shouldn't be a problem since the
block_size is supposed to be calculated from maximum bitpool range so
the buffer will always be big enough.
Well it seems the block_size is not fixed on my phone, sometimes it is 1
sample more, sometimes 2, the net effect is 1 or 2 frames stay undecoded
leading to massive underruns.
I don't think u-block_size has to be fixed per se, a2dp-code_size seems to
be though.
Again if we want to be able to support devices that changes bitpool we
have to recalculate the block_size on every frame (if bitpool has
really changed), doubling the buffer is not the right fix, sorry.
for (;;) {
@@ -1442,7 +1442,8 @@ static int a2dp_process_push(struct userdata *u) {
to_decode = l - sizeof(*header) - sizeof(*payload);
d = pa_memblock_acquire(memchunk.memblock);
- to_write = memchunk.length =
pa_memblock_get_length(memchunk.memblock);
+ to_write = pa_memblock_get_length(memchunk.memblock);
+ memchunk.length = 0;
while (PA_LIKELY(to_decode 0 to_write 0)) {
size_t written;
@@ -1464,7 +1465,7 @@ static int a2dp_process_push(struct userdata *u) {
/* pa_log_debug(SBC: frame_length: %lu; codesize: %lu,
(unsigned long) a2dp-frame_length, (unsigned long) a2dp-codesize); */
pa_assert_fp((size_t) decoded= to_decode);
- pa_assert_fp((size_t) decoded == a2dp-frame_length);
+ pa_assert_fp((size_t) decoded= a2dp-frame_length);
This is the real problem, either we do this if the bitpool is changing
or we have to call sbc_reinit in each frame.
Well, a2dp-frame_length was calculated based on the maximum bitpool size.
The frames decoded have a smaller bitpool size. a2dp-codesize is still the
same though, so I don't think this change matters much.
It was you that suggested this changes, so when you say it doesn't
matter it makes me wonder why this is part of the patch then, Btw,
changing bitpool changes the frame length and it actually does assert,
I tried it myself with this patch to source:
http://gitorious.org/pulseaudio/mainline/commit/9b938f8f8f40772e626b3e71fa4d7b7cbd5de5e7
pa_assert_fp((size_t) written= to_write);
pa_assert_fp((size_t) written == a2dp-codesize);
@@ -1474,10 +1475,14 @@ static int a2dp_process_push(struct userdata *u)
{
d = (uint8_t*) d + written;
to_write -= written;
+ memchunk.length += written;
frame_count++;
}
+ if (to_decode)
+ pa_log_error(SBC: %lu bytes not decoded\n, to_decode);
+
pa_memblock_release(memchunk.memblock);
Actually I think we should be solving this in libsbc, both sbc_encode
and sbc_decode should be checking if the bitpool has changed and
reinit automatically, otherwise we gonna get performance penalty doing
sbc_reinit for every frame.
I don't think the bitpool changed, the code just assumes the maximum bitpool
size, while my phone sends data at a lower one.
Well what other bitpool would you use instead? If we don't have any
frame to parse obviously we need to start with the biggest possible
one, it the only possible way to avoid decode errors due to too small
buffer to decode.
--
Luiz Augusto von Dentz
Computer Engineer
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] *reminder* Re: [PATCH 0/3] Fighting rewinds
On 2010-12-09 14:54, David Henningsson wrote: As some of you have seen on IRC, I spent the some of the last week fighting rewinds. An never-ending stream of rewinds seems to be one of the most common reasons PulseAudio crashes or produces crackling/stuttering output, so there is a strong incentive to fix it. However, the problem is quite complex and there does not seem to be one perfect fix, it's more of an optimisation problem. GStreamer in particular sends out many small data packages, and PulseAudio does not handle that very well. When the sink-input buffer is empty, going from there to a full buffer is an uphill battle in terms of CPU power, as PulseAudio will try to rewind (at RT priority!) and mix the new data into it; all with the very best intent, but the end result of taking CPU power away from the client. GStreamer has at least two problems: * It starts/uncorks the stream when the buffer is empty (this might have been fixed by Wim a day or two ago) * It sends out very small packages (c:a 1 - 4K). Here are three patches trying to help out on the PA side. * The first one is a relatively simple optimisation than can cut the rewinds in half by allowing both a seek and a post to be merged into one rewind. * The second one builds on the first, and adds the possibility for several data packages to share a rewind by checking if there are more data packets in queue before doing a rewind. * The third makes sure that after an underrun, there is a little headroom before asking for a rewind. Hopefully this will improve the situation for at least a few users. The idea is to let you do initial comment and review, then make a package and ask some Ubuntu users to test it. After that I'll report back and we can consider inclusion into stable-queue. I never saw any feedback on the patches, is this because of lack of time, interest (i e you're not affected by the problem anyway), or knowledge (i e you don't know how the stuff works), or something else? For users running Ubuntu Maverick, there is a ppa for easy testing here: https://launchpad.net/~diwic/+archive/fighting-rewinds It also includes two fixes on the gstreamer side. -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
