[pulseaudio-discuss] [PATCH 4/7] card: move profile selection after pa_card_new()

[Tanu Kaskinen]

On Tue, 2015-10-27 at 09:20 +0100, David Henningsson wrote:
> 
> On 2015-10-27 07:27, Tanu Kaskinen wrote:
> > On Mon, 2015-10-26 at 15:15 +0100, David Henningsson wrote:
> > > 
> > > On 2015-10-23 12:56, Tanu Kaskinen wrote:
> > > > I want module-alsa-card to set the availability of unavailable
> > > > profiles before the initial card profile gets selected, so that the
> > > > selection logic can use correct availability information.
> > > > module-alsa-card initializes the jack state after calling
> > > > pa_card_new(), however, and the profile selection happens in
> > > > pa_card_new(). This patch solves that by introducing pa_card_put() and
> > > > moving the profile selection code there.
> > > 
> > > I think it's a good idea to split card_new into card_new and card_put.
> > > 
> > > > An alternative solution would have been to move the jack
> > > > initialization to happen before pa_card_new() and use pa_card_new_data
> > > > instead of pa_card in the jack initialization code, but I disliked
> > > > that idea (I want to get rid of the "new data" pattern eventually).
> > > > 
> > > > The CARD_NEW hook is used when applying the initial profile policy, so
> > > > that was moved to pa_card_put().
> > > 
> > > So now CARD_NEW is called from CARD_PUT, and they are called almost
> > > immediately after one another (the only difference is the card state).
> > > 
> > > The only module using CARD_NEW and CARD_PUT hooks is
> > > module-card-restore, so the only reason to move it is to accommodate the
> > > needs of module-card-restore. And here I'm lost - I'm not understanding
> > > why you need to move CARD_NEW.
> > > 
> > > As an option, if we set the state to linked *after* the CARD_PUT hook,
> > > would that help?
> > 
> > The point of CARD_PUT is to fire after the card is fully initialized.
> > It's used by module-dbus-protocol in addition to module-card-restore.
> > 
> > I moved the "profile" modarg handling in alsa and bluetooth after the
> > pa_card_put() call, which I now realize was a bad idea; the initial
> > profile should be set before pa_card_put(), because the initial profile
> > has to be finalized before CARD_PUT is fired.
> > 
> > I don't really understand what you're proposing above,
> 
> I'm not sure I had a complete proposal, but I think CARD_NEW should be 
> fired from pa_card_new, because firing CARD_NEW from pa_card_put would 
> be confusing.

Is the hook name the source of the confusion? The hook name can easily
be changed. I'd prefer splitting it into CARD_SET_INITIAL_PROFILE and
CARD_SET_INITIAL_LATENCY_OFFSETS, but I'm not sure you like that, since
it adds more code that isn't strictly necessary. An alternative
suggestion: CARD_SET_INITIAL_ATTRIBUTES would mean only one hook. (Note
that both suggestions imply that CARD_NEW wouldn't exist anymore, since
it's not needed.)

> What I was also thinking was that maybe one could use the 
> PA_HOOK_EARLY/LATE to do different stages of the process, so that 3.2 
> below could be done on CARD_PUT + PA_HOOK_EARLY and what you call 4. 
> below could be done on CARD_PUT + PA_HOOK_LATE. But maybe that would be 
> confusing too.
> 
> > but I can
> > explain why CARD_NEW had to be moved: CARD_NEW is part of the "select
> > initial profile" process, and selecting the initial profile needs to
> > happen after the profile availability is figured out. The profile
> > availability is figured out after pa_card_new(), and therefore CARD_NEW
> > had to be moved out of pa_card_new().
> > 
> > Things should happen in this order:
> > 1. pa_card is allocated
> > 2. profile availability is figured out
> > 3. the initial profile is selected
> > 3.1. the core chooses the default profile
> > 3.2. policy modules override the core (CARD_NEW)
> > 3.3. backend overrides the policy modules based on user input (the
> > "profile" modarg)
> > 4. CARD_PUT is fired
> 
> So with the old order, it's natural to set the "profile" modarg between 
> pa_card_new and pa_card_put. Where is it supposed to be set with the new 
> order?

I suggest adding pa_card_apply_initial_profile_policy() that is called
between pa_card_new() and pa_card_put(), if the "profile" modarg isn't
given. That's not the least-code option, though. As a simpler
alternative, the "profile" modarg could also be given as an argument to
pa_card_put().

> Also, should a "profile" modarg reset save_profile? Otherwise we might 
> end up saving something that was not a user interaction?

Yes, it should do that.

-- 
Tanu