On 12/01/2011 09:20 PM, Tanu Kaskinen wrote: > Hi, > > On Fri, 2011-11-25 at 15:17 +0100, David Henningsson wrote: >> To be able to add ports to all profiles, we need to probe all >> profiles at startup. To speed this up, we now have a cache of >> probes paths which is owned by the profile set. Since paths >> are now owned by the profile set, the path set must now have >> a hashmap of paths instead of a linked list. > > I finally finished the review, or at least I have had enough of it ;) You're not the only one...glad to see it through at last. > I > pushed the patch, even though there are still some issues, because I > want to make some changes on top of this patch (the changes in question > are adding proplists to ports and making it possible to add port > properties in the path configuration files). Having property lists on ports is something desirable for all of us. Would you mind applying the patch I just posted? We could use that as a base - I could add it to the client API I'm working on, and you could work on parsing configuration files. > The remaining issues: > > * If mapping_probe_paths() fails to open the mixer, it's not handled > very well. If that happens, I think all paths should be removed from the > mapping's path sets. > * pa_path_set.probed isn't really used for anything (pretty cosmetic > issue). > * If profile probing is configured to be skipped, mapping_paths_probe() > won't be called, and path sets won't be created. > > Would you possibly have the time and energy to address these problems? I think they are all good catches, and I just posted a patch to fix this up (although I was lazy enough to use a goto for the last fix, maybe I should refactor the entire function a bit?). > Thanks for your work so far :) Thanks for the review! > Regarding the paths_dir brokenness, I don't mind about it right now. > I'll probably want to implement it in a different way anyway (maybe so > that the alsa configuration files are automatically loaded from ~/.pulse > if the files exist there). Ok. Maybe we should revert paths_dir altogether if nobody's else depending on it? -- David Henningsson, Canonical Ltd. http://launchpad.net/~diwic
