GNOME Bugzilla – Bug 721245
osxvideosink: Fails to build on OS X Leopard 10.5.8
Last modified: 2014-01-09 17:51:10 UTC
Firstly, the check for deployment target is incorrect. Please apply: --- sys/osxvideo/osxvideosink.h.orig 2013-12-30 12:02:55.000000000 -0800 +++ sys/osxvideo/osxvideosink.h 2013-12-30 12:03:16.000000000 -0800 @@ -42,7 +42,7 @@ GST_DEBUG_CATEGORY_EXTERN (gst_debug_osx /* The hack doesn't work on leopard, the _CFMainPThread symbol * is doesn't exist in the CoreFoundation library */ -#if MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_5 +#if MAC_OS_X_VERSION_MIN_REQUIRED < 1060 #ifdef RUN_NS_APP_THREAD #undef RUN_NS_APP_THREAD #endif @@ -122,7 +122,7 @@ GType gst_osx_video_sink_get_type(void); @end -#if MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_5 +#if MAC_OS_X_VERSION_MIN_REQUIRED < 1060 @interface GstWindowDelegate : NSObject #else @interface GstWindowDelegate : NSObject <NSWindowDelegate> --- gcc-4.2 will set MAC_OS_X_VERSION_MIN_REQUIRED to the current system version by default (eg 1058 when running on 10.5.8), so the check for <= 1050 will fail on Leopard systems with updates applied. After that, the build still fails: osxvideosink.m: In function 'gst_osx_video_sink_call_from_main_thread': osxvideosink.m:96: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m:99: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m: In function 'gst_osx_videosink_check_main_run_loop': osxvideosink.m:168: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m: In function 'gst_osx_video_sink_run_cocoa_loop': osxvideosink.m:198: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m: In function 'gst_osx_video_sink_stop_cocoa_loop': osxvideosink.m:236: error: 'sink_klass' undeclared (first use in this function) osxvideosink.m:236: error: (Each undeclared identifier is reported only once osxvideosink.m:236: error: for each function it appears in.) osxvideosink.m: In function 'gst_osx_video_sink_osxwindow_create': osxvideosink.m:281: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m: In function 'gst_osx_video_sink_class_init': osxvideosink.m:582: error: 'GstOSXVideoSinkClass' has no member named 'ns_app_thread' osxvideosink.m: In function '-[GstOSXVideoSinkObject createInternalWindow]': osxvideosink.m:814: error: 'GstOSXVideoSink' has no member named 'app_started' osxvideosink.m:817: error: 'GstOSXVideoSink' has no member named 'app_started'
Also some other place where things were wrong. Can you test if this is all good now on your system? Afterwards I will backport to the 1.2 branch. commit 5b1c0a4cfd52ee25b6d6caa2851f7699a736b187 Author: Sebastian Dröge <sebastian@centricular.com> Date: Tue Dec 31 10:10:05 2013 +0100 osx: Make OSX version checks more consistent And especially also consider update versions, e.g. 10.5 with updates will be 1051 or similar and thus bigger than MAC_OS_X_VERSION_10_5 but still won't have the API we want to use. commit 3d163680f0c84db3ba9424a1bee5df78e548ff90 Author: Jeremy Huddleston <jeremyhu@freedesktop.org> Date: Tue Dec 31 10:07:22 2013 +0100 osxvideosink: Fix build on updated OS X Leopard https://bugzilla.gnome.org/show_bug.cgi?id=721245
Sure. I this is on a VM that I dust off periodically when users report bugs with Leopard, so I won't be living on the changes, but I'll at least make sure it builds with your update. Thanks.
Looking at those two commits, I don't see how that will fix the build failure I reported. Yes, you've fixed the version checks, but the ! RUN_NS_APP_THREAD build failure will still remain. I suspect you can reproduce on current systems if you just edit sys/osxvideo/osxvideosink.h to undefine RUN_NS_APP_THREAD all the time (rather than just for Leopard and earlier). Note that you might want to consider just dropping support for Leopard and earlier and running 'unifdef -DRUN_NS_APP_THREAD=1' on everything and removing all the MAC_OS_X_VERSION_MIN_REQUIRED messiness.
Indeed, sorry, I missed the other part you mentioned. Do you want to provide a patch to remove Leopard support? I don't think keeping Leopard support makes sense these days.
Sure, will do.
Created attachment 265109 [details] [review] 0001-configure-Disable-osxvideo-on-Leopard-and-earlier.patch
Created attachment 265110 [details] [review] 0002-osxvideo-Assume-SDK-and-deployment-target-are-at-lea.patch
Created attachment 265111 [details] [review] 0003-osxvideo-unifdef-DRUN_NS_APP_THREAD.patch
Ok, a three patch series is attached which disables osxvideo on Leopard and cleans up some now-dead code. Note that osxaudio still builds fine on Leopard, so I left it alone. It is also a bit more important since users are able to utilize ximagesink with X11 for video, but osxaudio is the best viable audio plugin.
Thanks :) Seeing that Mozilla also only support >=10.6 this should not cause any problems. commit 2bc631bcd0051cbb6cf2e519bf103cfdf3c33f08 Author: Jeremy Huddleston Sequoia <jeremyhu@apple.com> Date: Wed Jan 1 12:11:43 2014 -0800 osxvideo: unifdef -DRUN_NS_APP_THREAD commit 6fe2115d77b2c67bf5f12a52f456f92496e6556a Author: Jeremy Huddleston Sequoia <jeremyhu@apple.com> Date: Wed Jan 1 12:10:01 2014 -0800 osxvideo: Assume SDK and deployment target are at least Snow Leopard commit aeb3fa72b2ddded8add89480a80c2b5bb2cb6aa3 Author: Jeremy Huddleston Sequoia <jeremyhu@apple.com> Date: Wed Jan 1 12:23:50 2014 -0800 configure: Disable osxvideo on Leopard and earlier This also moves the "other platforms" check in OS X video to before the variable is read https://bugzilla.gnome.org/show_bug.cgi?id=721245
This fails here in various cases. My OSX reports this as host: x86_64-apple-darwinx i386-apple-darwinx Also this has to work on iOS, where host is currently: arm-apple-darwin10 i386-apple-darwin10 Maybe we should check MAC_OS_X_VERSION_MIN_REQUIRED during configure with an AC_TRY_COMPILE
Ok, please revert aeb3fa72b2ddded8add89480a80c2b5bb2cb6aa3, leaving the others. I'll get you an updated configure.ac using AC_TRY_COMPILE within the next 2 weeks.
This should do it now. Please test commit ae08e9a4cf10ddd9c6a4fa19bed2cbfabf878561 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Jan 8 10:31:18 2014 +0100 osxvideosink: Improve configure check for OSX >= 10.6 https://bugzilla.gnome.org/show_bug.cgi?id=721245
No, unfortunately, that's not the right check. I'll get you a version to try soon.
I think this is what you want (to not error out for iOS): diff --git a/configure.ac b/configure.ac index bfccf6e..8cf0cd0 100644 --- a/configure.ac +++ b/configure.ac @@ -491,7 +491,8 @@ AG_GST_CHECK_FEATURE(OSX_VIDEO, [OSX video], osxvideosink, [ dnl also require Snow Leopard or newer AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ - #if MAC_OS_X_VERSION_MIN_REQUIRED < 1060 + #include <AvailabilityMacros.h> + #if defined(MAC_OS_X_VERSION_MIN_REQUIRED) && MAC_OS_X_VERSION_MIN_REQUIRED < 1060 #error Too old OSX version #endif ]], [[return 0;]])],[HAVE_OSX_VIDEO="yes"],[HAVE_OSX_VIDEO="no"])
Yes, so it seems. In my case MAC_OS_X_VERSION_MIN_REQUIRED was in CFLAGS though :)
commit 03dff9de0b7ad2baadd25fc82a1797a35e979a9e Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jan 9 17:32:15 2014 +0100 configure: Include AvailabilityMacros.h for osxvideo check Otherwise MAC_OS_X_VERSION_MIN_REQUIRED might not be defined
(In reply to comment #16) > Yes, so it seems. In my case MAC_OS_X_VERSION_MIN_REQUIRED was in CFLAGS though > :) Unless there is some odd bug in system headers (please file a radar if so or point me to it, so I can do so), there should be no reason to ever define MAC_OS_X_VERSION_MIN_REQUIRED in CFLAGS. From Availability.h: """ The min OS version is specified as an option to the compiler: -mmacosx-version-min=10.x when building for Mac OS X, and -miphoneos-version-min=y.z when building for the iPhone. The upper bound for the OS version is rarely needed, but it can be set on the command line via: -D__MAC_OS_X_VERSION_MAX_ALLOWED=10x0 for Mac OS X and __IPHONE_OS_VERSION_MAX_ALLOWED = y0z00 for iOS. """ So please use -mmacosx-version-min=10.x instead and let the headers do the magic for you, or you may run into conflicts. eg: $ echo | clang -E -dM -DMAC_OS_X_VERSION_MIN_REQUIRED=1060 - | grep MIN_REQ #define MAC_OS_X_VERSION_MIN_REQUIRED 1060 #define __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ 1090