After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 721245 - osxvideosink: Fails to build on OS X Leopard 10.5.8
osxvideosink: Fails to build on OS X Leopard 10.5.8
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Mac OS
: Normal blocker
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-30 20:10 UTC by Jeremy Huddleston
Modified: 2014-01-09 17:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-configure-Disable-osxvideo-on-Leopard-and-earlier.patch (1.33 KB, patch)
2014-01-01 20:40 UTC, Jeremy Huddleston
committed Details | Review
0002-osxvideo-Assume-SDK-and-deployment-target-are-at-lea.patch (3.10 KB, patch)
2014-01-01 20:41 UTC, Jeremy Huddleston
committed Details | Review
0003-osxvideo-unifdef-DRUN_NS_APP_THREAD.patch (4.61 KB, patch)
2014-01-01 20:41 UTC, Jeremy Huddleston
committed Details | Review

Description Jeremy Huddleston 2013-12-30 20:10:26 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'
Comment 1 Sebastian Dröge (slomo) 2013-12-31 09:12:20 UTC
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
Comment 2 Jeremy Huddleston 2014-01-01 01:17:07 UTC
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.
Comment 3 Jeremy Huddleston 2014-01-01 01:25:29 UTC
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.
Comment 4 Sebastian Dröge (slomo) 2014-01-01 09:38:25 UTC
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.
Comment 5 Jeremy Huddleston 2014-01-01 19:57:27 UTC
Sure, will do.
Comment 6 Jeremy Huddleston 2014-01-01 20:40:53 UTC
Created attachment 265109 [details] [review]
0001-configure-Disable-osxvideo-on-Leopard-and-earlier.patch
Comment 7 Jeremy Huddleston 2014-01-01 20:41:14 UTC
Created attachment 265110 [details] [review]
0002-osxvideo-Assume-SDK-and-deployment-target-are-at-lea.patch
Comment 8 Jeremy Huddleston 2014-01-01 20:41:29 UTC
Created attachment 265111 [details] [review]
0003-osxvideo-unifdef-DRUN_NS_APP_THREAD.patch
Comment 9 Jeremy Huddleston 2014-01-01 20:44:18 UTC
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.
Comment 10 Sebastian Dröge (slomo) 2014-01-02 09:02:49 UTC
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
Comment 11 Sebastian Dröge (slomo) 2014-01-07 09:08:32 UTC
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
Comment 12 Jeremy Huddleston 2014-01-07 17:26:03 UTC
Ok, please revert aeb3fa72b2ddded8add89480a80c2b5bb2cb6aa3, leaving the others.  I'll get you an updated configure.ac using AC_TRY_COMPILE within the next 2 weeks.
Comment 13 Sebastian Dröge (slomo) 2014-01-08 09:31:55 UTC
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
Comment 14 Jeremy Huddleston 2014-01-08 19:33:53 UTC
No, unfortunately, that's not the right check.  I'll get you a version to try soon.
Comment 15 Jeremy Huddleston 2014-01-08 23:44:40 UTC
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"])
Comment 16 Sebastian Dröge (slomo) 2014-01-09 16:28:20 UTC
Yes, so it seems. In my case MAC_OS_X_VERSION_MIN_REQUIRED was in CFLAGS though :)
Comment 17 Sebastian Dröge (slomo) 2014-01-09 16:33:00 UTC
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
Comment 18 Jeremy Huddleston 2014-01-09 17:51:10 UTC
(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