[vlc-devel] [PATCH] npapi-vlc: Enable building npapi

[James Bates]

On Oct 8, 2012, at 11:55 AM, vlc-devel-request at videolan.org wrote:

> Message: 6
> Date: Sun, 7 Oct 2012 16:20:17 +0200
> From: Felix Paul K?hne <fkuehne.videolan at gmail.com>
> To: Mailing list for VLC media player developers
> 	<vlc-devel at videolan.org>
> Subject: Re: [vlc-devel] [PATCH] npapi-vlc: Enable building npapi
> 	plugin on	Mac OS X
> Message-ID: <C493F4E7-D67F-4725-825D-C35E14CD06FC at gmail.com>
> Content-Type: text/plain; charset=us-ascii
> 
> 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.
I have renamed it build-package.sh, and updated it with the recent updates to that file in the vlc/ project. The diff compared to the vlc-version (i.e. my changes) is:

--- ../vlc/extras/package/macosx/build-package.sh	2012-10-04 19:51:13.000000000 +0200
+++ extras/macosx/build-package.sh	2012-10-08 15:07:32.000000000 +0200
@@ -2,7 +2,10 @@
 #
 # build-package.sh
 #
-# Script that installs libvlc to VLC.app
+# Script that installs libvlc and modules to the VLC Plugin.plugin
+#
+# This script is adapted from the build-package.sh script found in the vlc project
+# at extras/package/macosx/build-package.sh
 
 # We are building VLC.app or the moz plugin
 #
@@ -139,7 +142,7 @@
     local type=$4
 
     if test "$use_archs" = "no"; then
-        vlc_install_object "$VLC_BUILD_DIR/$src_dir/$src" "$dest_dir" "$type" $5
+        vlc_install_object "$src_dir/$src" "$dest_dir" "$type" $5
     else
         if test $type = "data"; then
             vlc_install_object "$main_build_dir/$src_dir/$src" "$dest_dir" "$type" $5
@@ -237,7 +240,7 @@
     chmod +x ${target}/VLC
 elif [ "$FULL_PRODUCT_NAME" = "VLC-Plugin.plugin" ] ; then
     # install Safari webplugin
-    vlc_install "projects/mozilla/${prefix}" "npvlc.${suffix}" "${target}" "lib" "@loader_path/lib"
+    vlc_install "${src_dir}/${prefix}" "npvlc.${suffix}" "${target}" "lib" "@loader_path/lib"
     mv ${target}/npvlc.${suffix} "${target}/VLC Plugin"
     chmod +x "${target}/VLC Plugin"
 else
@@ -248,17 +251,14 @@
 # Build the plugins folder
 echo "Building plugins folder..."
 # Figure out what plugins are available to install
-for module in `find ${main_build_dir}/modules -path "*dylib.dSYM*" -prune -o -name "lib*_plugin.dylib" -print | sed -e s:${main_build_dir}/::` ; do
-    # Check to see that the reported module actually exists
-    if test -n ${module}; then
-        vlc_install `dirname ${module}` `basename ${module}` ${target_plugins} "module"
-    fi
+for module in `find ${libvlc_dir}/lib/vlc/plugins -name 'lib*_plugin.dylib' -print` ; do
+    vlc_install `dirname ${module}` `basename ${module}` ${target_plugins} "module"
 done
 
 ##########################
 # Build the lib folder
-vlc_install "lib/${prefix}" "libvlc.5.dylib" "${target_lib}" "library"
-vlc_install "src/${prefix}" "libvlccore.5.dylib" "${target_lib}" "library"
+vlc_install "${libvlc_dir}/lib" "libvlc.5.dylib" "${target_lib}" "library"
+vlc_install "${libvlc_dir}/lib" "libvlccore.5.dylib" "${target_lib}" "library"
 pushd `pwd` > /dev/null
 cd ${target_lib}
 ln -sf libvlc.5.dylib libvlc.dylib
@@ -269,10 +269,10 @@
 # Build the share folder
 if [ $PRODUCT != "VLC.app" ]; then
     echo "Building share folder..."
-    pbxcp="/Developer/Library/PrivateFrameworks/DevToolsCore.framework/Resources/pbxcp -exclude .DS_Store -resolve-src-symlinks -v -V"
+    pbxcp="cp -av" 
     mkdir -p ${target_share}
     if test "$use_archs" = "no"; then
-        $pbxcp ${VLC_BUILD_DIR}/share/lua ${target_share}
+        $pbxcp ${libvlc_dir}/share/vlc/lua ${target_share}
     else
         $pbxcp ${main_build_dir}/share/lua ${target_share}
     fi

>> 
>> 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.
Absolutely correct. In fact, in the MacOSX10.8 SDK, which I am principally using, there is no Types.r at all anywhere. I removed the resource file npvlc.rsrc completely from the build, with apparently no averse consequences (The plugin loads and plays videos just as well as before).


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

>> +
>> +    // 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?
I guess what we need to check is that the browser-supplied table contains at least the last function from the npapi that we need ("setexception" as far as I can tell…). I've modified (and reactivated) the code accordingly.

>> +//    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?
Done

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

>> 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..
It's a dirty way of commenting out code. The code used to be included only for 32-bit builds. Since I introduced drawing- and event-model negotiation, the event model is now always Cocoa, regardless of whether it's a 32- or 64-bit plugin being built, and as a result this (Carbon) event code should never be used; it should be ported to the Cocoa event model. I've commented it out properly, with a note that I should still do that port.


I have resubmitted the patch in another message to the mailing list. Thanks for the review!

James