On 2015-10-21 17:26, Tanu Kaskinen wrote:
On Tue, 2015-10-20 at 15:25 +0200, David Henningsson wrote:
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)?
Yes.
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?
So you think it's ok to have pa_device_port.preferred_profile, even if
the field doesn't actually refer to a profile? I'm definitely not ok
with that.
The field refers to either:
1) one or more profiles' input_name, or
2) one or more profiles' output_name, or
3) a profile's name
Hence, in its current form, it does refer to one or more profiles. Given
that
"preferred_profile_name_or_profile_input_name_or_profile_output_name"
was too long, I shortened it to "preferred_profile". Do you have a
better name suggestion for the field name?
But if you think it's "too much effort for little gain" to
make the code correct,it's probably the least hassle if we apply your
patches, and I fix it up, since I have more motivation, assuming that
you won't be blocking patches that add name reservation to the namereg
etc.
Ok, that sounds like a compromise I can agree on.
(Even though I disagree with your wording - I don't think my code is
incorrect just because it doesn't improve the situation for multi-sink
profiles.)
(By the way, name reservation is something that I have wanted on
several occasions, because it has caused trouble that we can't rely on
the name in the foo_new_data struct to be the final object name.)
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
