[vlc-devel] [PATCH] npapi-vlc: Enable building npapi plugin on Mac OS X

[Felix Paul Kühne]

Hello,

On Oct 6, 2012, at 3:18 PM, James Bates wrote:
> extras/macosx/Pre-Compile.sh |  284 ++++++++++++++++++++++++++++++++++++++++++
This seems to be more or less a copy of the same script from the mainline vlc repo. Could you split this commit and add your changes in a separate one so we can see what you actually changed and what's your copyright in contrast to the existing code?
Additionally, please clean the script a bit since I'm pretty sure that you don't need the VLC.app parts for the web plugins.

> diff --git a/npapi/Makefile.am b/npapi/Makefile.am
> index ac7f2ba..81c9291 100644
> --- a/npapi/Makefile.am
> +++ b/npapi/Makefile.am
> @@ -104,25 +104,25 @@ else
> lib_LTLIBRARIES = npvlc.la
> 
> AM_CPPFLAGS += -I. -I$(top_builddir) -c \
> -	-F/System/Library/Frameworks/CoreFoundation.framework \
> -	-I/Developer/Headers/FlatCarbon -fno-common -fpascal-strings \
> 	-Wmost -Wno-four-char-constants -Wno-unknown-pragmas \
> 	-DXP_UNIX -DXP_MACOSX=1 \
> 	-DNO_X11=1 -DUSE_SYSTEM_CONSOLE=1 -pipe -fmessage-length=0
> 
> -SOURCES_support = support/npmac.cpp
> +SOURCES_support = support/npmac.cpp \
> +                  vlcplugin_mac.cpp \
> +                  vlcplugin_mac.h
> libvlcplugin_la_LDFLAGS += \
> 	-bundle -Wl,-read_only_relocs -Wl,suppress \
> 	-Wl,-headerpad_max_install_names \
> 	-Wl,-framework,Carbon -Wl,-framework,System \
> -	-shrext $(LIBEXT)
> +	-shrext .dylib
> 
> noinst_DATA = npvlc.rsrc VLC\ Plugin.plugin
> MOSTLYCLEANFILES += npvlc.rsrc
> CLEANFILES += VLC\ Plugin.plugin
> 
> npvlc.rsrc: vlc.r
> -	/Developer/Tools/Rez -useDF /Developer/Headers/FlatCarbon/Types.r $< -o $@
> +	xcrun Rez -useDF /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk/Developer/Headers/FlatCarbon/Types.r $< -o $@
This looks fishy to me, since Xcode.app is neither guaranteed to be called Xcode.app nor to be located in that folder.

> diff --git a/npapi/support/npmac.cpp b/npapi/support/npmac.cpp
> index b8e2d0e..7dc0893 100644
> --- a/npapi/support/npmac.cpp
> +++ b/npapi/support/npmac.cpp
> @@ -27,14 +27,14 @@
> #include "config.h"
> 
> #include <string.h>
> -
> +/*
> #include <Processes.h>
> #include <Gestalt.h>
> #include <CodeFragments.h>
> #include <Timer.h>
> #include <Resources.h>
> #include <ToolUtils.h>
> -
> +*/
Please don't comment Carbon includes but remove them entirely if they aren't needed anymore.

> +
> +    // This check is too simplisitic: some browsers (e.g. Safari) update the NPAPI less frequently, and so
> +    // provide a table containing all netspace functions we are interested, but not necessarily all defined
> +    // in the newest version of the npapi against which this plugin is compiled.
> +    /*if (nsTable->size < sizeof(NPNetscapeFuncs)) {
> 
> -    if (nsTable->size < sizeof(NPNetscapeFuncs))
> +    	PLUGINDEBUGSTR("\pNP_Initialize error: NPERR_INVALID_FUNCTABLE_ERROR: table too small");
>         return NPERR_INVALID_FUNCTABLE_ERROR;
> +    }*/
So what's the consequence here? The check seems to be mood, so it's better not to use it. So, do we need a better check (aka is this a "FIXME!"?) or can we safely remove it?


> diff --git a/npapi/vlcplugin_base.cpp b/npapi/vlcplugin_base.cpp
> index 3f6a00c..7bb57f9 100644
> --- a/npapi/vlcplugin_base.cpp
> +++ b/npapi/vlcplugin_base.cpp
> @@ -386,7 +386,7 @@ void VlcPluginBase::event_callback(const libvlc_event_t* event,
>     NPN_PluginThreadAsyncCall(getBrowser(), eventAsync, this);
> #else
> #   warning NPN_PluginThreadAsyncCall not implemented yet.
> -    printf("No NPN_PluginThreadAsyncCall(), doing nothing.\n");
> +//    printf("No NPN_PluginThreadAsyncCall(), doing nothing.\n");
> #endif
> }
> 
> @@ -398,12 +398,14 @@ NPError VlcPluginBase::init(int argc, char* const argn[], char* const argv[])
> 
> #ifndef NDEBUG
>     ppsz_argv[ppsz_argc++] = "--no-plugins-cache";
> +#else
> +#   error "Debugging disabled"
> #endif
> 
>     /* locate VLC module path */
> #ifdef XP_MACOSX
> -    ppsz_argv[ppsz_argc++] = "--plugin-path=/Library/Internet\\ Plug-Ins/VLC\\ Plugin.plugin/Contents/MacOS/plugins";
> -    ppsz_argv[ppsz_argc++] = "--vout=minimal_macosx";
> +//    ppsz_argv[ppsz_argc++] = "--plugin-path=/Library/Internet\\ Plug-Ins/VLC\\ Plugin.plugin/Contents/MacOS/plugins";
> +    ppsz_argv[ppsz_argc++] = "--vout=vout_macosx";

Since this libvlc option is no longer needed, why not remove it?

> diff --git a/npapi/vlcplugin_mac.h b/npapi/vlcplugin_mac.h
> index df8e573..c1cd4ea 100644
> --- a/npapi/vlcplugin_mac.h
> +++ b/npapi/vlcplugin_mac.h
> @@ -29,7 +29,7 @@
> 
> #include "vlcplugin_base.h"
> 
> -#include <Quickdraw.h>
> +//#include <Quickdraw.h>
See above. QD is dead since at least 5 years, so nobody is ever going to care about it again.

> diff --git a/npapi/vlcshell.cpp b/npapi/vlcshell.cpp
> index 76c7013..7dc5ff4 100644
> --- a/npapi/vlcshell.cpp
> +++ b/npapi/vlcshell.cpp
> @@ -138,7 +138,7 @@ int16_t NPP_HandleEvent( NPP instance, void * event )
>         return false;
>     }
> 
> -#ifndef __x86_64__
> +#if 0
I don't really get this part..


Cheers,

Felix