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 765292 - gl:cocoa:Fix incompatible type warning
gl:cocoa:Fix incompatible type warning
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Mac OS
: Normal normal
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-20 07:03 UTC by Hyunjun Ko
Modified: 2016-04-21 06:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gl:cocoa:Fix incompatible type warning (1.04 KB, patch)
2016-04-20 07:04 UTC, Hyunjun Ko
committed Details | Review
gl: caopengllayersink: fix a minor warning (821 bytes, patch)
2016-04-20 07:26 UTC, Hyunjun Ko
committed Details | Review
applemedia: fix a minor warning (1012 bytes, patch)
2016-04-20 07:26 UTC, Hyunjun Ko
none Details | Review
applemedia: vtenc: Fix a warning by weak-link symbol (948 bytes, patch)
2016-04-20 08:41 UTC, Hyunjun Ko
none Details | Review
applemedia: vtenc: Fix a warning by weak-link symbol (981 bytes, patch)
2016-04-20 12:11 UTC, Hyunjun Ko
committed Details | Review
applemedia: vtenc: fix build break on iOS (827 bytes, patch)
2016-04-21 01:10 UTC, Hyunjun Ko
committed Details | Review

Description Hyunjun Ko 2016-04-20 07:03:47 UTC
Fix incompatible type warning
Comment 1 Hyunjun Ko 2016-04-20 07:04:30 UTC
Created attachment 326378 [details] [review]
gl:cocoa:Fix incompatible type warning
Comment 2 Sebastian Dröge (slomo) 2016-04-20 07:11:23 UTC
commit 6a0b35bf3710240affe119ee362a0805e7a3fa85
Author: Hyunjun Ko <zzoon@igalia.com>
Date:   Wed Apr 20 16:00:36 2016 +0900

    gl/cocoa: Fix incompatible type warning
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765292
Comment 3 Hyunjun Ko 2016-04-20 07:26:15 UTC
Created attachment 326381 [details] [review]
gl: caopengllayersink: fix a minor warning

See commit message:)
Comment 4 Hyunjun Ko 2016-04-20 07:26:48 UTC
Created attachment 326382 [details] [review]
applemedia: fix a minor warning

see commit message.
Comment 5 Sebastian Dröge (slomo) 2016-04-20 07:48:10 UTC
Comment on attachment 326381 [details] [review]
gl: caopengllayersink: fix a minor warning

Attachment 326381 [details] pushed as 7bee220 - gl: caopengllayersink: fix a minor warning
Comment 6 Sebastian Dröge (slomo) 2016-04-20 07:51:34 UTC
Review of attachment 326382 [details] [review]:

::: sys/applemedia/vtenc.c
@@ +840,3 @@
   }
 #ifdef HAVE_VIDEOTOOLBOX_10_9_6
+  if (&VTCompressionSessionPrepareToEncodeFrames) {

I think this is wrong. We're checking here if that function exists. It will be NULL if it does not exist because of using -weak_framework to link to VideoToolbox. This is to make sure the plugin works against all versions of VT but works better with newer versions.
Comment 7 Hyunjun Ko 2016-04-20 08:20:23 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)

> I think this is wrong. We're checking here if that function exists. It will
> be NULL if it does not exist because of using -weak_framework to link to
> VideoToolbox. This is to make sure the plugin works against all versions of
> VT but works better with newer versions.

I don't think this patch is not doing check if that function exists.
It's just to silence this warning using address-of operator.
Comment 8 Sebastian Dröge (slomo) 2016-04-20 08:25:45 UTC
(In reply to Hyunjun Ko from comment #7)
> (In reply to Sebastian Dröge (slomo) from comment #6)
> 
> > I think this is wrong. We're checking here if that function exists. It will
> > be NULL if it does not exist because of using -weak_framework to link to
> > VideoToolbox. This is to make sure the plugin works against all versions of
> > VT but works better with newer versions.
> 
> I don't think this patch is not doing check if that function exists.
> It's just to silence this warning using address-of operator.

The problem is that &function is always going to give you something else than NULL AFAIU.

See also https://developer.apple.com/library/mac/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/WeakLinking.html , it uses the same check for a function without the & operator.
Comment 9 Hyunjun Ko 2016-04-20 08:41:26 UTC
Created attachment 326387 [details] [review]
applemedia: vtenc: Fix a warning by weak-link symbol

Thanks for the reference.
I hope this patch is enough good to fix this warning.
Honestly, I hate to re-compile with ERROR_CFLAGS= or something like that. :)
Comment 10 Sebastian Dröge (slomo) 2016-04-20 09:51:46 UTC
Review of attachment 326387 [details] [review]:

I don't like duplicating the headers, but what can we do :)

::: sys/applemedia/vtenc.c
@@ +59,3 @@
 
+#ifdef HAVE_VIDEOTOOLBOX_10_9_6
+extern int VTCompressionSessionPrepareToEncodeFrames ()

Let's at least give it the correct declaration :) It takes a session as parameter
Comment 11 Hyunjun Ko 2016-04-20 12:11:50 UTC
Created attachment 326403 [details] [review]
applemedia: vtenc: Fix a warning by weak-link symbol

Following slomo's comment.
I agree with you. I don't like extern thing either.
Thanks.:)
Comment 12 Sebastian Dröge (slomo) 2016-04-20 14:16:40 UTC
Attachment 326403 [details] pushed as 5023a7c - applemedia: vtenc: Fix a warning by weak-link symbol
Comment 13 Sebastian Dröge (slomo) 2016-04-20 15:00:51 UTC
This breaks the iOS build https://ci.gstreamer.net/job/cerbero-ios-universal-7.1/4221/console


15:57:11 vtenc.c:62:1: error: conflicting types for 'VTCompressionSessionPrepareToEncodeFrames'
15:57:11 VTCompressionSessionPrepareToEncodeFrames (VTCompressionSessionRef session)
15:57:11 ^
15:57:11 /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS9.3.sdk/System/Library/Frameworks/VideoToolbox.framework/Headers/VTCompressionSession.h:210:1: note: previous declaration is here
15:57:11 VTCompressionSessionPrepareToEncodeFrames( CM_NONNULL VTCompressionSessionRef session ) __OSX_AVAILABLE_STARTING(__MAC_10_9, __IPHONE_8_0);
Comment 14 Hyunjun Ko 2016-04-21 01:10:39 UTC
Created attachment 326462 [details] [review]
applemedia: vtenc: fix build break on iOS

Sorry for inconvenience.
I doubt that this patch could fix the error.
Please someone confirm this is ok.
Comment 15 Sebastian Dröge (slomo) 2016-04-21 06:28:17 UTC
commit 45b258127c0563b5608d256c094468de02452036
Author: Hyunjun Ko <zzoon@igalia.com>
Date:   Thu Apr 21 10:06:40 2016 +0900

    applemedia: vtenc: fix build break on iOS
    
    Fix 'conflicting types' error
    https://bugzilla.gnome.org/show_bug.cgi?id=765292

Attachment 326462 [details] pushed as 45b2581 - applemedia: vtenc: fix build break on iOS