On 2015-10-20 06:20, Tanu Kaskinen wrote:
> On Tue, 2015-05-05 at 17:01 +0200, David Henningsson wrote:
>> diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c
>> index 5c55cec..5d278c1 100644
>> --- a/src/modules/module-card-restore.c
>> +++ b/src/modules/module-card-restore.c
>> @@ -375,8 +385,44 @@ finish:
>> return PA_HOOK_OK;
>> }
>>
>> +static const char* profile_name_for_dir(pa_card_profile *cp, pa_direction_t dir) {
>> + if (dir == PA_DIRECTION_OUTPUT && cp->output_name)
>> + return cp->output_name;
>> + if (dir == PA_DIRECTION_INPUT && cp->input_name)
>> + return cp->input_name;
>> + return cp->name;
>> +}
>> +
>> +static void update_profile_for_port(struct entry *entry, pa_card *card, pa_device_port *p) {
>> + struct port_info *p_info;
>> + const char *profilename;
>> +
>> + if (p == NULL)
>> + return;
>> +
>> + if (!(p_info = pa_hashmap_get(entry->ports, p->name))) {
>> + p_info = port_info_new(p);
>> + pa_assert_se(pa_hashmap_put(entry->ports, p_info->name, p_info) >= 0);
>> + }
>> +
>> + profilename = profile_name_for_dir(card->active_profile, p->direction);
>> + if (pa_safe_streq(p_info->profile, profilename))
>> + return;
>> +
>> + pa_xfree(p->preferred_profile);
>> + p->preferred_profile = pa_xstrdup(profilename);
>
> I don't think updating the preferred sink or source belongs in module-
> card-restore. module-card-restore should only concern itself with
> updating the database, and restoring things when new cards appear.
> pa_card_set_profile() seems like a better place to me to set
> pa_device_port.preferred_device (my previous suggestion to name the
> field "preferred_sink" was obviously bad, since we need to cover
> sources too). I'd also use add a function to the pa_device_port API to
> set the preferred device name, to keep writes to the struct
> encapsulated within device-port.c.
Trying to summarize the IRC discussion and add my own reflections:
Moving the assignment to the preferred_profile variable is okay, I don't
mind either way. (That said, I have a slight preference for keeping it
as it is because it opens up for other modules using the field in other
ways.)
I assume that "restoring things when new cards appear" means that the
code in card_new_hook_callback remains as it is (modulo a wrapper
function in device-port.c)?
The rest: changing input_name / output_name to hashmaps of
sinks/sources, and/or changing preferred_profile to
preferred_sink/source (and have correct sink/source names) seems to grow
out of proportion, to the extent that we need to change namereg to
reserve all possible sink names at card creation time.
While the comment that a port prefers a sink/source rather than a
profile is technically correct, I don't see an easy split-up of all this
that can fix some of your comments and doing everything is probably too
much effort for little gain IMO.
Would you like me to send out a new patch set with the assignment moved?
Would it be okay to let the rest of the patches in as they are?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
