Nice thanks for this Jyri. I was actually going to ping you to make sure you got my comments as I don't think I CC'ed you directly, just replied to the list. Anyway, I'll grab these in... likely over the weekend as I've got a few things piling up now! Col 'Twas brillig, and oku at iki.fi at 13/01/11 14:44 did gyre and gimble: > From: Jyri Sarha <jyri.sarha at nokia.com> > > On Wed, 22 Dec 2010, Colin Guthrie wrote: >> 'Twas brillig, and oku at iki.fi at 20/12/10 16:47 did gyre and gimble: > ... >>> >>> + pa_xfree(l->name); >>> pa_xfree(l); >>> } >> >> The assert on l->name on free is probably overkill as pa_xfree will >> silently ignore nulls anyway, so it would do no harm. That and it's >> pretty much impossible for l->name to be null anyway... >> >> OK to drop that assert? > > Assert dropped. > > On Wed, 22 Dec 2010, Colin Guthrie wrote: > >> 'Twas brillig, and oku at iki.fi at 20/12/10 16:47 did gyre and gimble: >>> From: Jyri Sarha <jyri.sarha at nokia.com> > ... >>> } >> >> While I'm not 100% sure here, I think the mixer_callback is needed when >> u->sink->flags & PA_SINK_HW_MUTE_CTRL, but you now only activate this >> when u->sink->flags & PA_SINK_HW_VOLUME_CTRL. While it's quite unlikely >> that some h/w exsist that has PA_SINK_HW_MUTE_CTRL but not >> PA_SINK_HW_VOLUME_CTRL I think this should either be set unconditionally >> (as before) or at least within an if block that checks for either flag. >> > > Yes, you are right about that. I changed the condition to check for the > both flags. > > I also rebased the patches against the lastest and run couple of brief tests > with it. > > Cheers, > Jyri > > The old cover letter contents: >> These are all pretty obvious fixes, but maybe the "core: Use >> volume_change_safety_margin when rewinding sync-volume events" >> >> A volume change always triggers a rewinding on the sink. This is >> sometimes a problem when the flat-volume is turning HW volume down and >> compensating with SW volume. The rewind gets the samples with the new >> SW-volume very close to HW-pointer and we may not have enough headroom >> to process rewinding on the streams and monitor source before the >> first samples are played out. >> >> With the patch I first of all rewind the volume changes differently >> depending on whether volume is going up or down. The volume events are >> rewound as soon as possible after DMA-buffer rewinding and immediately >> after it there is a check if we should write something to HW. > > Jyri Sarha (5): > core: Change sematics of pa_flist_new_with_name() (v1.1) > core: Use volume_change_safety_margin when rewinding sync-volume > events > core: Use pa_sink_get_latency_within_thread() in sync-volume code > alsa-sink: Fix double use of string > alsa-sink: Don't assume we were able to enable hw-volume or > sync-volume (v1.1) > > src/modules/alsa/alsa-sink.c | 56 +++++++++++++++++++++-------------------- > src/pulsecore/flist.c | 4 ++- > src/pulsecore/flist.h | 4 +- > src/pulsecore/sink.c | 33 +++++++++++++----------- > 4 files changed, 52 insertions(+), 45 deletions(-) -- 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/]
