[vlc-devel] VLC log system regression (was: Re: [PATCH] macosx: fix startup and shutdown procedure)

[Rémi Denis-Courmont]

Le mardi 26 mars 2013 19:56:55, David Fuhrmann a écrit :
> Am 26.03.2013 um 17:59 schrieb "Rémi Denis-Courmont" <remi at remlab.net>:
> >>     return _o_sharedInstance;
> >> 
> >> diff --git a/modules/gui/macosx/intf.m b/modules/gui/macosx/intf.m
> >> index c5fced6..398f8dc 100644
> >> --- a/modules/gui/macosx/intf.m
> >> +++ b/modules/gui/macosx/intf.m
> >> @@ -280,7 +280,7 @@ static void Run(intf_thread_t *p_intf)
> >> 
> >>     [[VLCMain sharedInstance] setIntf: p_intf];
> >>     
> >>     /* subscribe to LibVLCCore's messages */
> >> 
> >> -    vlc_LogSet(p_intf->p_libvlc, MsgCallback, NULL);
> >> +    //vlc_LogSet(p_intf->p_libvlc, MsgCallback, NULL);
> > 
> > This should be moved to wherever the message dialog or MacOS equivalent
> > gets created, and then only after the callback data is initialized.
> 
> Hi Remi,
> 
> I also noticed this problem recently. You introduced this regression on
> purpose?

Yes of course. I am an incarnation of the antichrist and I also have signed 
shady deals with QuickTime and gstreamer to derail VLC on MacOS. But please do 
not tell anyone about it.

> Why??

I am intrinsically evil and I enjoy it. To top it all, it is a small sub-sub-
sub-bullet point within my plan A to take over the World.

> It is just inconsistent, annoying to developers and hardens
> debugging.

Seriously, the log subscription interface that was in place previously 
followed the same conventions. Once you called it, you had to potentially 
handle logging callbacks up until you deregistered it. This is very much like 
var_AddCallback and var_DelCallback by the way.

In other words, I think that was a latent bug in the MacOS UI code. I guess 
the regression is not even caused by the change of interface, but by the move 
of the early log messages.

> In any case, IMO the current solution for the macosx interface is better,
> as it logs everything. So if the user experiences some problems, he can
> simply opens the log and copy the messages. No need for this strange "open
> log window and restart playback".

It might be so, but then you need to be extra-careful with memory consumption 
over time...

> Sometimes, the user even may not be able
> to reproduce the problem so easily, because it does not happen every time
> or has something to do which interfaces or other persistent modules.
> 
> Please also note that deinitializing the log when the window closes also is
> not implemented in qt interface as of now, as far as I tested.

That code seems to work fine for me:

MessagesDialog::~MessagesDialog()
{
    saveWidgetPosition( "Messages" );
    vlc_LogSet( p_intf->p_libvlc, NULL, NULL );
};


-- 
Rémi Denis-Courmont
http://www.remlab.net/