Re: [pulseaudio-discuss] [PATCH 4/4] alsa-mixer: Add force-hw-volume flag to alsa profile sets
On Sat, 09 Apr 2011 20:20:40 +0200, David Henningsson wrote: > On 2011-04-08 17:18, Colin Guthrie wrote: >> 'Twas brillig, and o...@iki.fi at 08/04/11 15:18 did gyre and gimble: >>> From: Jyri Sarha >>> >>> Before this patch, if any of the paths in a path set do not >>> support HW volume then the HW volume is disabled for the whole >>> set. In some cases this is a bit drastic measure. For instance, >>> if all but one of the paths support HW volume and dB there no >>> problem to pretend that we have HW volume for the whole set. The >>> path without any mixers to control will just always return 0 dB >>> and the rest is handled by SW volume. This patch adds a flag to >>> the mapping section of profile set file to enables this behavior. >> >> David, this sounds similar to your USB Headset issue from a couple days >> ago... or am I just reading too much into the description? > > Sort of - it just feels like neither of us has tried to do the right > thing so far - I added a workaround/quirk for a few devices, and this > patch adds a setting to turn something on and off. > > I'd like it to "just work". > > Or put in another way - what's the recommended default setting of this > new parameter, and why? > Target for my patch was to be non intrusive, but if you agree I can easily change my patch to always behave like force-hw-volume flag was set to true and remove the flag. It is even easier just to change the default for the flag. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 3/4] core: no rewinding on volume change if the sink does not support it.
On Fri, 8 Apr 2011, Colin Guthrie wrote: 'Twas brillig, and o...@iki.fi at 08/04/11 15:18 did gyre and gimble: From: Jyri Sarha ... Would it not be possible to move this check into the function itself and just make it a noop? The reason for this patch in the first place was the load spikes when a user took his time to adjust volume while listening music. This fixed the problem and did not give it any further consideration, but I can now see that it is not the most complete fix. It appears to me that if "flush" parameter of pa_sink_input_request_rewind() call is not true (IOW, reuse the rewound samples) then we could ignore the the whole request if sink does not support rewinding. However, the logic of different boolean parameters for pa_sink_input_request_rewind() is not completely clear to me. What do you think? I appreciate there would be FCO but it would reduce the likelihood of this check being missed on some other calls... unless that is desirable sometimes? As far as I can see the only cases we want to rewind, if sink does not support it, is when we want to throw away the already queued samples and not just process them again with new SW volume etc. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] call-state-tracker: New component.
On Tue, 5 Apr 2011, Maarten Bosmans wrote: What are the users of this state tracker? Is it only some out-of-tree Meego modules? Are you planning to submit those too? At least a snippet of how it should be used would be nice. Hi Maarten, To answer your question about upstreaming the "meego modules" or part of them, I can only say that we had a plan to upstream the applicable parts. However, now with this new Nokia strategy it is very hard to predict what is the future of those modules. The biggest reason why we can not just do it now is how product specific the code currently is. The driving force have been developing for a specific product and not so much trying to make a generic implementations. On the other hand with Intel/Meego cooperation we were starting to take the steps to the right direction. Anyway, in this new situation we just plan to upstream everything that is generic enough. For product specific modules we plan to open source as much as possible in the out of source tree compiled modules [1]. Best regards, Jyri Sarha [1] The source code for the pulseaudio modules mentioned can be found here: http://meego.gitorious.org/maemo-multimedia/pulseaudio-modules-meego The heavily patched pulseaudio tree we are using can be found here: http://meego.gitorious.org/maemo-multimedia/pulseaudio Almost all the patches are now part of the pulseaudio upstream. ps. The above repositories are currently not up to date, but we'll update them soon gain. ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-sink: take base volume into account when applying hw volume
On Wed, 1 Dec 2010, David Henningsson wrote: On 2010-12-01 11:31, Jyri Sarha wrote: Am currently working on a problem caused by safety-margin not taken into account when rewinding. When I get this fixed I'll add a fix for double use bug too. Hmm, for tsched or non-tsched? I think we merged a patch related to this from pl bossart a while ago. Sorry, should have been more specific. I was talking about sync-volume-safety-margin, not rewind_safeguard related to tsched mode rewinding. However, the two things are closely related in this problem case. When DMA buffer is rewound the sync-volume events should rewound at the same time. This works already now. However the volume event rewinding is not modifying the new expiry time with sync-volume-safety-margin depending on whether we are changing the volume up or down. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa-sink: take base volume into account when applying hw volume
On Tue, 30 Nov 2010, Colin Guthrie wrote: Any comments on this one Jyri? It seems add code to sink_write_volume_cb() to mirror the normal method sink_set_volume_cb() so looks OK to me but not followed the path through to see if this is supposed to be factored in already in some other way... Your thoughts would be appreciated. Yes, The base volume stuff appeared some time between the first version of sync volume and my upstream rebase. AFAIK, the fix is correct. However, there is still the double use of string bug (originally coming from me, but it looks quite different now). The bug is not serious, the print just is not very informative. Am currently working on a problem caused by safety-margin not taken into account when rewinding. When I get this fixed I'll add a fix for double use bug too. Cheers, Jyri Col 'Twas brillig, and Juho Hämäläinen at 25/11/10 13:15 did gyre and gimble: Currently if sink base volume differs from 0dB and sync-volume is used, wrong volume values are written to hw. This patch fixes that. Signed-off-by: Juho Hämäläinen --- src/modules/alsa/alsa-sink.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss -- 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 ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] core: New LIFO style flist implementation
On Fri, 20 Aug 2010, Colin Guthrie wrote:
'Twas brillig, and o...@iki.fi at 20/08/10 14:24 did gyre and gimble:
From: Jyri Sarha
It seems the mail archive at 0pointer does not show attachments of type
"Text/X-PATCH", so here this goes again.
It's OK, it came through on gmane and no doubt to list subscribers.
Lennart is probably the best person to review this as I don't feel
confident accepting it without fully grokking the reasons for the
original implem vs. this one.
As he's currently distracted by systemd it may take him a while to
reply, but he will defo get to it in due course :)
I browsed the pulseaudio-discuss mail archives and noticed that this issue
was left hanging in the air also on my side. At least I could not find the
the latest version of this patch anywhere. So here it comes just in case
Lennart would have time to take a look at it.
To give some extra motivation, with this patch you can solve issues like
this :) : https://bugs.maemo.org/show_bug.cgi?id=7190
Cheers,
Jyri
--
From 12563ed356f3bb81f99aa32ba45337cfdf427b50 Mon Sep 17 00:00:00 2001
From: Jyri Sarha
Date: Mon, 30 Nov 2009 16:55:27 +0200
Subject: [PATCH] core: New LIFO style flist implementation v2.2
The old free list implementation used objects in FIFO style. This is
bad because it tries keep all the objects ever used alive and in
memory. This minimizes the changes that an allocated object is already
in cache. When there is shortage of physical memory this may also
increase change that newly allocated object is swapped out. LIFO
(e.g. stack) style free list should help these issues. Like the old
one the new implementation is also lock free. This version (v2.1) of
the patch has a potential weakness fixed. The previous version (2.0)
did segfault when popping from empty flist, this does not.
---
src/pulsecore/flist.c | 218 ++--
1 files changed, 64 insertions(+), 154 deletions(-)
diff --git a/src/pulsecore/flist.c b/src/pulsecore/flist.c
index 7e5ee24..e728d30 100644
--- a/src/pulsecore/flist.c
+++ b/src/pulsecore/flist.c
@@ -1,7 +1,9 @@
/***
This file is part of PulseAudio.
- Copyright 2006-2008 Lennart Poettering
+ Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+
+ Contact: Jyri Sarha
PulseAudio is free software; you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as
@@ -34,198 +36,106 @@
#include "flist.h"
-/* Algorithm is not perfect, in a few corner cases it will fail to pop
- * from the flist although it isn't empty, and fail to push into the
- * flist, although it isn't full.
- *
- * We keep a fixed size array of entries, each item is an atomic
- * pointer.
- *
- * To accelerate finding of used/unused cells we maintain a read and a
- * write index which is used like a ring buffer. After each push we
- * increase the write index and after each pop we increase the read
- * index.
- *
- * The indexes are incremented atomically and are never truncated to
- * the buffer size. Instead we assume that the buffer size is a power
- * of two and that the truncation can thus be done by applying a
- * simple AND on read.
- *
- * To make sure that we do not look for empty cells indefinitely we
- * maintain a length value which stores the number of used cells. From
- * this value the number of unused cells is easily calculated. Please
- * note that the length value is not updated atomically with the read
- * and write index and might thus be a few cells off the real
- * value. To deal with this we always look for N_EXTRA_SCAN extra
- * cells when pushing/popping entries.
- *
- * It might make sense to replace this implementation with a link list
- * stack or queue, which however requires DCAS to be simple. Patches
- * welcome.
- *
- * Please note that this algorithm is home grown.*/
-
#define FLIST_SIZE 128
-#define N_EXTRA_SCAN 3
-/* For debugging purposes we can define _Y to put and extra thread
- * yield between each operation. */
+/* Lock free single linked list element. */
+struct pa_flist_elem {
+pa_atomic_ptr_t next;
+pa_atomic_ptr_t ptr;
+};
-#ifdef PROFILE
-#define _Y pa_thread_yield()
-#else
-#define _Y do { } while(0)
-#endif
+typedef struct pa_flist_elem pa_flist_elem;
struct pa_flist {
unsigned size;
-pa_atomic_t length;
-pa_atomic_t read_idx;
-pa_atomic_t write_idx;
+/* Stack that contains pointers stored into free list */
+pa_atomic_ptr_t stored;
+/* Stack that contains empty list elements */
+pa_atomic_ptr_t empty;
+pa_flist_elem table[0];
};
-#define PA_FLIST_CELLS(x) ((pa_atomic_ptr_t*) ((uint8_t*) (x) +
PA_ALIGN(sizeof(struct pa_flist
+/* Lock free pop from linked list stack */
+static pa_flist_elem *stack_pop(pa_atomic_ptr_t *list) {
+pa_flist_elem *poped;
+pa_assert(
Re: [pulseaudio-discuss] Compiler optimisation dependency?
On Sun, 24 Oct 2010, tarantism wrote: I'm building pulse under Scratchbox for Maemo 5 then copying the module-sine.so file to Nokia N900 to test. If I use compiler options -O0 or -O1, it works fine. If I use -O2 or -O3 I just get a sqwarking noise. Any ideas? How does the precompiled module-sine.so from Maemo 5 work for you (it is in pulseaudio-module-extra package)? Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/5] core: Add infrastructure for synchronizing HW and SW volume changes
> Hi,
>
> Review below:
>
> 'Twas brillig, and o...@iki.fi at 13/10/10 17:40 did gyre and gimble:
>
>> +PA_SINK_SYNC_VOLUME = 0x0200U,
>> +/**< The HW volume changes are syncronized with SW volume.
>> + * \since X.X.XX */
>> +
>> } pa_sink_flags_t;
>
> Can you put 0.9.22 here? If needed I'll do a global find and replace on
> any \since 0.9.22's if/when we release that version from stable queue
> instead, but I'll almost certainly forget to fix up x.x.xx replacements.
>
Sure.
>
>> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
>> index ff4cc17..0f2af4f 100644
>> --- a/src/pulsecore/sink.c
>> +++ b/src/pulsecore/sink.c
>
>
>> +struct sink_message_set_port {
>> +pa_device_port *port;
>> +int ret;
>> +};
>> +
>
> I'll refer to this below[1]
>
>> @@ -1459,7 +1493,8 @@ void pa_sink_set_volume(
>> * apply one to s->soft_volume */
>>
>> pa_cvolume_reset(&s->soft_volume, s->sample_spec.channels);
>> -s->set_volume(s);
>> +if (!(s->flags & PA_SINK_SYNC_VOLUME))
>> +s->set_volume(s);
>>
>> } else
>> /* If we have no function set_volume(), then the soft volume
>
> Hmm, should this "if" have an "else" that sets send_msg to TRUE
> (send_msg is used just below where the context ends) otherwise if
> send_msg is set to false, then the message will not be pushed to the
> asyncq and thus the set_volume() function will never be called in
> pa_sink_process_msg()[2]
>
It looks like you are right. I am only wondering how the code
still seems to work. If I understand correctly the same problem
is there also without sync-volume if sink updates soft volume
within set_volume(). The updated soft volume would not be
propagated to IO-thread.
The problem would be there also if we were using soft-volume only,
but then we would not be using flat volume and there would be no
pa_sink_set_volume() calls with send_msg=FALSE.
This all seems clear in theory. However, it does not work like
that when I try to produce the problem by updating a sink-input
volume and check that HW volume remains unchanged. Everything
just seems to work as it should. I'll offer virtual beer to one
who can explain why the code still works :).
Anyway I did change the code as you proposed and tested it and
the code still work.
>> @@ -1474,18 +1509,22 @@ void pa_sink_set_volume(
>> pa_subscription_post(s->core,
>> PA_SUBSCRIPTION_EVENT_SINK|PA_SUBSCRIPTION_EVENT_CHANGE,
>> s->index);
>> }
>>
>> -/* Called from main thread. Only to be called by sink implementor */
>> +/* Called from the io thread if sync volume is used, otherwise from the
>> main thread.
>> + * Only to be called by sink implementor */
>> void pa_sink_set_soft_volume(pa_sink *s, const pa_cvolume *volume) {
>> pa_sink_assert_ref(s);
>> -pa_assert_ctl_context();
>> +if (s->flags & PA_SINK_SYNC_VOLUME)
>> +pa_sink_assert_io_context(s);
>> +else
>> +pa_assert_ctl_context();
>>
>> if (!volume)
>> pa_cvolume_reset(&s->soft_volume, s->sample_spec.channels);
>> else
>> s->soft_volume = *volume;
>>
>> -if (PA_SINK_IS_LINKED(s->state))
>> -pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
>> PA_SINK_MESSAGE_SET_VOLUME, NULL, 0, NULL) == 0);
>> +if (PA_SINK_IS_LINKED(s->state) && !(s->flags &
>> PA_SINK_SYNC_VOLUME))
>> +pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
>> PA_SINK_MESSAGE_SET_SOFT_VOLUME, NULL, 0, NULL) == 0);
>> else
>> s->thread_info.soft_volume = s->soft_volume;
>> }
>
> This is a little concerning. If I have a sink (e.g. an roap-sink) whcih
> does not support PA_SINK_SYNC_VOLUME flag, then you are now sending me a
> different message to the one I got before.
>
> While there is nothing in the tree that does this, there may be external
> uses of this in private trees and I don't think there is any need to
> change the semantics of PA_SINK_MESSAGE_SET_VOLUME.
>
> Would it be better to instead define:
> PA_SINK_MESSAGE_SET_VOLUME_SYNCED to go before
> PA_SINK_MESSAGE_SET_VOLUME, and not change the message here and instead
> change the o->process_msg() calls that use PA_SINK_MESSAGE_SET_VOLUME to
> use PA_SINK_MESSAGE_SET_VOLUME_SYNCED?
>
> I think this would be semantically the same with regards to your
> requirements and keeps any out-of-tree reliance on
> PA_SINK_MESSAGE_SET_VOLUME message semantically unchanged.[3]
>
Seems reasonable. Changed.
>> @@ -1555,6 +1594,16 @@ static void propagate_real_volume(pa_sink *s,
>> const pa_cvolume *old_real_volume)
>> pa_subscription_post(s->core,
>> PA_SUBSCRIPTION_EVENT_SINK|PA_SUBSCRIPTION_EVENT_CHANGE,
>> s->index);
>> }
>>
>> +/* Called from io thread */
>> +void pa_sink_update_volume_and_mute(pa_sink *s) {
>> +pa_assert(s);
>> +pa_sink_assert_io_context(s);
>> +
>> +pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(s),
>> + PA_SINK_MESSAGE_UPDATE_VOLUME_AND_MUTE,
>> +
Re: [pulseaudio-discuss] [PATCH 0/5] HW and SW volume syncronization in IO-thread
>> On 2010-10-13 18:40, o...@iki.fi wrote: >>> From: Jyri Sarha > ... >> ...rather than trying to add explicit delays just for the volume sync. >> Either that, or some kind of volume ramping. Just curious if you >> considered that solution as well? >> Ah, only after reading my reply from the mailing list I understand your concern. The sync-volume is really not only using some explicit timers, but it uses sinks latency information (got with pa_sink_get_latency_within_thread()) for timing the HW volume changes and adjust the timers when ever the DMA buffer is rewound. The parameters given are there only to compensate for timer and reported latency inaccuracy. The "safety margin" is for compensation dynamic inaccuracy and "extra delay" is there to compensate static errors. On most hardware "extra delay" should be set to zero. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] HW and SW volume syncronization in IO-thread
>> On 2010-10-13 18:40, o...@iki.fi wrote: >>> From: Jyri Sarha > ... >> >> Thank you for your work! This is the main reason I've been advocating >> against enabling flat-volumes by default in Ubuntu, so I'm glad to see >> something that addresses the issue. >> >> The basic problem is that we can't have sample accurate volume changes, >> because no HW supports it. Right? > > Yes. On on non real-time OS and having less than accurate audio > pipeline latency information this is a best effort task. Making > this perfect would require HW support. > >> >> So about the implementation - what you're saying is that it is better to >> have two guaranteed down-volume "snaps" rather than to risk an up-volume >> "snap". That sounds reasonable, but are these down-volume transients >> hearable? Perhaps more hearable with playing a sine wave, rather than >> white noise? >> > > Since implementing ramps on a HW mixer is really not applicable, > this is the only solution that I can see and in practice it seems > to work pretty well. I also think that most analog components > have enough "slowness" in them which in effect causes a kind of > ramp. > > BTW, ran the test from my previous mail, but using sine wave, and > still I could not hear any snapping sound on my T60. > >> Anyway, I've been thinking of something like this but never looked more >> closely at implementing a solution, but I would probably have tried to >> do something like: >> start: >> 1) rewind >> 2) write stream with lower SW volume >> 3) wait until rewind margin has passed >> 4) raise HW volume > > This is pretty much what my patch is doing, just in slightly > different order: > > 1. Insert volume event to happen after sink's reported latency has > expired (+ modifications). > 2. rewind and adjust volume events > 3. wait until volume event expires = rewind margin has passed > 4. raise HW volume > > This approach works also with sinks that do not support > rewinding (e.g. alsa-sink not using tsched). > >> end: >> 1) lower HW volume >> 2) rewind >> 3) write stream with higher SW volume >> > > Here you are forgetting that there may still be some samples left > within the rewind margin. You should add the delay also there. In > effect the both volume ups and downs become quite symmetrical. > >> ...rather than trying to add explicit delays just for the volume sync. >> Either that, or some kind of volume ramping. Just curious if you >> considered that solution as well? >> > > Well, I can't think how you get past "wait until rewind margin > has passed" without having explicit delays. > > This code is really only chaning the way HW mixers are > used. Ramping to SW volume could be implemented separately and it > would not interfere with this patch in fact they would > complement each other. > > But, hey please try out my patch. At least it solved the problem > on N900 (yes the earlier version of the patch is on all N900s out there) > and it works quite well on my both Linux boxes. > > Cheers, > Jyri > ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] HW and SW volume syncronization in IO-thread
>> On 2010-10-13 18:40, o...@iki.fi wrote: >>> From: Jyri Sarha > ... >> ...rather than trying to add explicit delays just for the volume sync. >> Either that, or some kind of volume ramping. Just curious if you >> considered that solution as well? >> Ah, only after reading my reply from the mailing list I understand your concern. The sync-volume is really not only using some explicit timers, but it uses sinks latency information (got with pa_sink_get_latency_within_thread()) for timing the HW volume changes and adjust the timers when ever the DMA buffer is rewound. The parameters given are there only to compensate for timer and reported latency inaccuracy. The "safety margin" is for compensation dynamic inaccuracy and "extra delay" is there to compensate static errors. On most hardware "extra delay" should be set to zero. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 0/5] HW and SW volume syncronization in IO-thread
> On 2010-10-13 18:40, o...@iki.fi wrote: >> From: Jyri Sarha ... > > Thank you for your work! This is the main reason I've been advocating > against enabling flat-volumes by default in Ubuntu, so I'm glad to see > something that addresses the issue. > > The basic problem is that we can't have sample accurate volume changes, > because no HW supports it. Right? Yes. On on non real-time OS and having less than accurate audio pipeline latency information this is a best effort task. Making this perfect would require HW support. > > So about the implementation - what you're saying is that it is better to > have two guaranteed down-volume "snaps" rather than to risk an up-volume > "snap". That sounds reasonable, but are these down-volume transients > hearable? Perhaps more hearable with playing a sine wave, rather than > white noise? > Since implementing ramps on a HW mixer is really not applicable, this is the only solution that I can see and in practice it seems to work pretty well. I also think that most analog components have enough "slowness" in them which in effect causes a kind of ramp. BTW, ran the test from my previous mail, but using sine wave, and still I could not hear any snapping sound on my T60. > Anyway, I've been thinking of something like this but never looked more > closely at implementing a solution, but I would probably have tried to > do something like: > start: > 1) rewind > 2) write stream with lower SW volume > 3) wait until rewind margin has passed > 4) raise HW volume This is pretty much what my patch is doing, just in slightly different order: 1. Insert volume event to happen after sink's reported latency has expired (+ modifications). 2. rewind and adjust volume events 3. wait until volume event expires = rewind margin has passed 4. raise HW volume This approach works also with sinks that do not support rewinding (e.g. alsa-sink not using tsched). > end: > 1) lower HW volume > 2) rewind > 3) write stream with higher SW volume > Here you are forgetting that there may still be some samples left within the rewind margin. You should add the delay also there. In effect the both volume ups and downs become quite symmetrical. > ...rather than trying to add explicit delays just for the volume sync. > Either that, or some kind of volume ramping. Just curious if you > considered that solution as well? > Well, I can't think how you get past "wait until rewind margin has passed" without having explicit delays. This code is really only chaning the way HW mixers are used. Ramping to SW volume could be implemented separately and it would not interfere with this patch in fact they would complement each other. But, hey please try out my patch. At least it solved the problem on N900 (yes the earlier version of the patch is on all N900s out there) and it works quite well on my both Linux boxes. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] core: New LIFO style flist implementation v2.0
On Tue, 31 Aug 2010, Shouqun Liu wrote: After apply this patch, pulseaudio starts with segmentation fault..my pulseadudio version is 0.9.21 thanks Sorry about that. There was a clear bug in the patch. I must have been running different code than I was testing. Any way I have fixed the bug and I'll post yet another version of the patch shortly. Cheers, Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] core: New LIFO style flist implementation
On Mon, 23 Aug 2010, Shouqun Liu wrote: Hi Jyri, I have applied the patch, but makes no effort to improve the performance, the CPU usage is still high :) Well, I originally created the patch to avoid pulseaudio getting stuck while waiting swapped out memblocks on Nokia N900. After applying the patch I also noticed that it slightly improved the performance in some usecases. I think that was because of better CPU cache hit ratio, but I guess you need to have a lot of concurrent streams or some audio filtering ongoing on a limited memory-bandwidth device like N900 to notice the difference. I'm still confusing about this issue, there is no such problem in my PC, but its critical in my netbook. btw, the PA version on PC and netbook are both latest 0.9.21 release. Do you have a constant high load on your netbook or do you see it for instance while PA is rewinding streams? You should be able to decrease the load spikes from rewinding by shortening the ALSA DMA buffer. Do your streams have the same sample-rate as your default sink? If no, relaxing your sample-rate-converter settings should help (or configuring your sink to match the samplerate of your stremas). Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] core: New LIFO style flist implementation
I forgot to mail this patch earlier. Anyway like the comment says this
flist implementation improves performance at least on devices with limited
physical memory. In my opinion this version is also easier to understand.
Cheers,
Jyri
>From 76aa7522d415984dafd564f7395f403d4407e21a Mon Sep 17 00:00:00 2001
From: Jyri Sarha
Date: Mon, 30 Nov 2009 16:55:27 +0200
Subject: [PATCH] core: New LIFO style flist implementation
The old free list implementation used objects in FIFO style. This is
bad because it tries keep all the objects ever used alive and in
memory. This minimizes the changes that an allocated object is already
in cache. When there is shortage of physical memory this may also
increase change that newly allocated object is swapped out. LIFO
(e.g. stack) style free list should help these issues. Like the old
one the new implementation is also lock free.
---
src/pulsecore/flist.c | 211 +---
1 files changed, 57 insertions(+), 154 deletions(-)
diff --git a/src/pulsecore/flist.c b/src/pulsecore/flist.c
index 7e5ee24..47bfed8 100644
--- a/src/pulsecore/flist.c
+++ b/src/pulsecore/flist.c
@@ -1,7 +1,9 @@
/***
This file is part of PulseAudio.
- Copyright 2006-2008 Lennart Poettering
+ Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+
+ Contact: Jyri Sarha
PulseAudio is free software; you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as
@@ -34,198 +36,99 @@
#include "flist.h"
-/* Algorithm is not perfect, in a few corner cases it will fail to pop
- * from the flist although it isn't empty, and fail to push into the
- * flist, although it isn't full.
- *
- * We keep a fixed size array of entries, each item is an atomic
- * pointer.
- *
- * To accelerate finding of used/unused cells we maintain a read and a
- * write index which is used like a ring buffer. After each push we
- * increase the write index and after each pop we increase the read
- * index.
- *
- * The indexes are incremented atomically and are never truncated to
- * the buffer size. Instead we assume that the buffer size is a power
- * of two and that the truncation can thus be done by applying a
- * simple AND on read.
- *
- * To make sure that we do not look for empty cells indefinitely we
- * maintain a length value which stores the number of used cells. From
- * this value the number of unused cells is easily calculated. Please
- * note that the length value is not updated atomically with the read
- * and write index and might thus be a few cells off the real
- * value. To deal with this we always look for N_EXTRA_SCAN extra
- * cells when pushing/popping entries.
- *
- * It might make sense to replace this implementation with a link list
- * stack or queue, which however requires DCAS to be simple. Patches
- * welcome.
- *
- * Please note that this algorithm is home grown.*/
-
#define FLIST_SIZE 128
-#define N_EXTRA_SCAN 3
-/* For debugging purposes we can define _Y to put and extra thread
- * yield between each operation. */
+struct pa_flist_elem {
+struct pa_flist_elem *next;
+void *ptr;
+};
-#ifdef PROFILE
-#define _Y pa_thread_yield()
-#else
-#define _Y do { } while(0)
-#endif
+typedef struct pa_flist_elem pa_flist_elem;
struct pa_flist {
unsigned size;
-pa_atomic_t length;
-pa_atomic_t read_idx;
-pa_atomic_t write_idx;
+pa_atomic_ptr_t stored;
+pa_atomic_ptr_t empty;
+pa_flist_elem table[0];
};
-#define PA_FLIST_CELLS(x) ((pa_atomic_ptr_t*) ((uint8_t*) (x) + PA_ALIGN(sizeof(struct pa_flist
+static pa_flist_elem *stack_pop(pa_atomic_ptr_t *list) {
+pa_flist_elem * poped;
+pa_assert(list);
+
+do {
+poped = (pa_flist_elem *) pa_atomic_ptr_load(list);
+} while (poped != NULL && !pa_atomic_ptr_cmpxchg(list, poped, poped->next));
+
+return poped;
+}
+
+static void stack_push(pa_atomic_ptr_t *list, pa_flist_elem *new_elem) {
+pa_assert(list);
+
+do {
+new_elem->next = (pa_flist_elem *) pa_atomic_ptr_load(list);
+} while (!pa_atomic_ptr_cmpxchg(list, new_elem->next, new_elem));
+}
pa_flist *pa_flist_new(unsigned size) {
pa_flist *l;
+unsigned i;
if (!size)
size = FLIST_SIZE;
-pa_assert(pa_is_power_of_two(size));
-
-l = pa_xmalloc0(PA_ALIGN(sizeof(pa_flist)) + (sizeof(pa_atomic_ptr_t) * size));
+l = pa_xmalloc0(sizeof(pa_flist) + sizeof(pa_flist_elem) * size);
l->size = size;
-
-pa_atomic_store(&l->read_idx, 0);
-pa_atomic_store(&l->write_idx, 0);
-pa_atomic_store(&l->length, 0);
-
+pa_atomic_ptr_store(&l->stored, NULL);
+pa_atomic_ptr_store(&l->empty, NULL);
+for (i=0; i < size; i++) {
+stack_push(&l->empty, &l->table[i]);
+}
return l;
}
-static unsigned reduce(pa_flist *l, unsigned value) {
-return value & (l->
[pulseaudio-discuss] [PATCH 5/5] alsa-card: Add sync_volume parameter
From: Jyri Sarha
---
src/modules/alsa/module-alsa-card.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/modules/alsa/module-alsa-card.c
b/src/modules/alsa/module-alsa-card.c
index 6bea33d..8e613e7 100644
--- a/src/modules/alsa/module-alsa-card.c
+++ b/src/modules/alsa/module-alsa-card.c
@@ -63,7 +63,8 @@ PA_MODULE_USAGE(
"tsched_buffer_size= "
"tsched_buffer_watermark= "
"profile= "
-"ignore_dB=");
+"ignore_dB= "
+"sync_volume=");
static const char* const valid_modargs[] = {
"name",
@@ -84,6 +85,7 @@ static const char* const valid_modargs[] = {
"tsched_buffer_watermark",
"profile",
"ignore_dB",
+"sync_volume",
NULL
};
--
1.7.0
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/5] core: Add infrastructure for delayed HW volume setting
From: Jyri Sarha
To make concurrent use of SW and HW volume glitchles their application
needs to be synchronized. For accurate synchronization the HW volume
needs to be applied in IO thread. This patch adds infrastructure to
delay the applying of HW volume to match with SW volume timing. For
this patch to have any effect it needs to be taken into use by sink
implementor.
---
src/pulse/def.h |8 ++-
src/pulsecore/sink.c | 201 +-
src/pulsecore/sink.h | 39 ++-
3 files changed, 245 insertions(+), 3 deletions(-)
diff --git a/src/pulse/def.h b/src/pulse/def.h
index 82106ef..e84edde 100644
--- a/src/pulse/def.h
+++ b/src/pulse/def.h
@@ -729,9 +729,14 @@ typedef enum pa_sink_flags {
/**< This sink is in flat volume mode, i.e. always the maximum of
* the volume of all connected inputs. \since 0.9.15 */
-PA_SINK_DYNAMIC_LATENCY = 0x0080U
+PA_SINK_DYNAMIC_LATENCY = 0x0080U,
/**< The latency can be adjusted dynamically depending on the
* needs of the connected streams. \since 0.9.15 */
+
+PA_SINK_SYNC_VOLUME = 0x0100U,
+/**< The HW volume changes are syncronized with SW volume.
+ * \since X.X.XX */
+
} pa_sink_flags_t;
/** \cond fulldocs */
@@ -743,6 +748,7 @@ typedef enum pa_sink_flags {
#define PA_SINK_DECIBEL_VOLUME PA_SINK_DECIBEL_VOLUME
#define PA_SINK_FLAT_VOLUME PA_SINK_FLAT_VOLUME
#define PA_SINK_DYNAMIC_LATENCY PA_SINK_DYNAMIC_LATENCY
+#define PA_SINK_SYNC_VOLUME PA_SINK_SYNC_VOLUME
/** \endcond */
/** Sink state. \since 0.9.15 */
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index d69f038..62f0d71 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -43,6 +44,7 @@
#include
#include
#include
+#include
#include "sink.h"
@@ -51,6 +53,8 @@
#define ABSOLUTE_MIN_LATENCY (500)
#define ABSOLUTE_MAX_LATENCY (10*PA_USEC_PER_SEC)
#define DEFAULT_FIXED_LATENCY (250*PA_USEC_PER_MSEC)
+#define VOLUME_CHANGE_SAFETY_MARGIN_DEFAULT (8*PA_USEC_PER_MSEC)
+#define VOLUME_CHANGE_EXTRA_DELAY_DEFAULT (0*PA_USEC_PER_MSEC)
PA_DEFINE_PUBLIC_CLASS(pa_sink, pa_msgobject);
@@ -310,6 +314,12 @@ pa_sink* pa_sink_new(
s->thread_info.max_latency = ABSOLUTE_MAX_LATENCY;
s->thread_info.fixed_latency = flags & PA_SINK_DYNAMIC_LATENCY ? 0 :
DEFAULT_FIXED_LATENCY;
+PA_LLIST_HEAD_INIT(pa_sink_volume_change, s->thread_info.volume_changes);
+s->thread_info.volume_changes_tail = NULL;
+pa_sw_cvolume_multiply(&s->thread_info.current_hw_volume, &s->soft_volume,
&s->real_volume);
+s->thread_info.volume_change_safety_margin =
VOLUME_CHANGE_SAFETY_MARGIN_DEFAULT;
+s->thread_info.volume_change_extra_delay =
VOLUME_CHANGE_EXTRA_DELAY_DEFAULT;
+
/* FIXME: This should probably be moved to pa_sink_put() */
pa_assert_se(pa_idxset_put(core->sinks, s, &s->index) >= 0);
@@ -444,6 +454,7 @@ void pa_sink_put(pa_sink* s) {
s->thread_info.soft_volume = s->soft_volume;
s->thread_info.soft_muted = s->muted;
+pa_sw_cvolume_multiply(&s->thread_info.current_hw_volume, &s->soft_volume,
&s->real_volume);
pa_assert((s->flags & PA_SINK_HW_VOLUME_CTRL) || (s->base_volume ==
PA_VOLUME_NORM && s->flags & PA_SINK_DECIBEL_VOLUME));
pa_assert(!(s->flags & PA_SINK_DECIBEL_VOLUME) || s->n_volume_steps ==
PA_VOLUME_NORM+1);
@@ -733,6 +744,10 @@ void pa_sink_process_rewind(pa_sink *s, size_t nbytes) {
if (nbytes > 0)
if (s->monitor_source &&
PA_SOURCE_IS_LINKED(s->monitor_source->thread_info.state))
pa_source_process_rewind(s->monitor_source, nbytes);
+
+if (nbytes > 0) {
+pa_sink_volume_change_rewind(s, nbytes);
+}
}
/* Called from IO thread context */
@@ -1467,7 +1482,7 @@ void pa_sink_set_soft_volume(pa_sink *s, const pa_cvolume
*volume) {
s->soft_volume = *volume;
if (PA_SINK_IS_LINKED(s->state))
-pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
PA_SINK_MESSAGE_SET_VOLUME, NULL, 0, NULL) == 0);
+pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
PA_SINK_MESSAGE_SET_SOFT_VOLUME, NULL, 0, NULL) == 0);
else
s->thread_info.soft_volume = s->soft_volume;
}
@@ -1975,6 +1990,13 @@ int pa_sink_process_msg(pa_msgobject *o, int code, void
*userdata, int64_t offse
case PA_SINK_MESSAGE_SET_VOLUME:
+if (s->flags & PA_SINK_SYNC_VOLUME)
+pa_sink_volume_change_push(s);
+
+/* Fall through ... */
+
+case PA_SINK_MESSAGE_SET_SOFT_VOLUME:
+
if (!pa_cvolume_equal(&s->thread_info.soft_volume,
&s->soft_volume)) {
s->thread_info.soft_volume = s->
[pulseaudio-discuss] [PATCH 2/5] alsa-sink: Take syncronized HW volume infra into use
From: Jyri Sarha
---
src/modules/alsa/alsa-mixer.c | 38 +---
src/modules/alsa/alsa-mixer.h |2 +-
src/modules/alsa/alsa-sink.c| 81 +++---
src/modules/alsa/alsa-source.c |2 +-
src/modules/alsa/module-alsa-sink.c |8 +++-
5 files changed, 113 insertions(+), 18 deletions(-)
diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index 93f2ed0..0e77a4b 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -671,7 +671,13 @@ int pa_alsa_path_get_mute(pa_alsa_path *p, snd_mixer_t *m,
pa_bool_t *muted) {
return 0;
}
-static int element_set_volume(pa_alsa_element *e, snd_mixer_t *m, const
pa_channel_map *cm, pa_cvolume *v) {
+static int element_set_volume(
+pa_alsa_element *e,
+snd_mixer_t *m,
+const pa_channel_map *cm,
+pa_cvolume *v,
+pa_bool_t write_to_hw) {
+
snd_mixer_selem_id_t *sid;
pa_cvolume rv;
snd_mixer_elem_t *me;
@@ -720,14 +726,26 @@ static int element_set_volume(pa_alsa_element *e,
snd_mixer_t *m, const pa_chann
* if the channel is available, ALSA behaves ver
* strangely and doesn't fail the call */
if (snd_mixer_selem_has_playback_channel(me, c)) {
-if ((r = snd_mixer_selem_set_playback_dB(me, c, value,
+1)) >= 0)
-r = snd_mixer_selem_get_playback_dB(me, c, &value);
+if (write_to_hw) {
+if ((r = snd_mixer_selem_set_playback_dB(me, c, value,
+1)) >= 0)
+r = snd_mixer_selem_get_playback_dB(me, c, &value);
+} else {
+long alsa_val;
+if ((r = snd_mixer_selem_ask_playback_dB_vol(me,
value, +1, &alsa_val)) >= 0)
+r = snd_mixer_selem_ask_playback_vol_dB(me,
alsa_val, &value);
+}
} else
r = -1;
} else {
if (snd_mixer_selem_has_capture_channel(me, c)) {
-if ((r = snd_mixer_selem_set_capture_dB(me, c, value, +1))
>= 0)
-r = snd_mixer_selem_get_capture_dB(me, c, &value);
+if (write_to_hw) {
+if ((r = snd_mixer_selem_set_capture_dB(me, c, value,
+1)) >= 0)
+r = snd_mixer_selem_get_capture_dB(me, c, &value);
+} else {
+long alsa_val;
+if ((r = snd_mixer_selem_ask_capture_dB_vol(me, value,
+1, &alsa_val)) >= 0)
+r = snd_mixer_selem_ask_capture_vol_dB(me,
alsa_val, &value);
+}
} else
r = -1;
}
@@ -782,7 +800,13 @@ static int element_set_volume(pa_alsa_element *e,
snd_mixer_t *m, const pa_chann
return 0;
}
-int pa_alsa_path_set_volume(pa_alsa_path *p, snd_mixer_t *m, const
pa_channel_map *cm, pa_cvolume *v) {
+int pa_alsa_path_set_volume(
+pa_alsa_path *p,
+snd_mixer_t *m,
+const pa_channel_map *cm,
+pa_cvolume *v,
+pa_bool_t write_to_hw) {
+
pa_alsa_element *e;
pa_cvolume rv;
@@ -807,7 +831,7 @@ int pa_alsa_path_set_volume(pa_alsa_path *p, snd_mixer_t
*m, const pa_channel_ma
pa_assert(!p->has_dB || e->has_dB);
ev = rv;
-if (element_set_volume(e, m, cm, &ev) < 0)
+if (element_set_volume(e, m, cm, &ev, write_to_hw) < 0)
return -1;
if (!p->has_dB) {
diff --git a/src/modules/alsa/alsa-mixer.h b/src/modules/alsa/alsa-mixer.h
index a0d4fcb..ce95edf 100644
--- a/src/modules/alsa/alsa-mixer.h
+++ b/src/modules/alsa/alsa-mixer.h
@@ -202,7 +202,7 @@ int pa_alsa_path_probe(pa_alsa_path *p, snd_mixer_t *m,
pa_bool_t ignore_dB);
void pa_alsa_path_dump(pa_alsa_path *p);
int pa_alsa_path_get_volume(pa_alsa_path *p, snd_mixer_t *m, const
pa_channel_map *cm, pa_cvolume *v);
int pa_alsa_path_get_mute(pa_alsa_path *path, snd_mixer_t *m, pa_bool_t
*muted);
-int pa_alsa_path_set_volume(pa_alsa_path *path, snd_mixer_t *m, const
pa_channel_map *cm, pa_cvolume *v);
+int pa_alsa_path_set_volume(pa_alsa_path *path, snd_mixer_t *m, const
pa_channel_map *cm, pa_cvolume *v, pa_bool_t write_to_hw);
int pa_alsa_path_set_mute(pa_alsa_path *path, snd_mixer_t *m, pa_bool_t muted);
int pa_alsa_path_select(pa_alsa_path *p, snd_mixer_t *m);
void pa_alsa_path_set_callback(pa_alsa_path *p, snd_mixer_t *m,
snd_mixer_elem_callback_t cb, void *userdata);
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 581b943..45ba24a 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -1160,6 +1160,7 @@ static void sink_set_volume_cb(pa_sink *s) {
struct userdata *u = s->userdata;
pa_cvolu
[pulseaudio-discuss] [PATCH 4/5] udev-detect: Add sync_volume parameter
From: Jyri Sarha
---
src/modules/module-udev-detect.c | 17 ++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/modules/module-udev-detect.c b/src/modules/module-udev-detect.c
index 7695d3c..205c737 100644
--- a/src/modules/module-udev-detect.c
+++ b/src/modules/module-udev-detect.c
@@ -45,7 +45,8 @@ PA_MODULE_VERSION(PACKAGE_VERSION);
PA_MODULE_LOAD_ONCE(TRUE);
PA_MODULE_USAGE(
"tsched= "
-"ignore_dB=");
+"ignore_dB= "
+"sync_volume=");
struct device {
char *path;
@@ -62,6 +63,7 @@ struct userdata {
pa_bool_t use_tsched:1;
pa_bool_t ignore_dB:1;
+pa_bool_t sync_volume:1;
struct udev* udev;
struct udev_monitor *monitor;
@@ -74,6 +76,7 @@ struct userdata {
static const char* const valid_modargs[] = {
"tsched",
"ignore_dB",
+"sync_volume",
NULL
};
@@ -385,12 +388,14 @@ static void card_changed(struct userdata *u, struct
udev_device *dev) {
"card_name=\"%s\" "
"tsched=%s "
"ignore_dB=%s "
+"sync_volume=%s "
"card_properties=\"module-udev-detect.discovered=1\"",
path_get_card_id(path),
n,
d->card_name,
pa_yes_no(u->use_tsched),
-pa_yes_no(u->ignore_dB));
+pa_yes_no(u->ignore_dB),
+pa_yes_no(u->sync_volume));
pa_xfree(n);
pa_hashmap_put(u->devices, d->path, d);
@@ -660,7 +665,7 @@ int pa__init(pa_module *m) {
struct udev_enumerate *enumerate = NULL;
struct udev_list_entry *item = NULL, *first = NULL;
int fd;
-pa_bool_t use_tsched = TRUE, ignore_dB = FALSE;
+pa_bool_t use_tsched = TRUE, ignore_dB = FALSE, sync_volume=FALSE;
pa_assert(m);
@@ -686,6 +691,12 @@ int pa__init(pa_module *m) {
}
u->ignore_dB = ignore_dB;
+if (pa_modargs_get_value_boolean(ma, "sync_volume", &sync_volume) < 0) {
+pa_log("Failed to parse sync_volume= argument.");
+goto fail;
+}
+u->sync_volume = sync_volume;
+
if (!(u->udev = udev_new())) {
pa_log("Failed to initialize udev library.");
goto fail;
--
1.7.0
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 0/5] Synchronize HW and SW volume timing
From: Jyri Sarha The first patch here is part of Maemo Fremantle release, so has gone trough considerable amount of testing already. However, the adaptation on top of the latest upstream is fairly recent. Jyri Sarha (5): core: Add infrastructure for delayed HW volume setting alsa-sink: Take syncronized HW volume infra into use alsa-sink: Add locking to protect concurrent mixer and mixer_path access udev-detect: Add sync_volume parameter alsa-card: Add sync_volume parameter src/modules/alsa/alsa-mixer.c | 38 ++-- src/modules/alsa/alsa-mixer.h |2 +- src/modules/alsa/alsa-sink.c| 132 +-- src/modules/alsa/alsa-source.c |2 +- src/modules/alsa/module-alsa-card.c |4 +- src/modules/alsa/module-alsa-sink.c |8 ++- src/modules/module-udev-detect.c| 17 +++- src/pulse/def.h |8 ++- src/pulsecore/sink.c| 201 ++- src/pulsecore/sink.h| 39 +++- 10 files changed, 424 insertions(+), 27 deletions(-) ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 3/5] alsa-sink: Add locking to protect concurrent mixer and mixer_path access
From: Jyri Sarha
---
src/modules/alsa/alsa-sink.c | 55 ++---
1 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 45ba24a..43b0b09 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -98,6 +98,12 @@ struct userdata {
snd_mixer_t *mixer_handle;
pa_alsa_path_set *mixer_path_set;
pa_alsa_path *mixer_path;
+/* NOTE: This lock is a crude way of protecting both mixer_path and
mixer_element
+ * when accessing them from main and IO-thread concurrently.
Everything
+ * seem to work fine even without this lock when stress testing the
sync
+ * volume with multiple concurrent transient streams, but better
safe than
+ * sorry. */
+pa_mutex *mixer_mutex;
pa_cvolume hardware_volume;
@@ -1133,12 +1139,16 @@ static void sink_get_volume_cb(pa_sink *s) {
struct userdata *u = s->userdata;
pa_cvolume r;
char t[PA_CVOLUME_SNPRINT_MAX];
+int status;
pa_assert(u);
pa_assert(u->mixer_path);
pa_assert(u->mixer_handle);
-if (pa_alsa_path_get_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &r) < 0)
+pa_mutex_lock(u->mixer_mutex);
+status = pa_alsa_path_get_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &r);
+pa_mutex_unlock(u->mixer_mutex);
+if (status < 0)
return;
/* Shift down by the base volume, so that 0dB becomes maximum volume */
@@ -1161,6 +1171,7 @@ static void sink_set_volume_cb(pa_sink *s) {
pa_cvolume r;
char t[PA_CVOLUME_SNPRINT_MAX];
pa_bool_t write_to_hw = (s->flags & PA_SINK_SYNC_VOLUME) ? FALSE : TRUE;
+int status;
pa_assert(u);
pa_assert(u->mixer_path);
@@ -1169,7 +1180,10 @@ static void sink_set_volume_cb(pa_sink *s) {
/* Shift up by the base volume */
pa_sw_cvolume_divide_scalar(&r, &s->real_volume, s->base_volume);
-if (pa_alsa_path_set_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &r, write_to_hw) < 0)
+pa_mutex_lock(u->mixer_mutex);
+status = pa_alsa_path_set_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &r, write_to_hw);
+pa_mutex_unlock(u->mixer_mutex);
+if (status < 0)
return;
/* Shift down by the base volume, so that 0dB becomes maximum volume */
@@ -1212,13 +1226,18 @@ static void sink_set_volume_cb(pa_sink *s) {
static void sink_write_volume_cb(pa_sink *s) {
struct userdata *u = s->userdata;
pa_cvolume hw_vol = s->thread_info.current_hw_volume;
+int status;
pa_assert(u);
pa_assert(u->mixer_path);
pa_assert(u->mixer_handle);
pa_assert(s->flags & PA_SINK_SYNC_VOLUME);
-if (pa_alsa_path_set_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &hw_vol, TRUE) < 0)
+pa_mutex_lock(u->mixer_mutex);
+status = pa_alsa_path_set_volume(u->mixer_path, u->mixer_handle,
&s->channel_map, &hw_vol, TRUE);
+pa_mutex_unlock(u->mixer_mutex);
+
+if (status < 0)
pa_log_error("Writting HW volume failed");
else {
pa_cvolume tmp_vol;
@@ -1239,12 +1258,16 @@ static void sink_write_volume_cb(pa_sink *s) {
static void sink_get_mute_cb(pa_sink *s) {
struct userdata *u = s->userdata;
pa_bool_t b;
+int status;
pa_assert(u);
pa_assert(u->mixer_path);
pa_assert(u->mixer_handle);
-if (pa_alsa_path_get_mute(u->mixer_path, u->mixer_handle, &b) < 0)
+pa_mutex_lock(u->mixer_mutex);
+status = pa_alsa_path_get_mute(u->mixer_path, u->mixer_handle, &b);
+pa_mutex_unlock(u->mixer_mutex);
+if (status < 0)
return;
s->muted = b;
@@ -1257,7 +1280,9 @@ static void sink_set_mute_cb(pa_sink *s) {
pa_assert(u->mixer_path);
pa_assert(u->mixer_handle);
+pa_mutex_lock(u->mixer_mutex);
pa_alsa_path_set_mute(u->mixer_path, u->mixer_handle, s->muted);
+pa_mutex_unlock(u->mixer_mutex);
}
static int sink_set_port_cb(pa_sink *s, pa_device_port *p) {
@@ -1271,6 +1296,13 @@ static int sink_set_port_cb(pa_sink *s, pa_device_port
*p) {
data = PA_DEVICE_PORT_DATA(p);
pa_assert_se(u->mixer_path = data->path);
+
+/* This forces flushing of pending volume change events */
+if (u->sink->flags & PA_SINK_SYNC_VOLUME)
+pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s),
PA_SINK_MESSAGE_GET_VOLUME, NULL, 0, NULL) == 0);
+
+pa_mutex_lock(u->mixer_mutex);
+
pa_alsa_path_select(u->mixer_path, u->mixer_handle);
if (u->mixer_path->has_volume && u->mixer_path->has_dB) {
@@ -1289,10 +1321,16 @@ static int sink_set_port_cb(pa_sin
[pulseaudio-discuss] [PATCH] optimization: Optimized pa_sink_render_full.
From: Jyri Sarha
This is finally the latest version of the patch.
---
src/pulsecore/sink.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 28b3440..bd4130f 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -1010,14 +1010,13 @@ void pa_sink_render_full(pa_sink *s, size_t length,
pa_memchunk *result) {
pa_memchunk chunk;
size_t l, d;
pa_memchunk_make_writable(result, length);
- result->length = length;
l = length - result->length;
d = result->index + result->length;
while (l > 0) {
chunk = *result;
- chunk.index += d;
- chunk.length -= d - result->index;
+ chunk.index = d;
+ chunk.length = l;
pa_sink_render_into(s, &chunk);
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] bluetooth-device: Add safe guard
On Wed, 13 May 2009, Jyri Sarha wrote: Sorryyy.. this is the same thing that I posted couple of days ago. The second patch in this thread has the working version of the optimized pa_sink_render_full(). And sorry for the typo in the comment, I was working i bit too much of a hurry :). BTW, patches are in clean order at: http://git.gitorious.org/~oku/pulseaudio/oku-clone.git Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluetooth-device: Add safe guard against BT streaming irregularities.
From: Jyri Sarha
Some bad quality BT-headsets block bluez socket sometimes for hundreds
of milliseconds, especially when changing mode. When the module tries
catch up the lost time it may SBC encode up to half a second of audio
without yielding. On slow machine this may cause maximum RT time slice
to be exceeded. Cleaned out the minor fix that slipped into first version
of the patch.
---
src/modules/bluetooth/module-bluetooth-device.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-device.c
b/src/modules/bluetooth/module-bluetooth-device.c
index ecb5e83..1b42f4a 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -174,6 +174,8 @@ struct userdata {
#define FIXED_LATENCY_PLAYBACK_HSP (125*PA_USEC_PER_MSEC)
#define FIXED_LATENCY_RECORD_HSP (25*PA_USEC_PER_MSEC)
+#define MAX_PLAYBACK_CATCH_UP_USEC (100*PA_USEC_PER_MSEC)
+
#ifdef NOKIA
#define USE_SCO_OVER_PCM(u) (u->profile == PROFILE_HSP && (u->hsp.sco_sink &&
u->hsp.sco_source))
#endif
@@ -1296,15 +1298,27 @@ static void thread_func(void *userdata) {
if ((!u->source ||
!PA_SOURCE_IS_LINKED(u->source->thread_info.state)) && do_write <= 0 &&
writable) {
pa_usec_t time_passed;
-uint64_t should_have_written;
+pa_usec_t audio_sent;
/* Hmm, there is no input stream we could synchronize
* to. So let's do things by time */
time_passed = pa_rtclock_usec() - u->started_at;
-should_have_written = pa_usec_to_bytes(time_passed,
&u->sample_spec);
-
-do_write = u->write_index <= should_have_written;
+audio_sent = pa_bytes_to_usec(u->write_index,
&u->sample_spec);
+
+if (audio_sent <= time_passed) {
+pa_usec_t audio_to_send = time_passed - audio_sent;
+if (u->write_index > 0 && audio_to_send >
MAX_PLAYBACK_CATCH_UP_USEC) {
+pa_usec_t skip_usec = audio_to_send -
MAX_PLAYBACK_CATCH_UP_USEC;
+uint64_t skip_bytes = pa_usec_to_bytes(skip_usec,
&u->sample_spec);
+pa_memchunk tmp;
+pa_log_warn("Skipping %lld us (= %lld bytes) in
audio stream", skip_usec, skip_bytes);
+pa_sink_render_full(u->sink, skip_bytes, &tmp);
+pa_memblock_unref(tmp.memblock);
+u->write_index += skip_bytes;
+}
+do_write = 1;
+}
}
if (writable && do_write > 0) {
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] optimization: Optimized pa_sink_render_full.
From: Jyri Sarha
This is finally the latest version of the patach.
---
src/pulsecore/sink.c | 77 ++---
1 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 30fa557..63752d9 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -933,22 +933,89 @@ void pa_sink_render_into_full(pa_sink *s, pa_memchunk
*target) {
/* Called from IO thread context */
void pa_sink_render_full(pa_sink *s, size_t length, pa_memchunk *result) {
+pa_mix_info info[MAX_MIX_CHANNELS];
+size_t length1st = length;
+unsigned n;
+
pa_sink_assert_ref(s);
pa_assert(PA_SINK_IS_LINKED(s->thread_info.state));
pa_assert(length > 0);
pa_assert(pa_frame_aligned(length, &s->sample_spec));
pa_assert(result);
+pa_sink_ref(s);
+
pa_assert(!s->thread_info.rewind_requested);
pa_assert(s->thread_info.rewind_nbytes == 0);
-/*** This needs optimization ***/
+pa_assert(length > 0);
+
+n = fill_mix_info(s, &length1st, info, MAX_MIX_CHANNELS);
+
+if (n == 0) {
+ pa_silence_memchunk_get(&s->core->silence_cache,
+ s->core->mempool,
+ result,
+ &s->sample_spec,
+ length1st);
+} else if (n == 1) {
+ pa_cvolume volume;
-result->index = 0;
-result->length = length;
-result->memblock = pa_memblock_new(s->core->mempool, length);
+ *result = info[0].chunk;
+ pa_memblock_ref(result->memblock);
+
+ if (result->length > length)
+ result->length = length;
+
+ pa_sw_cvolume_multiply(&volume, &s->thread_info.soft_volume,
&info[0].volume);
+
+ if (s->thread_info.soft_muted || !pa_cvolume_is_norm(&volume)) {
+pa_memchunk_make_writable(result, length);
+if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume))
+pa_silence_memchunk(result, &s->sample_spec);
+else
+pa_volume_memchunk(result, &s->sample_spec, &volume);
+ }
+} else {
+void *ptr;
-pa_sink_render_into_full(s, result);
+ result->index = 0;
+ result->memblock = pa_memblock_new(s->core->mempool, length);
+
+ ptr = pa_memblock_acquire(result->memblock);
+
+result->length = pa_mix(info, n,
+(uint8_t*) ptr + result->index, length1st,
+&s->sample_spec,
+&s->thread_info.soft_volume,
+s->thread_info.soft_muted);
+
+pa_memblock_release(result->memblock);
+}
+
+inputs_drop(s, info, n, result);
+
+if (result->length < length) {
+ pa_memchunk chunk;
+ size_t l, d;
+ pa_memchunk_make_writable(result, length);
+
+ l = length - result->length;
+ d = result->index + result->length;
+ while (l > 0) {
+ chunk = *result;
+ chunk.index = d;
+ chunk.length = l;
+
+ pa_sink_render_into(s, &chunk);
+
+ d += chunk.length;
+ l -= chunk.length;
+ }
+ result->length = length;
+}
+
+pa_sink_unref(s);
}
/* Called from main thread */
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] **Fwd: Re: [PATCH] optimization:
On Sat, 9 May 2009, Lennart Poettering wrote: On Thu, 07.05.09 19:55, Jyri Sarha (jyri.sa...@nokia.com) wrote: I have used this fix for quite a while and I am pretty confident about it. However, the performance gain was not what I expected. Thanks. Looks good. Applied. Damn, this was old version of the patch and does not work correctly :(. I'll post the new version shortly. Sorry for the inconvenience. Cheers, Jyri ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluetooth-device: Add safe guard against BT streaming irregularities.
From: Jyri Sarha
Some bad quality BT-headsets block bluez socket sometimes for hundreds
of milliseconds, especially when changing mode. When the module tries
catch up the lost time it may SBC encode up to half a second of audio
without yielding. On slow machine this may cause maximum RT time slice
to be exceeded. Cleaned out the minor fix that slipped into first version
of the patch.
---
src/modules/bluetooth/module-bluetooth-device.c | 22 ++
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-device.c
b/src/modules/bluetooth/module-bluetooth-device.c
index ecb5e83..1b42f4a 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -174,6 +174,8 @@ struct userdata {
#define FIXED_LATENCY_PLAYBACK_HSP (125*PA_USEC_PER_MSEC)
#define FIXED_LATENCY_RECORD_HSP (25*PA_USEC_PER_MSEC)
+#define MAX_PLAYBACK_CATCH_UP_USEC (100*PA_USEC_PER_MSEC)
+
#ifdef NOKIA
#define USE_SCO_OVER_PCM(u) (u->profile == PROFILE_HSP && (u->hsp.sco_sink &&
u->hsp.sco_source))
#endif
@@ -1296,15 +1298,27 @@ static void thread_func(void *userdata) {
if ((!u->source ||
!PA_SOURCE_IS_LINKED(u->source->thread_info.state)) && do_write <= 0 &&
writable) {
pa_usec_t time_passed;
-uint64_t should_have_written;
+pa_usec_t audio_sent;
/* Hmm, there is no input stream we could synchronize
* to. So let's do things by time */
time_passed = pa_rtclock_usec() - u->started_at;
-should_have_written = pa_usec_to_bytes(time_passed,
&u->sample_spec);
-
-do_write = u->write_index <= should_have_written;
+audio_sent = pa_bytes_to_usec(u->write_index,
&u->sample_spec);
+
+if (audio_sent <= time_passed) {
+pa_usec_t audio_to_send = time_passed - audio_sent;
+if (u->write_index > 0 && audio_to_send >
MAX_PLAYBACK_CATCH_UP_USEC) {
+pa_usec_t skip_usec = audio_to_send -
MAX_PLAYBACK_CATCH_UP_USEC;
+uint64_t skip_bytes = pa_usec_to_bytes(skip_usec,
&u->sample_spec);
+pa_memchunk tmp;
+pa_log_warn("Skipping %lld us (= %lld bytes) in
audio stream", skip_usec, skip_bytes);
+pa_sink_render_full(u->sink, skip_bytes, &tmp);
+pa_memblock_unref(tmp.memblock);
+u->write_index += skip_bytes;
+}
+do_write = 1;
+}
}
if (writable && do_write > 0) {
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] optimization: Take samples from silence cache rather than write zeros.
If the only stream to render from is muted take samples from the
silence cache. This should shrink memory/cache bandwidth. Again the
gain was not what I hoped for.
---
src/pulsecore/sink.c | 32 ++--
1 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index d3a6dc9..6cda8c8 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -788,11 +788,17 @@ void pa_sink_render(pa_sink*s, size_t length, pa_memchunk
*result) {
pa_sw_cvolume_multiply(&volume, &s->thread_info.soft_volume,
&info[0].volume);
if (s->thread_info.soft_muted || !pa_cvolume_is_norm(&volume)) {
-pa_memchunk_make_writable(result, 0);
-if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume))
-pa_silence_memchunk(result, &s->sample_spec);
-else
+if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume)) {
+pa_memblock_unref(result->memblock);
+pa_silence_memchunk_get(&s->core->silence_cache,
+s->core->mempool,
+result,
+&s->sample_spec,
+result->length);
+} else {
+pa_memchunk_make_writable(result, 0);
pa_volume_memchunk(result, &s->sample_spec, &volume);
+}
}
} else {
void *ptr;
@@ -969,13 +975,19 @@ void pa_sink_render_full(pa_sink *s, size_t length,
pa_memchunk *result) {
pa_sw_cvolume_multiply(&volume, &s->thread_info.soft_volume,
&info[0].volume);
- if (s->thread_info.soft_muted || !pa_cvolume_is_norm(&volume)) {
-pa_memchunk_make_writable(result, length);
-if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume))
-pa_silence_memchunk(result, &s->sample_spec);
-else
+if (s->thread_info.soft_muted || !pa_cvolume_is_norm(&volume)) {
+if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume)) {
+pa_memblock_unref(result->memblock);
+pa_silence_memchunk_get(&s->core->silence_cache,
+s->core->mempool,
+result,
+&s->sample_spec,
+result->length);
+} else {
+pa_memchunk_make_writable(result, length);
pa_volume_memchunk(result, &s->sample_spec, &volume);
- }
+}
+}
} else {
void *ptr;
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] optimization: Optimized pa_sink_render_full.
I have used this fix for quite a while and I am pretty confident about
it. However, the performance gain was not what I expected.
---
src/pulsecore/sink.c | 78 ++---
1 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 30fa557..d3a6dc9 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -933,22 +933,90 @@ void pa_sink_render_into_full(pa_sink *s, pa_memchunk
*target) {
/* Called from IO thread context */
void pa_sink_render_full(pa_sink *s, size_t length, pa_memchunk *result) {
+pa_mix_info info[MAX_MIX_CHANNELS];
+size_t length1st = length;
+unsigned n;
+
pa_sink_assert_ref(s);
pa_assert(PA_SINK_IS_LINKED(s->thread_info.state));
pa_assert(length > 0);
pa_assert(pa_frame_aligned(length, &s->sample_spec));
pa_assert(result);
+pa_sink_ref(s);
+
pa_assert(!s->thread_info.rewind_requested);
pa_assert(s->thread_info.rewind_nbytes == 0);
-/*** This needs optimization ***/
+pa_assert(length > 0);
+
+n = fill_mix_info(s, &length1st, info, MAX_MIX_CHANNELS);
-result->index = 0;
-result->length = length;
-result->memblock = pa_memblock_new(s->core->mempool, length);
+if (n == 0) {
+ pa_silence_memchunk_get(&s->core->silence_cache,
+ s->core->mempool,
+ result,
+ &s->sample_spec,
+ length1st);
+} else if (n == 1) {
+ pa_cvolume volume;
-pa_sink_render_into_full(s, result);
+ *result = info[0].chunk;
+ pa_memblock_ref(result->memblock);
+
+ if (result->length > length)
+ result->length = length;
+
+ pa_sw_cvolume_multiply(&volume, &s->thread_info.soft_volume,
&info[0].volume);
+
+ if (s->thread_info.soft_muted || !pa_cvolume_is_norm(&volume)) {
+pa_memchunk_make_writable(result, length);
+if (s->thread_info.soft_muted || pa_cvolume_is_muted(&volume))
+pa_silence_memchunk(result, &s->sample_spec);
+else
+pa_volume_memchunk(result, &s->sample_spec, &volume);
+ }
+} else {
+void *ptr;
+
+ result->index = 0;
+ result->memblock = pa_memblock_new(s->core->mempool, length);
+
+ ptr = pa_memblock_acquire(result->memblock);
+
+result->length = pa_mix(info, n,
+(uint8_t*) ptr + result->index, length1st,
+&s->sample_spec,
+&s->thread_info.soft_volume,
+s->thread_info.soft_muted);
+
+pa_memblock_release(result->memblock);
+}
+
+inputs_drop(s, info, n, result);
+
+if (result->length < length) {
+ pa_memchunk chunk;
+ size_t l, d;
+ pa_memchunk_make_writable(result, length);
+ result->length = length;
+
+ l = length - result->length;
+ d = result->index + result->length;
+ while (l > 0) {
+ chunk = *result;
+ chunk.index += d;
+ chunk.length -= d - result->index;
+
+ pa_sink_render_into(s, &chunk);
+
+ d += chunk.length;
+ l -= chunk.length;
+ }
+ result->length = length;
+}
+
+pa_sink_unref(s);
}
/* Called from main thread */
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] bluetooth-device: Add safe guard against BT streaming irregularities.
Some bad quality BT-headsets block bluez socket sometimes for hundreds
of milliseconds, especially when changing mode. When the module tries
catch up the lost time it may SBC encode up to half a second of audio
without yielding. On slow machine this may cause maximum RT time slice
to be exceeded.
---
src/modules/bluetooth/module-bluetooth-device.c | 27 ++
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-device.c
b/src/modules/bluetooth/module-bluetooth-device.c
index ecb5e83..2a70496 100644
--- a/src/modules/bluetooth/module-bluetooth-device.c
+++ b/src/modules/bluetooth/module-bluetooth-device.c
@@ -174,6 +174,8 @@ struct userdata {
#define FIXED_LATENCY_PLAYBACK_HSP (125*PA_USEC_PER_MSEC)
#define FIXED_LATENCY_RECORD_HSP (25*PA_USEC_PER_MSEC)
+#define MAX_PLAYBACK_CATCH_UP_USEC (100*PA_USEC_PER_MSEC)
+
#ifdef NOKIA
#define USE_SCO_OVER_PCM(u) (u->profile == PROFILE_HSP && (u->hsp.sco_sink &&
u->hsp.sco_source))
#endif
@@ -1296,15 +1298,27 @@ static void thread_func(void *userdata) {
if ((!u->source ||
!PA_SOURCE_IS_LINKED(u->source->thread_info.state)) && do_write <= 0 &&
writable) {
pa_usec_t time_passed;
-uint64_t should_have_written;
+pa_usec_t audio_sent;
/* Hmm, there is no input stream we could synchronize
* to. So let's do things by time */
time_passed = pa_rtclock_usec() - u->started_at;
-should_have_written = pa_usec_to_bytes(time_passed,
&u->sample_spec);
-
-do_write = u->write_index <= should_have_written;
+audio_sent = pa_bytes_to_usec(u->write_index,
&u->sample_spec);
+
+if (audio_sent <= time_passed) {
+pa_usec_t audio_to_send = time_passed - audio_sent;
+if (u->write_index > 0 && audio_to_send >
MAX_PLAYBACK_CATCH_UP_USEC) {
+pa_usec_t skip_usec = audio_to_send -
MAX_PLAYBACK_CATCH_UP_USEC;
+uint64_t skip_bytes = pa_usec_to_bytes(skip_usec,
&u->sample_spec);
+pa_memchunk tmp;
+pa_log_warn("Skipping %lld us (= %lld bytes) in
audio stream", skip_usec, skip_bytes);
+pa_sink_render_full(u->sink, skip_bytes, &tmp);
+pa_memblock_unref(tmp.memblock);
+u->write_index += skip_bytes;
+}
+do_write = 1;
+}
}
if (writable && do_write > 0) {
@@ -1822,8 +1836,11 @@ static int start_thread(struct userdata *u) {
#ifdef NOKIA
if (USE_SCO_OVER_PCM(u)) {
-if (start_stream_fd(u) < 0)
+if (start_stream_fd(u) < 0) {
+u->sink = NULL;
+u->source = NULL;
return -1;
+}
pa_sink_ref(u->sink);
pa_source_ref(u->source);
--
1.5.6.3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Couple problems when using 0.9.11 and fixes
Hi,
I found couple of problems when upgrading to pulseaduio 0.9.11. The
both are related to a case when tsched turned off on alsa-sink. First,
problem appears when trying the select a specific number fragment with
module-alsa-sink parameters, for instance this:
load-module module-alsa-sink device=hw:0 rate=44100 fragment_size=882
fragments=2 tsched=0
The above produces three fragments of 588 bytes each. The actual bug
appears to be in alsa-lib, but it can be by-passed without breaking
things when alsa-lib gets fixed. To do that apply
fix_incremented_number_of_fragments.patch.
The other problem appears when trying to use alsa-sink with tsched=0 on a
slow CPU. After writing a set of samples to device alsa-sink checks whether
there is room to write some more samples. On a slow CPU there is always room
for couple of samples more and alsa-sink keeps busy looping in the write
function. "parameter_to_set_minimum_write_size.patch" adds a parameter to
module-alsa-sink to set the minimum number of samples to write on device,
setting this parameter to a reasonable value fixes the above problem,
like this:
load-module module-alsa-sink device=hw:0 rate=44100 fragment_size=882
fragments=2 tsched=0 hwbuf_min_frames_to_write=32
Alsa-source would need a similar patch too. In the end I decided
to forward port the old alsa- sink and source from 0.9.10 to 0.9.11,
because they seem to work more reliably and generate much less load, when
fragment-size and number of fragments is pushed to the minimum and
tsched is not used. I can post those patches too if anybody is interested.
Cheers,
Jyri
===File ~/fix_incremented_number_of_fragments.patch=
diff --git a/src/modules/alsa-util.c b/src/modules/alsa-util.c
index 8abf834..64b4eff 100644
--- a/src/modules/alsa-util.c
+++ b/src/modules/alsa-util.c
@@ -369,12 +369,15 @@ int pa_alsa_set_hw_params(
goto finish;
if (_periods > 0) {
-dir = 1;
-if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams,
&_periods, &dir)) < 0) {
-dir = -1;
-if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle,
hwparams, &_periods, &dir)) < 0)
-goto finish;
-}
+ dir = 0;
+ if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams,
&_periods, &dir)) < 0) {
+ dir = 1;
+ if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle, hwparams,
&_periods, &dir)) < 0) {
+ dir = -1;
+ if ((ret = snd_pcm_hw_params_set_periods_near(pcm_handle,
hwparams, &_periods, &dir)) < 0)
+ goto finish;
+ }
+ }
}
if (_period_size > 0)
===File ~/parameter_to_set_minimum_write_size.patch=
diff --git a/src/modules/module-alsa-sink.c b/src/modules/module-alsa-sink.c
index 8e66f79..0135a0b 100644
--- a/src/modules/module-alsa-sink.c
+++ b/src/modules/module-alsa-sink.c
@@ -69,7 +69,8 @@ PA_MODULE_USAGE(
"tsched= "
"tsched_buffer_size= "
"tsched_buffer_watermark= "
-"mixer_reset=");
+"mixer_reset="
+ "hwbuf_min_frames_to_write=");
static const char* const valid_modargs[] = {
"sink_name",
@@ -86,6 +87,7 @@ static const char* const valid_modargs[] = {
"tsched_buffer_size",
"tsched_buffer_watermark",
"mixer_reset",
+"hwbuf_min_frames_to_write",
NULL
};
@@ -132,6 +134,7 @@ struct userdata {
uint64_t since_start;
snd_pcm_sframes_t hwbuf_unused_frames;
+snd_pcm_sframes_t hwbuf_min_frames_to_write;
};
static void fix_tsched_watermark(struct userdata *u) {
@@ -278,7 +281,7 @@ static int mmap_write(struct userdata *u, pa_usec_t
*sleep_usec) {
if (pa_bytes_to_usec(left_to_play, &u->sink->sample_spec) >
process_usec+max_sleep_usec/2)
break;
-if (PA_UNLIKELY(n <= u->hwbuf_unused_frames))
+if (PA_UNLIKELY(n <= u->hwbuf_min_frames_to_write || n <=
u->hwbuf_unused_frames))
break;
n -= u->hwbuf_unused_frames;
@@ -390,7 +393,7 @@ static int unix_write(struct userdata *u, pa_usec_t
*sleep_usec) {
if (pa_bytes_to_usec(left_to_play, &u->sink->sample_spec) >
process_usec+max_sleep_usec/2)
break;
-if (PA_UNLIKELY(n <= u->hwbuf_unused_frames))
+if (PA_UNLIKELY(n <= u->hwbuf_min_frames_to_write || n <=
u->hwbuf_unused_frames))
break;
n -= u->hwbuf_unused_frames;
@@ -1103,6 +1106,7 @@ int pa__init(pa_module*m) {
pa_bool_t use_mmap = TRUE, b, use_tsched = TRUE, d, mixer_reset = TRUE;
pa_usec_t usec;
pa_sink_new_data data;
+int32_t hwbuf_min_frames_to_write = 0;
snd_pcm_info_alloca(&pcm_info);
@@ -1162,6 +1166,11 @@ int pa__init(pa_module*m) {
goto fail;
}
+if (pa_modargs_get_value_s32(ma, "hwbuf_min_frames_to_write",
&hwbuf_min_frames_to_write) < 0) {
+
Re: [pulseaudio-discuss] Atomic operations on ARM
On Wed, 13 Feb 2008, Lennart Poettering wrote:
On Fri, 01.02.08 15:22, Jyri Sarha ([EMAIL PROTECTED]) wrote:
...
All code I saw that makes use of the kernel helper function calls it
in a loop. (At least that's what I remember)
The same is true in many cases for pa_atomic_cmpxchg too. That is
why the idea of having another loop inside pa_atomic_cmpxchg felt a
bit ugly to me at first.
So what should be done?
1. Change the above line in pulsecore/async.c to use pa_atomic_store
instead and try to look if there are other similar places.
2. Write loops like above to ARM specific implementations atomic
compare and exchange.
I'd certainly vote for #2. I see no real drawbacks on this.
Any way I'll produce a proper ARM atomic ops patch as soon as I am happy
with it. However it may take a while because I am still only learning
the autoconf magic and I have some other tasks I should take care of too.
Every patch greatly appreciated!
Here is my atomic ops patch implemented according to option #2. The patch
should apply cleanly to 0.9.9 release and probably to PA SVN HEAD too. After
applying the patch the source compiles cleanly at least in our scratchbox
based development environment and what is most important the resulting binaries
seem to work correctly.
Arm binaries are usually cross compiled, thus trying to detect CPU or
operating system capabilities at compile time may give bad results. Because
of this I added couple of configure flags so that wanted result can be
forced in all environments. Here are the options:
--enable-atomic-arm-memory-barrier
This should only be needed in SMP systems. Since I am not aware of any ARM
based SMP systems, this is disabled by default.
--disable-atomic-arm-linux-helpers
Disables usage of linux kernel helpers, inline assembler implementation or,
if that fails, libatomic_ops is used instead. By default kernel helpers are
always used when compiling for arm in a linux system.
Cheers,
Jyri
// Jyri Sarha -- [EMAIL PROTECTED]diff --git a/configure.ac b/configure.ac
index 10111e1..e2b584e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -121,6 +121,43 @@ if test "x$GCC" = "xyes" ; then
done
fi
+# Native atomic operation support
+AC_ARG_ENABLE([atomic-arm-linux-helpers],
+AC_HELP_STRING([--disable-atomic-arm-linux-helpers], [use inline asm or
libatomic_ops instead]),
+[
+case "${enableval}" in
+yes) atomic_arm_linux_helpers=yes ;;
+no) atomic_arm_linux_helpers=no ;;
+*) AC_MSG_ERROR(bad value ${enableval} for
--disable-atomic-arm-linux-helpers) ;;
+esac
+],
+[atomic_arm_linux_helpers=auto])
+
+AC_ARG_ENABLE([atomic-arm-memory-barrier],
+AC_HELP_STRING([--enable-atomic-arm-memory-barrier], [only really needed
in SMP arm systems]),
+[
+case "${enableval}" in
+yes) AC_DEFINE_UNQUOTED(ATOMIC_ARM_MEMORY_BARRIER_ENABLED, 1,
[Enable memory barriers]) ;;
+no) ;;
+*) AC_MSG_ERROR(bad value ${enableval} for
--disable-atomic-arm-linux-helpers) ;;
+esac
+],)
+
+AC_MSG_CHECKING([target operating system])
+case $host in
+ *-*-linux*)
+ AC_MSG_RESULT([linux])
+ pulse_target_os=linux
+ ;;
+ *)
+ AC_MSG_RESULT([unknown])
+ pulse_target_os=unknown
+ ;;
+esac
+
+# If everything else fails use libatomic_ops
+need_libatomic_ops=yes
+
AC_MSG_CHECKING([whether $CC knows __sync_bool_compare_and_swap()])
AC_LANG_CONFTEST([int main() { int a = 4; __sync_bool_compare_and_swap(&a, 4,
5); }])
$CC conftest.c $CFLAGS -o conftest > /dev/null 2> /dev/null
@@ -129,8 +166,53 @@ rm -f conftest.o conftest
if test $ret -eq 0 ; then
AC_DEFINE([HAVE_ATOMIC_BUILTINS], 1, [Have __sync_bool_compare_and_swap()
and friends.])
AC_MSG_RESULT([yes])
+need_libatomic_ops=no
else
AC_MSG_RESULT([no])
+# HW specific atomic ops stuff
+AC_MSG_CHECKING([architecture for native atomic operations])
+case $host_cpu in
+arm*)
+ AC_MSG_RESULT([arm])
+ AC_MSG_CHECKING([whether we can use Linux kernel helpers])
+ # The Linux kernel helper functions have been there since 2.6.16.
However
+ # compile time checking for kernel version in cross compile
environment
+ # (which is usually the case for arm cpu) is tricky (or impossible).
+ if test "x$pulse_target_os" = "xlinux" && test
"x$atomic_arm_linux_helpers" != "xno"; then
+ AC_MSG_RESULT([yes])
+ AC_DEFINE_UNQUOTED(ATOMIC_ARM_LINUX_HELPERS, 1, [special arm
linux implementation])
+ need_libatomic_ops=no
+ else
+ AC_MSG_RESULT([no])
+ AC_MSG_CHECKING([compiler support for arm inline asm atomic
[pulseaudio-discuss] Atomic operations on ARM
Hi,
I have written inline assembler implementations of pa_atomic
operations for arm for ARM6 and above. For compatibility with older
ARMs I have also written versions using ARM-Linux kernel helper
functions (see http://0pointer.de/blog#atomic-rt).
The both implementations run almost perfectly now. However there is a
thing to note about compare and exchange implementations for
ARM6 and above. The semantics of the usual ARM ldrex-strexeq instruction
sequence is not identical to x86 implementations of the same thing, e.g.
the exchange is not totally atomic after all.
The strexeq-instruction has two conditions the equality to the old value
and the exclusiveness of the operation (e.g. if the value in memory was
tampered between the operations). The operation fails if either of
these conditions fail, e.g. the value in memory is unchanged. So it is
possible that the old-value-condition is met, but the exclusiveness-
condition fails, but even the tampered memory value would meet the
old-value-condition.
The above applies also to kernel helper implementation of atomic exchange
for ARM6 and above.
Because of the above problem (I suspect) this assertion in pulsecore/async.c
fails sometimes under heavy load:
/* Guaranteed to succeed if we only have a single reader */
pa_assert_se(pa_atomic_ptr_cmpxchg(&cells[idx], ret, NULL));
The assertion failure has happened with the both kernel helper and inline asm
versions (they are identical in ARM6 environment anyway). The failures
are not very common thou.
The atomic compare and exchange can also be written in a way that it retries
the operation if the exclusiveness-condition fails but the equality-condition
was ok, which would resemble real atomicity more. The inline assembler version
would then look like this:
static inline int pa_atomic_cmpxchg(pa_atomic_t *a, int old_i, int new_i) {
unsigned long not_equal, not_exclusive;
pa_memory_barrier();
do {
__asm__ __volatile__("@ pa_atomic_cmpxchg\n"
"1: ldrex %0, [%2]\n"
" subs %0, %0, %3\n"
" mov%1, %0\n"
" strexeq %0, %4, [%2]\n"
: "=&r" (not_exclusive), "=&r" (not_equal)
: "r" (&a->value), "Ir" (old_i), "r" (new_i)
: "cc");
} while(not_exclusive && !not_equal);
pa_memory_barrier();
return !not_equal;
}
A similar kind of external loop can also be added to kernel helper
function, but if the kernel helper in fact makes a systemcall it is
unnecessary. I wonder if all this is worth the trouble.
So what should be done?
1. Change the above line in pulsecore/async.c to use pa_atomic_store
instead and try to look if there are other similar places.
2. Write loops like above to ARM specific implementations atomic
compare and exchange.
Any way I'll produce a proper ARM atomic ops patch as soon as I am happy
with it. However it may take a while because I am still only learning
the autoconf magic and I have some other tasks I should take care of too.
Cheers,
Jyri
// Jyri Sarha -- [EMAIL PROTECTED]
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Rendering data from sink with 0.9.7
< 0) if ((ret = snd_pcm_hw_params_set_access(pcm_handle, hwparams, SND_PCM_ACCESS_RW_INTERLEAVED)) < 0) goto finish; - + else + *use_mmap = 0; } else if ((ret = snd_pcm_hw_params_set_access(pcm_handle, hwparams, SND_PCM_ACCESS_RW_INTERLEAVED)) < 0) goto finish; -else if (*use_mmap) - *use_mmap = 0; if ((ret = set_format(pcm_handle, hwparams, &f)) < 0) goto finish; <\patch> Best regards, Jyri // Jyri Sarha - okuAtikiDotfi ___ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
