[vlc-devel] Playlists, locking, and callbacks

David Hoyt dhoyt at llnl.gov
Wed Nov 19 00:26:46 CET 2008


Can you submit your patches? It looks like it would help solve some of the
problems I've run into myself. Thanks! 

-----Original Message-----
From: vlc-devel-bounces at videolan.org [mailto:vlc-devel-bounces at videolan.org]
On Behalf Of Polar Humenn
Sent: Friday, November 14, 2008 9:38 AM
To: vlc-devel at videolan.org
Subject: [vlc-devel] Playlists, locking, and callbacks

Hi
I'm new to VLC, and have been doing some developement for a module for a
small personal project I am working on.

I have found two things that I think are problems with locking of the
playlists.

The first problem is that there are calls made to "var_Set" that trigger
callbacks, on playlists. This is problematic if the playlist is locked
during these call backs.  And is quite readily seen in the DBUS
implementation when you --enable-debug, which enables ERRORCHECK on the
vlc_mutexes. Dbus initialization fails with a mutex error because a function
sets a variable while the playlist is locked and the callback tries to lock
the playlist through some other deeper functions.

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. 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.

Are there any comments on/caveats with this approach?

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.

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.

I've taken care of this problem by copying the callback list to the stack
and iterating through that, and replacing the calls to GetUnused with the
simple call to Lookup in var_AddCallback and var_DelCallback.
Works well, and I don't think it alters the semantics because you couldn't
manipulate the callback list while in a callback for that variable. Now you
can manipulate the callback lists from within a callback, but you have to
understand that any changes made to the callback list will not be noticed
until after the current execution of the callback list is complete. Which
leads me to another question.

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"?

Anyway, I can supply patches or, if allowed, can git them through.
Just looking to give back to the project.

Cheers,
-Dr. Polar
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
http:// mailman.videolan.org/listinfo/vlc-devel




More information about the vlc-devel mailing list