[pulseaudio-discuss] [PATCH v2 2/2] switch-on-port-available: deactivate direction when the no ports are available for that direction

[Georg Chini]

On 29.12.2017 13:49, Tanu Kaskinen wrote:
> On Fri, 2017-12-29 at 08:25 +0100, Georg Chini wrote:
>> On 28.12.2017 16:14, Tanu Kaskinen wrote:
>>> This ensures that streams are moved away from unavailable outputs or
>>> inputs. For example, sometimes HDMI is on a dedicated alsa card, and if
>>> all HDMI outputs become unavailable, then the card profile will be set
>>> to "off", and the streams will be moved somewhere else.
>>> ---
>> I am not sure if changing the profile is a good idea. There is no
>> code which restores the profile when the cable gets plugged again.
>> Can't you fire a hook when a sink or source is suspended due to
>> PA_SUSPEND_UNAVAILABLE and let module-rescue-stream handle
>> the case?
> When the HDMI port becomes available, module-switch-on-port-available
> will change the profile as appropriate.

OK, I would have expected that jack state changes are ignored
once the card is switched to "off" but I see this is not the case.

>
> There's still the problem that once the HDMI sink is available again,
> streams won't be moved there automatically (unless module-switch-on-
> connect is loaded), but I think that's a lesser problem than keeping
> streams connected to a silent sink.
>
>> Further comment below.
>>> Changes in v2:
>>>    - Don't call deactivate_direction() unconditionally.
>>>
>>>
>>>    src/modules/module-switch-on-port-available.c | 40 ++++++++++++++++++++++++++-
>>>    1 file changed, 39 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/modules/module-switch-on-port-available.c b/src/modules/module-switch-on-port-available.c
>>> index 8fd3c9e5f..c8e3d552b 100644
>>> --- a/src/modules/module-switch-on-port-available.c
>>> +++ b/src/modules/module-switch-on-port-available.c
>>> @@ -80,7 +80,7 @@ static bool profile_good_for_output(pa_card_profile *profile, pa_device_port *po
>>>        if (card->active_profile->max_source_channels != profile->max_source_channels)
>>>            return false;
>>>    
>>> -    if (port == card->preferred_output_port)
>>> +    if (port && port == card->preferred_output_port)
>>>            return true;
>>>    
>>>        PA_IDXSET_FOREACH(sink, card->sinks, idx) {
>>> @@ -251,6 +251,42 @@ static void switch_to_port(pa_device_port *port) {
>>>            pa_sink_set_port(pp.sink, port->name, false);
>>>    }
>>>    
>>> +static void deactivate_direction(pa_card *card, pa_direction_t direction) {
>>> +    pa_card_profile *profile;
>>> +    void *state;
>>> +    pa_card_profile *best_profile = NULL;
>>> +    const char *direction_str;
>>> +
>>> +    PA_HASHMAP_FOREACH(profile, card->profiles, state) {
>>> +        switch (direction) {
>>> +            case PA_DIRECTION_OUTPUT:
>>> +                if (profile->n_sinks != 0)
>>> +                    continue;
>>> +                if (!profile_good_for_output(profile, NULL))
>> Should this not be profile_good_for_input()?
> No. profile_good_for_output() is called when modifying the output
> configuration. The function is used to check that the input side
> doesn't change. I'm occasionally confused about the function naming
> too, so it would be good to make it more clear...
>
I see, thanks.

One other question: Is it safe to pass NULL as port to profile_good_for_*()?
There is a comparison in the function which uses port->priority without
checking if port is NULL.