[pulseaudio-discuss] card: Only set active_profile with available profile

[Colin Guthrie]

'Twas brillig, and Tanu Kaskinen at 05/11/13 20:34 did gyre and gimble:
>  src/pulsecore/card.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> New commits:
> commit f434087e42bca15fae938f6cc01d2875c4c7728b
> Author: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
> Date:   Sun Nov 3 15:05:34 2013 +0200
> 
>     card: Only set active_profile with available profile
>     
>     When a card is being created and no profile has been assigned
>     pa_card_new will attempt to select one from the list but it does that
>     without checking the available flag which can lead to select profiles
>     not available.
> 
> diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
> index e6e0836..4ae16c2 100644
> --- a/src/pulsecore/card.c
> +++ b/src/pulsecore/card.c
> @@ -195,9 +195,14 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
>              c->save_profile = data->save_profile;
>  
>      if (!c->active_profile) {
> -        PA_HASHMAP_FOREACH(profile, c->profiles, state)
> +        PA_HASHMAP_FOREACH(profile, c->profiles, state) {
> +            if (profile->available == PA_AVAILABLE_NO)
> +                continue;
> +
>              if (!c->active_profile || profile->priority > c->active_profile->priority)
>                  c->active_profile = profile;
> +        }
> +        pa_assert(c->active_profile);

Doesn't this leave the possibility of a crash if no active profiles exist?

Wouldn't it make more sense to fall back to setting it to *something*
even if it is unavailable?

As a side note, would it be better to prefer PA_AVAILABLE_YES profiles
over PA_AVAILABLE_UNKNOWN ones?

I'll post two patches which does this (implements each of the above two
paragraphs), but as I don't know the bluez side of things (which is the
only case where any value other than PA_AVAILABLE_UNKNOWN is assigned),
this could be total garbage - i.e. if we absolutely know that bluez will
always give us at least one profile that is not unavailable then the
first patch is certainly not needed - some variation of the second one
might still be desirable tho'.




Additionally, this principle (of picking an appropriate profile based on
availability) could also apply to sinks/sources too when picking their
ports. It might not be absolutely needed with helper modules that can do
some of this task, but it still makes sense IMO.

I'll also include two more patches there - one to prefer either unknown
or available ports over known unavailable, and a second to check them in
order - known-available -> unknown -> any.

If they make sense (David is probably best equipped to comment as I
guess he knows the alsa jack states best!) if the 2nd part of that patch
makes sense in the real world. If you want to push them, then feel free
to squash them into one firstfirst.

Cheers!

Col

-- 

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/