[pulseaudio-discuss] [PATCH 8/8] source: When updating a monitor source's rate, update the sink rate too

[David Henningsson]

On 08/09/2013 08:57 AM, Tanu Kaskinen wrote:
> ---
>  src/pulsecore/source.c | 39 +++++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
> index 5784a78..b3a55b0 100644
> --- a/src/pulsecore/source.c
> +++ b/src/pulsecore/source.c
> @@ -972,8 +972,6 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
>      uint32_t desired_rate = rate;
>      uint32_t default_rate = s->default_sample_rate;
>      uint32_t alternate_rate = s->alternate_sample_rate;
> -    uint32_t idx;
> -    pa_source_output *o;
>      bool use_alternate = false;
>  
>      if (rate == s->sample_spec.rate)
> @@ -1034,19 +1032,44 @@ bool pa_source_update_rate(pa_source *s, uint32_t rate, bool passthrough) {
>      pa_log_debug("Suspending source %s due to changing the sample rate.", s->name);
>      pa_source_suspend(s, true, PA_SUSPEND_INTERNAL);
>  
> -    if (!s->update_rate || s->update_rate(s, desired_rate) == true) {
> -        if (!s->update_rate) {
> -            pa_assert(s->monitor_of);
> -            s->sample_spec.rate = desired_rate;
> +    if (s->update_rate) {
> +        if (s->update_rate(s, desired_rate) == true) {
> +            pa_log_info("Changed sampling rate successfully");
> +            ret = true;
> +        }
> +    } else {

Yes, this looks better, although "== true" looks ugly

if (s->update_rate) {
  ret = s->update_rate(s, desired_rate);
  if (ret)
    pa_log_info("Changed sampling rate successfully");
} else {
  /* Monitor sources */

...a comment is more readable than an assertion, but that might be a
matter of taste. Especially so as it is not at the top, but below a
variable declaration.


> +        uint32_t old_rate = s->sample_spec.rate;
> +
> +        pa_assert(s->monitor_of);
> +        s->sample_spec.rate = desired_rate;
> +
> +        /* XXX: This code is written with non-passthrough streams in mind. I
> +         * have no idea whether the behaviour with passthrough streams is
> +         * sensible. */
> +        if (!passthrough) {
> +            if (pa_sink_update_rate(s->monitor_of, desired_rate, false) == true) {
> +                pa_log_info("Changed sampling rate successfully");
> +                ret = true;
> +            } else {
> +                /* Changing the sink rate failed, roll back the old rate for
> +                 * the monitor source. We set the source rate before calling
> +                 * pa_sink_update_rate(), because that makes the
> +                 * rate == s->sample_spec.rate check in the beginning of this
> +                 * function return early, when pa_sink_update_rate() tries to
> +                 * update the monitor source rate. */
> +                s->sample_spec.rate = old_rate;
> +            }
>          }
> +    }
>  
> -        pa_log_info("Changed sampling rate successfully ");
> +    if (ret == true) {

if (ret) {

> +        uint32_t idx;
> +        pa_source_output *o;
>  
>          PA_IDXSET_FOREACH(o, s->outputs, idx) {
>              if (o->state == PA_SOURCE_OUTPUT_CORKED)
>                  pa_source_output_update_rate(o);
>          }
> -        ret = true;
>      }
>  
>      pa_source_suspend(s, false, PA_SUSPEND_INTERNAL);
> 



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic