[vlc-devel] [PATCH] macosx: fix startup and shutdown procedure

[Felix Paul Kühne]

Hello Michael,

thanks for the patch.

First of all, please don't do this many changes in a single commit. Split it, so we can keep a reasonable level of documentation within git.

On 26.03.2013, at 14:22, Michael Feurstein <michael.feurstein at gmail.com> wrote:

> * removed subscription to LibVLCCore's messages - fixes silent console with not msg_Dbg coming through
This is wrong. You should file a ticket against VLC core. If you do this, the messages window will remain empty, which makes debugging a lot harder, especially with end users.
Rémi? can you comment, please?

> * delete playlist root when quitting in order to prevent the playlist from playing again during termination
This is a hack. Please file a ticket against VLC's playlist core.


> diff --git a/modules/gui/macosx/MainMenu.m b/modules/gui/macosx/MainMenu.m
> index 0cc28b4..9d05844 100644
> --- a/modules/gui/macosx/MainMenu.m
> +++ b/modules/gui/macosx/MainMenu.m
> @@ -78,7 +78,7 @@ static VLCMainMenu *_o_sharedInstance = nil;
>     } else {
>         _o_sharedInstance = [super init];
> 
> -        o_ptc_translation_dict = [NSDictionary dictionaryWithObjectsAndKeys:
> +        o_ptc_translation_dict = [[NSDictionary dictionaryWithObjectsAndKeys:
>                       _NS("Track Number"),  TRACKNUM_COLUMN,
>                       _NS("Title"),         TITLE_COLUMN,
>                       _NS("Author"),        ARTIST_COLUMN,
> @@ -89,10 +89,10 @@ static VLCMainMenu *_o_sharedInstance = nil;
>                       _NS("Date"),          DATE_COLUMN,
>                       _NS("Language"),      LANGUAGE_COLUMN,
>                       _NS("URI"),           URI_COLUMN,
> -                      nil];
> +                      nil] retain];
>         // this array also assigns tags (index) to type of menu item
> -        o_ptc_menuorder = [NSArray arrayWithObjects:TRACKNUM_COLUMN, TITLE_COLUMN, ARTIST_COLUMN, DURATION_COLUMN,
> -                       GENRE_COLUMN, ALBUM_COLUMN, DESCRIPTION_COLUMN, DATE_COLUMN, LANGUAGE_COLUMN, URI_COLUMN, nil];
> +        o_ptc_menuorder = [[NSArray arrayWithObjects:TRACKNUM_COLUMN, TITLE_COLUMN, ARTIST_COLUMN, DURATION_COLUMN,
> +                       GENRE_COLUMN, ALBUM_COLUMN, DESCRIPTION_COLUMN, DATE_COLUMN, LANGUAGE_COLUMN, URI_COLUMN, nil] retain];
>     }
Why not use the alloc] init] counterpart directly instead of the retain?

> 
>     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);
see above

> @@ -808,8 +810,10 @@ static VLCMain *_o_sharedMainInstance = nil;
>     [o_appLock unlock];
> 
>     if (isTerminating)
> +    {
wrong coding style.

> diff --git a/modules/gui/macosx/misc.m b/modules/gui/macosx/misc.m
> index 62779b1..9e3d7eb 100644
> --- a/modules/gui/macosx/misc.m
> +++ b/modules/gui/macosx/misc.m
> @@ -177,7 +177,8 @@ static NSMutableArray *blackoutWindows = NULL;
> 
>     for ( NSUInteger i = 0; i < count; i++ ) {
>         NSScreen *screen = [[NSScreen screens] objectAtIndex: i];
> -        if ([screen displayID] == displayID)
> +        CGDirectDisplayID thisDisplayID = (CGDirectDisplayID)[[[screen deviceDescription] objectForKey: @"NSScreenNumber"] intValue];
> +        if (thisDisplayID == displayID)
>             return screen;
>     }
I don't really understand this part. What's wrong with the [NSScreen displayID] implementation in misc.*? wouldn't it be cleaner to fix it instead?

Thanks,

Felix