On 11/10/2011 09:34 PM, Tanu Kaskinen wrote:
Hi,
Some more comments on this last patch. Still not finished, but I've got
to go to sleep...
I hope I'm not driving you folks too hard :-/
I got a bit confused about the path naming, or more specifically about
the uniqueness of the path names. I didn't have time to double check,
but it seemed like in the old system there was no reason to have the
path_set_make_paths_unique() function at all, because it looked like the
path names would be unique anyway (within one path set). I'm pretty sure
that I've missed something...
Two different path files can have the same name, as "name" refers to the
"Name" key in the "General" section, not the file name. Does that clear
things up?
Since same paths are used for multiple mappings, in the new system there
clearly will be duplicate path names for one card.
But this is the same path object, used by more than one mapping. So the
duplicates should not increase.
But if the only
reason for that is that different mappings reuse the same path
configuration files, I think it's not a good idea to just append a
number at the end of the path description that will be shown in UIs. I
think the path descriptions could incorporate the mapping description or
something, instead of using just numbers to tell the paths apart... But
I didn't have the time to double check this, so I'm probably missing
something.
On Thu, 2011-10-27 at 16:45 +0200, David Henningsson wrote:
+static void mapping_paths_probe(pa_alsa_mapping *m, pa_alsa_profile *profile,
+ pa_alsa_direction_t direction) {
+
+ pa_alsa_path *p;
+ void *state;
+ snd_pcm_t *pcm_handle;
+ pa_alsa_path_set *ps;
+ snd_mixer_t *mixer_handle;
+
+ if (direction == PA_ALSA_DIRECTION_OUTPUT) {
+ if (m->output_path_set)
+ return; /* Already probed */
+ m->output_path_set = ps = pa_alsa_path_set_new(m, direction, NULL); /*
FIXME: Handle paths_dir */
The path set should be created already at configuration parsing time.
There shouldn't be need to create anything at probe time - I think the
purpose of probing is just to remove those paths that are not available.
It is a slight optimisation to create it here, this way we only create
path sets for profiles that are supported.
Maybe doing the path set initialization sooner would also help with
paths_dir... or maybe not, I don't know. I didn't do the work to figure
out where exactly the path set should be created.
@@ -248,6 +241,8 @@ struct pa_alsa_mapping {
/* Temporarily used during probing */
snd_pcm_t *input_pcm;
snd_pcm_t *output_pcm;
+ pa_alsa_path_set *input_path_set;
+ pa_alsa_path_set *output_path_set;
These appear to be used after probing also, so they shouldn't be under
the "Temporarily used during probing" comment.
Hmm, and they don't seem to freed correctly either? Needs fixing.
pa_sink *sink;
pa_source *source;
@@ -289,8 +284,11 @@ struct pa_alsa_profile_set {
pa_hashmap *mappings;
pa_hashmap *profiles;
pa_hashmap *decibel_fixes;
+ pa_hashmap *input_paths;
+ pa_hashmap *output_paths;
Are separate hashmaps really needed for input and output? I didn't
notice that at least this patch would have any need to handle them
separately. Separate hashmaps make it possible to have an input path and
an output path with the same name, and I wouldn't like to allow that
unless there's a good reason to do so.
Hmm, good question. I think I just kept it that way to not regress
anything I didn't think of. If we merge them, I think I at least need to
check that someone isn't trying to use the same path for input and output.
--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
