'Twas brillig, and Arun Raghavan at 21/06/11 04:16 did gyre and gimble: > On Fri, 2011-06-17 at 11:48 -0700, Arun Raghavan wrote: >> On Fri, 2011-06-17 at 12:15 +0100, Colin Guthrie wrote: >>> 'Twas brillig, and Colin Guthrie at 06/06/11 19:33 did gyre and gimble: >>>> Hi, >>>> >>>> I'm intending on changing module-device-restore to store some elements >>>> of sink proplists (for Arun and Pierre's passthrough work in order to >>>> allow the user to specify what formats his receiver supports). >>>> >>>> In order to do that, I need to be able to store an arbitrary sized chunk >>>> of data in the database. At present, all the files that use it are based >>>> on fixed sizes. >>>> >>>> So in order to make this neater, I've converted the internal database >>>> reading/writing to use tagstructs. This allows us to version the data >>>> stored in the same way our protocol is versioned (while this isn't >>>> really awesome from a code perspective, at least it's extensible which >>>> is nice). At present when we change something we have to invalidate all >>>> old data. This way we can save some of it at least if we want to. >>>> >>>> Of course during this transition, it's much simpler to just trash the >>>> old data and start again which is what I've done. If people are super >>>> opposed to this, I guess we could load the old format and transition it >>>> to the new format... let me know your opinions on this. Personally I >>>> don't really mind about losing the data this once (especially as we're >>>> moving to 1.0) >>>> >>>> >>>> The memory management is a little more complicated now (the entries are >>>> no longer stack based as they are more dynamic), but otherwise the >>>> principles are identical. There will be marginally greater overhead but >>>> this isn't something that is done super often so I think that's permissible. >>>> >>>> Please check the attached patch (particularly for memory leaks!) >>> >>> OK, I've updated this patch here: >>> http://colin.guthr.ie/git/pulseaudio/log/?h=master-sv-tagstuct >>> >>> This includes also another patch on for a protocol extension to >>> module-device-restore to allow us to save the formats it supports (this >>> will allow for manual configuration of receiver formats for passthrough). >> >> Awesome! >> >>> I've also added support for loading the legacy formats. I did this as a >>> separate commit as it should hopefully be easier to revert at a later date. >> >> Awesome^{2}. :) >> >>> If no objections on principle here I plan to merge this when I merge the >>> source-output volume branch which I hope to do soon so we can get some >>> test tarballs out and make a call for testing. >> >> I'll try to take a quick look over the code this weekend or early next >> week, but if you don't hear from me by Mon/Tue, just push. :) > > Did a quick review -- makes a lot of sense, and looks good! Awesome. I was thinking about what you said about the heap vs. memory tho'. What do you think about that? IMO it's not a massive difference either way as you'd still need to call a _done() function rahter than a _free() one... tho' for some simpler "entry" objects in the future, maybe this won't be needed (i.e. nothing to free), but the practice of putting in the calls would be helpful if that hypothetical simple structure were ever made more complex in the future. So IMO the heap vs. malloc thing is a bit pretty moot, but perhaps some performance gains are to be had with heap allocation? Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/]
