Bug#859017: pulseaudio: switch-on-port-available picks unavailable device port

[Felipe Sateler]

Control: forwarded -1 https://bugs.freedesktop.org/show_bug.cgi?id=100451

On Wed, Mar 29, 2017 at 1:51 PM, Vivek Das Mohapatra
 wrote:
> On Wed, 29 Mar 2017, Felipe Sateler wrote:
>
>> Could you forward this issue upstream to
>> pulseaudio-disc...@lists.freedesktop.org ? This looks like
>> something that should be fixed upstream.
>
>
> I've already filed https://bugs.freedesktop.org/show_bug.cgi?id=100451
>
> Is that good enough or should I also ping the list?

That's good enough, thanks! I somehow missed that you had filed this.

I'll wait for that discussion to settle before applying the patch
though. It appears the patched code was correct after all.

-- 

Saludos,
Felipe Sateler

Bug#859017: pulseaudio: switch-on-port-available picks unavailable device port

[Vivek Das Mohapatra]

On Wed, 29 Mar 2017, Felipe Sateler wrote:


Could you forward this issue upstream to
pulseaudio-disc...@lists.freedesktop.org ? This looks like
something that should be fixed upstream.


I've already filed https://bugs.freedesktop.org/show_bug.cgi?id=100451

Is that good enough or should I also ping the list?

Bug#859017: pulseaudio: switch-on-port-available picks unavailable device port

[Felipe Sateler]

Hi,

On Wed, Mar 29, 2017 at 12:14 PM, Vivek Das Mohapatra
 wrote:
> Package: pulseaudio
> Version: 5.0-13
> Severity: normal
>
> Dear Maintainer,
>
>* What led up to the situation?
>
>  Volume/Mute state was not restored on a laptop on reboot:
>  The speaker was always muted.
>
>* What exactly did you do (or not do) that was effective (or
>  ineffective)?
>
>  Set the volume (in either gdm or gnome), rebooted (either from
>  within the gnome session or after logging out).
>
>* What was the outcome of this action?
>
>  Sound (speaker) was always muted on reboot.
>
>* What outcome did you expect instead?
>
>  Volume should have been restored to the value recorded
>  in the pulseaudio .tdb files.
>
> Eventually tracked this down to the wrong device port being selected by
> module-switch-on-port-available - the helper function new_sink_source
> iterates over the device ports and picks the one with the highest priority
> but then, just before returning, does this:
>
> if (p->available != PA_AVAILABLE_NO)
> return NULL;
>
> ie the selected port is returned _only_ if it is NOT available.
>
> This logic error appears to be in pulseaudio 10.x (the stretch version)
> also.
>
> This is used by (for example):
>
>   static pa_hook_result_t sink_new_hook_callback(pa_core *c,
> pa_sink_new_data *new_data, void *u) {
>
>   pa_device_port *p = new_sink_source(new_data->ports,
> new_data->active_port);
>
>   if (p) {
>   pa_log_debug("Switching initial port for sink '%s' to '%s'",
> new_data->name, p->name);
>   pa_sink_new_data_set_port(new_data, p->name);
>   }
>   return PA_HOOK_OK;
>   }
>
> So this module will never choose an available port.
>
> The p->available != PA_AVAILABLE_NO pattern appears all over the place in
> this module but I _think_ this is the only place where it's used the
> wrong way round.
>
> The following fixed the symptom here (and the pa - log indicated that
> a more sensible set of choices was being made with it applied):
>
> --- a/src/modules/module-switch-on-port-available.c
> +++ b/src/modules/module-switch-on-port-available.c
> @@ -256,7 +256,7 @@
>  p = i;
>  if (!p)
>  return NULL;
> -if (p->available != PA_AVAILABLE_NO)
> +if (p->available == PA_AVAILABLE_NO)
>  return NULL;
>
>  pa_assert_se(p = find_best_port(ports));
>

Your analysis looks correct to me. Could you forward this issue
upstream to pulseaudio-disc...@lists.freedesktop.org ? This looks like
something that should be fixed upstream.


-- 

Saludos,
Felipe Sateler

Bug#859017: pulseaudio: switch-on-port-available picks unavailable device port

[Vivek Das Mohapatra]

Package: pulseaudio
Version: 5.0-13
Severity: normal

Dear Maintainer,

   * What led up to the situation?

 Volume/Mute state was not restored on a laptop on reboot:
 The speaker was always muted.

   * What exactly did you do (or not do) that was effective (or
 ineffective)?

 Set the volume (in either gdm or gnome), rebooted (either from
 within the gnome session or after logging out).

   * What was the outcome of this action?

 Sound (speaker) was always muted on reboot.

   * What outcome did you expect instead?

 Volume should have been restored to the value recorded
 in the pulseaudio .tdb files.

Eventually tracked this down to the wrong device port being selected by
module-switch-on-port-available - the helper function new_sink_source
iterates over the device ports and picks the one with the highest priority
but then, just before returning, does this:

if (p->available != PA_AVAILABLE_NO)
return NULL;

ie the selected port is returned _only_ if it is NOT available.

This logic error appears to be in pulseaudio 10.x (the stretch version)
also.

This is used by (for example):

  static pa_hook_result_t sink_new_hook_callback(pa_core *c, pa_sink_new_data 
*new_data, void *u) {

  pa_device_port *p = new_sink_source(new_data->ports, 
new_data->active_port);

  if (p) {
  pa_log_debug("Switching initial port for sink '%s' to '%s'", 
new_data->name, p->name);
  pa_sink_new_data_set_port(new_data, p->name);
  }
  return PA_HOOK_OK;
  }

So this module will never choose an available port.

The p->available != PA_AVAILABLE_NO pattern appears all over the place in
this module but I _think_ this is the only place where it's used the
wrong way round.

The following fixed the symptom here (and the pa - log indicated that
a more sensible set of choices was being made with it applied):

--- a/src/modules/module-switch-on-port-available.c
+++ b/src/modules/module-switch-on-port-available.c
@@ -256,7 +256,7 @@
 p = i;
 if (!p)
 return NULL;
-if (p->available != PA_AVAILABLE_NO)
+if (p->available == PA_AVAILABLE_NO)
 return NULL;

 pa_assert_se(p = find_best_port(ports));


-- System Information:
Debian Release: 8.7
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386