[vlc-devel] Playlists, locking, and callbacks
rem at videolan.org
Sun Nov 16 16:28:02 CET 2008
On Friday 14 November 2008 19:37:52 Polar Humenn, you wrote:
> It would seem that the playlist should be forcedly unlocked before
> going into a "var_Set" call. I have done this all around the playlist
> implementation with great success.
Might very well be. Variable callbacks are a big mess at the moment.
We should check if it makes sense for this variable to be set with the
playlist (un)locked. It might also be worse checking if a dedicated API would
not make more sense than var_Set().
> There is also one other function I had to create, which was
> playlist_GetCurrentInputLocked(). This is because playlist_GetCurrentInput
> was being called in in places where the playlist was already locked, such as
> in variable callbacks, causing errors.
> There might be a more elegant way to do this, perhaps
> with a boolean value as with some of the other playlist functions,
> stipulating whether the playlist is already locked.
Not elegant, but looks valid.
> Are there any comments on/caveats with this approach?
A boolean flag would be an alternative. Pierre d'H. would be a more
knowledgeable person than I.
> It looks like people have been burned by this problem before as some
> "var_Set" calls have been wrapped with a PL_UNLOCK and PL_LOCK, but
> obviously not all of them.
Similarly, it would have been worth checking that it makes sense to unlock -
does this not introduce some race condition/consistency problems.
> Another point, which is more systemic, is that in the variables.c
> implementation. The callback list is iterated directly. This approach
> forces the need (and correctly so) to lock the variable to prevent
> changes to the callback list through the use of GetUnused in
> var_AddCallback and var_DelCallback functions. This prevents such
> simple dynamic behavior has a a callback removing itself.
The variable callback stuff looks quite messy to me, not using locks and
condition variables properly (e.g. waiting for pending callbacks to complete
before removing callbacks).
> Why are the callbacks iterated through from last to first? Is that a
> design decision? Or just a desire to not use two variables, and a
> comparison of two numbers, in contrast to just a comparison against
> zero? Is the "public" documented behavior supposed to be "undefined"?
Probably the only ones who know is Sam, and he hasn't been very active lately.
> Anyway, I can supply patches or, if allowed, can git them through.
> Just looking to give back to the project.
It won't hurt to post them here.
More information about the vlc-devel