[vlc-devel] RE : [PATCH] -- skins2 (more corrections)

[brezhoneg1]

> 
> Le dimanche 15 mars 2009 02:49:23 brezhoneg1, vous avez écrit :
> > patch4: qt4 missing vlc_object_release in menus (the easy way)
> > --------------------------------------------------------------
> > For want of a better solution, the patch just adds a missing
> > vlc_object_release to pair the vlc_object_hold in a menu.
> 
> I fail to see how this solves anything. It does not make any sense to
hold
> an
> object and then release with absolutely no synchronization in-between.
> Normally, you call _hold either from inside a lock, to keep the object
> past
> unlocking, or before you pass the object to another thread, or both.
> 
> To re-iterate, if vlc_object_release() works there, then the pair of
> vlc_object_hold()/vlc_object_release() is a no-op, and should both be
> removed. If the reference is needed, then the call to
vlc_object_release()
> is
> too early and might cause a crash.


After several hesitations, my understanding is that all vlc_object_hold
are useless (and wrongly used anyway) in menus.cpp. But, I would prefer
somebody in charge of this matter to confirm this statement.

Yet, from a skins point of view, the patch still solves one thing,
having the right count of release when it terminates. Otherwise, there
is a risk of hanging since refcount for input will never reach zero.
This patch is sent here to help reviewers for the whole skins thing.
(not necessarily to be applied in git master)



 
> > Patch5: better termination for qt4
> > ----------------------------------
> > This patch makes sure quit()is executed in the right thread (qt4
main
> > thread). That should remove the error: "QObject::killTimers: timers
> > cannot be stopped from another thread"
> 
> What is calling quit() from the wrong thread? Maybe that's what should
be
> fixed instead.
> 
> --

The patch fixes things in both places (the calling thread and the qt4
thread).

QApplication::quit() is actually in qt4.cpp. Yet, it is not run by the
qt4 thread, but by the main thread (the ancestor of all threads),
because it is inside the close function of the interface.

So I changed it into QVLCApp::triggerQuit(), a static function that just
signals the QT4 application to quit. Separating things into a signal and
a connect ensure quit() will be run in the qt4 thread.

Erwan10