Re: [pulseaudio-discuss] [PATCH 0/4] Some bug fixes
On 03/14/2013 09:07 PM, Tanu Kaskinen wrote: I investigated a bug in module-loopback reported[1] by Frédéric Dalleau last year. The instructions for reproduction were as follows: 1. Have one sound card (index 0) loaded with one sink (index 0) and one source (index 1), plus the monitor source (index 0) 2. pactl load-module module-loopback sink=0 source=0 3. pactl set-card-profile 0 off These steps caused reliable crashing, and after fixing the first issue, two more cropped up. The first three patches fix these three issues. When I investigated in more detail the exact conditions that caused the first crash, I found out that module-alsa-card was doing something that it shouldn't do: it was moving streams. The last patch fixes that. Removing the stream moving from module-alsa-card causes a change in behavior that perhaps isn't acceptable: if a sink is removed due to a profile change and a new sink is created, the streams connected to the old sink aren't necessarily moved to the new sink. If this is unacceptable, the old policy should be reimplemented somewhere else, perhaps in module-rescue-streams. Yes, I think we need to reimplement this, otherwise it would be a regression. (Yet another hack just because the Real Routing System (tm) isn't finished, argh.) [1] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/13408 Tanu Kaskinen (4): loopback: Fix segfault in may_move_to() callbacks filter-apply: Fix segfault with moving streams loopback: Flush asyncmsgq from the right context alsa: Don't move streams when changing profiles src/modules/alsa/module-alsa-card.c | 19 src/modules/module-filter-apply.c | 10 +-- src/modules/module-loopback.c | 55 --- 3 files changed, 45 insertions(+), 39 deletions(-) -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 7/9] proplist: Add pa_proplist_update_info
On 03/14/2013 03:58 PM, Tanu Kaskinen wrote: On Thu, 2013-03-14 at 14:58 +0100, David Henningsson wrote: On 02/20/2013 07:24 PM, Tanu Kaskinen wrote: I was writing function pa_device_port_update_proplist(), and I wanted it to send change notifications only if the proplist actually changes. pa_proplist_update() doesn't provide any indication about whether the proplist changed or not, so some kind of a solution was needed. The simple solution would be to create a copy of the port proplist before calling pa_proplist_update() and check if the copy equals the port proplist after calling pa_proplist_update(). That felt overly wasteful, however: it would mean copying the whole property list and comparing every property in it whenever someone changes even just one property. So, I invented a more complex solution: a pa_proplist_update_info object that holds a description of per-property operations to be applied to a property list. pa_proplist_apply_update_info() iterates through the operations and applies them one by one, keeping track of whether the operations cause actual changes. I guess that's one way to solve it. I would probably have gone for the slightly simpler solution of just keeping a flag inside the proplist. The proplist object itself will set the flag whenever a real change occurs, and it can be manually reset by just calling, say pa_proplist_reset_change_flag() or so. That sounds pretty sensible. Do you think I should redo the patches? I'd prefer not to do that, but that's just because I'm lazy. You mean, it's just because you want to get more time for fixing other PulseAudio issues ;-) Anyway, I'm pragmatic. As long a patch improves the current condition and does not cause regressions, APIs we have to commit to in the future, or other implications, I don't mind it going in. It can be rewritten, simplified, or whatever, by someone else later if they feel like it. --- src/map-file |4 + I don't think we need to add this to the external API unless somebody complains about missing that feature. Fair enough, I don't mind hiding this from clients. Ok. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 6/9] device-port: Add linked flag
On 03/14/2013 04:17 PM, Tanu Kaskinen wrote: On Thu, 2013-03-14 at 15:02 +0100, David Henningsson wrote: On 02/20/2013 07:24 PM, Tanu Kaskinen wrote: The flag will be used in the upcoming pa_device_port_update_proplist() function. If the flag's function is to protect against an initial change notification, does it really work? init_eld_ctls is called after pa_card_new in module-alsa-card.c. Good observation. It looks like we need to separate pa_card_put() from pa_card_new(), because init_eld_ctls() needs the card object, so moving it before the pa_card_new() call. If we need a flag at all, I'd prefer to keep it in the card struct rather than the port. That seems cleaner to me. OK, I don't necessarily agree with this, but there aren't any big issues with reusing the card state as the port state. Since it's the card we actually send a notification for, following the card's state seems more logical to me. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 6/9] device-port: Add linked flag
On Fri, 2013-03-15 at 15:43 +0100, David Henningsson wrote: On 03/14/2013 04:17 PM, Tanu Kaskinen wrote: On Thu, 2013-03-14 at 15:02 +0100, David Henningsson wrote: On 02/20/2013 07:24 PM, Tanu Kaskinen wrote: The flag will be used in the upcoming pa_device_port_update_proplist() function. If the flag's function is to protect against an initial change notification, does it really work? init_eld_ctls is called after pa_card_new in module-alsa-card.c. Good observation. It looks like we need to separate pa_card_put() from pa_card_new(), because init_eld_ctls() needs the card object, so moving it before the pa_card_new() call. If we need a flag at all, I'd prefer to keep it in the card struct rather than the port. That seems cleaner to me. OK, I don't necessarily agree with this, but there aren't any big issues with reusing the card state as the port state. Since it's the card we actually send a notification for, following the card's state seems more logical to me. That's a native protocol specific quirk. The D-Bus protocol would send the notification for the port, if it was implemented (the D-Bus protocol doesn't currently send any notification). I would prefer to do the same in the native protocol too, if changing that wouldn't have compatibility issues. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Cmake for PA
Hey, I want to use cmake to build Pulse Audio , not sure if there is one that exists today. Any input is appreciated as to how can i do it . thanks, Keith ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
