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
