[pulseaudio-discuss] [PATCH v2 7/9] proplist: Add pa_proplist_update_info

[David Henningsson]

On 03/14/2013 03:58 PM, Tanu Kaskinen wrote:
> On Thu, 2013-03-14 at 14:58 +0100, David Henningsson wrote:
>> On 02/20/2013 07:24 PM, Tanu Kaskinen wrote:
>>> I was writing function pa_device_port_update_proplist(), and I wanted
>>> it to send change notifications only if the proplist actually changes.
>>> pa_proplist_update() doesn't provide any indication about whether the
>>> proplist changed or not, so some kind of a solution was needed.
>>>
>>> The simple solution would be to create a copy of the port proplist
>>> before calling pa_proplist_update() and check if the copy equals the
>>> port proplist after calling pa_proplist_update(). That felt overly
>>> wasteful, however: it would mean copying the whole property list and
>>> comparing every property in it whenever someone changes even just one
>>> property.
>>>
>>> So, I invented a more complex solution: a pa_proplist_update_info
>>> object that holds a description of per-property operations to be
>>> applied to a property list. pa_proplist_apply_update_info() iterates
>>> through the operations and applies them one by one, keeping track of
>>> whether the operations cause actual changes.
>>
>> I guess that's one way to solve it. I would probably have gone for the
>> slightly simpler solution of just keeping a flag inside the proplist.
>> The proplist object itself will set the flag whenever a "real" change
>> occurs, and it can be manually reset by just calling, say
>> pa_proplist_reset_change_flag() or so.
>
> That sounds pretty sensible. Do you think I should redo the patches? I'd
> prefer not to do that, but that's just because I'm lazy.

You mean, it's just because you want to get more time for fixing other 
PulseAudio issues ;-)

Anyway, I'm pragmatic. As long a patch improves the current condition 
and does not cause regressions, APIs we have to commit to in the future, 
or other implications, I don't mind it going in. It can be rewritten, 
simplified, or whatever, by someone else later if they feel like it.

>>> ---
>>>    src/map-file         |    4 +
>>
>> I don't think we need to add this to the external API unless somebody
>> complains about missing that feature.
>
> Fair enough, I don't mind hiding this from clients.

Ok.



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic