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

[Luiz Augusto von Dentz]

Hi Tanu,

On Tue, Nov 19, 2013 at 9:37 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Mon, 2013-11-18 at 16:51 +0200, Luiz Augusto von Dentz wrote:
>> Hi Tanu,
>>
>> On Mon, Nov 18, 2013 at 9:25 AM, Tanu Kaskinen
>> <tanu.kaskinen at linux.intel.com> wrote:
>> > On Sun, 2013-11-17 at 17:53 +0200, Luiz Augusto von Dentz wrote:
>> >> Hi Colin, Tanu,
>> >>
>> >> On Sun, Nov 17, 2013 at 3:17 PM, Colin Guthrie <gmane at colin.guthr.ie> wrote:
>> >> > 'Twas brillig, and Tanu Kaskinen at 16/11/13 08:51 did gyre and gimble:
>> >> >> On Fri, 2013-11-15 at 09:31 +0100, Colin Guthrie wrote:
>> >> >> I haven't concentrated on the patches enough to even know what
>> >> >> problem you're solving - is it just that the assertion makes you
>> >> >> nervous, or is there some other benefit.
>> >> >
>> >> > I'm not solving any specific problem per-se. I'm just looking at several
>> >> > bugs where incorrect profiles appear to be selected by default on first
>> >> > boot where HDMI is picked over regular analog output (for those alsa
>> >> > cards which share a single card for both HDMI and Analog output rather
>> >> > than having it presented as separate cards) despite HDMI having a lower
>> >> > priority. This lead me to look at this code and got me thinking - this
>> >> > is the only reason for the patch.
>> >> >
>> >> > A further complication to my current bugs is that sometimes Headphone
>> >> > *ports* are picked by default on first boot even when they are
>> >> > unavailable and again are not the highest priority port. It seems like
>> >> > the same problem but in a different, but similar, part of the code. This
>> >> > is why the second set of patches applied the same logical fixes to the
>> >> > Sink/Source code that Luiz added to the Card profiles. I think it makes
>> >> > sense to keep these bits of code approximately in sync regardless of the
>> >> > outcome of this discussion - the same logic should apply.
>> >>
>> >> I can try and apply the same logic to Sink/Source, it just that
>> >> overall this kind of policy should be in one place or then remove it
>> >> altogether and make sure the list is always ordered by their module so
>> >> the core should just pick the first one, the problem we were trying to
>> >> solve was that there is no hook for priority changes thus they are
>> >> considered static which is probably why we are relying on
>> >> pa_card_set_profile_available +
>> >> PA_CORE_HOOK_CARD_PROFILE_AVAILABLE_CHANGED to indicate the profiles
>> >> changes.
>> >>
>> >> So perhaps we should came to an agreement where and how we should be
>> >> doing this sort of policy, we could perhaps generalize
>> >> module-bluetooth-policy, or do we want to continue with per technology
>> >> policy?
>> >
>> > I don't think the policy in module-bluetooth-policy is useful outside
>> > Bluetooth. module-bluetooth-policy is mainly concerned with the
>> > transport state in its policy, and the profile availability is only used
>> > as a proxy for the transport state. I'd be happy if
>> > module-bluetooth-policy would be changed so that instead of using the
>> > profile states, it would directly track the transport states using the
>> > APIs that pa_bluez_discovery provides. But since using the profile
>> > states as a proxy works fine too, I'm not really asking anyone to do
>> > anything about that.
>> >
>> > From the proposed changes, I think Colin's patches 1 and 3 make sense.
>> > Preferring PA_AVAILABLE_YES over PA_AVAILABLE_UNKNOWN in generic code
>> > isn't a good idea.
>>
>> Im having a hard time to understand why any policy would prefer a
>> profile that is less likely to work than another,
>
> If you have a "bad" (not very well suited for the use case) profile and
> a "good" (very well suited for the use case) profile, and the
> probability of the "bad" profile working is 100% and the probability of
> the "good" profile working is 99%, do you really see no reason to choose
> the "good" profile?

Well there is nothing to classify a profile as good/bad, is there?
Also the chance can be 99% or 1%, so it is not deterministic. My
argument is that pa_card_new don't have this information so it should
not pretend it knows best and select a profile with available set to
unknown.

>> but perhaps we are
>> not willing the mess with other parts of the code that were using this
>> information in a different way. But then why we are bothering
>> selecting any profile on pa_card_new? This is a policy decision and it
>> seems we are not able to do a generic one so it should not even try,
>> pick the first and be done with it.
>
> Yes, it's a policy decision. I would prefer pa_card_new() to always
> choose the "off" profile by default, but that would require work in
> policy modules to keep old use cases working. For example, on the first
> boot of a PC something else than "off" should be selected for the sound
> card, and currently we don't have any policy module that would ensure
> that.

And nothing fits the "good" profile that works 99% of the time? I
guess that should be marked as available 'yes', or perhaps the modules
should prefer something when they create the card with
pa_card_new_data_set_profile but then we perhaps need to change when
PA_CORE_HOOK_CARD_NEW is fired otherwise it might break things like
module-card-restore.

>> Anyway if pa_card_new do not select 'off' by default it pretty much
>> breaks Bluetooth qualification and if I change the priority of 'off'
>> profile to force it to be selected it breaks module-bluetooth-policy.
>
> Can you tell the Bluetooth qualification requirement that is failing? If
> it's something HFGW specific, you should be able to implement the
> desired policy in module-bluetooth-policy, without modifying
> pa_card_new().

The qualification requires that no audio connection should be open in
certain tests, but right now pa_card_new will select 'hfgw' as it has
the highest priority thus creating a SCO connection.
module-bluetooth-policy doesn't use PA_CORE_HOOK_CARD_NEW perhaps you
want it to use it to select it to 'off' before that. Still this will
only work for modules-bluetooth-policy and internal modules, external
policy components are at the mercy of pa_card_new auto selecting the
right profile.

-- 
Luiz Augusto von Dentz