GNOME Bugzilla – Bug 679112
controller: fix GStreamer build on Visual C++ by adding isnan() to math-compat.h
Last modified: 2012-07-03 01:50:19 UTC
Created attachment 217591 [details] [review] Add fallback implementation for isnan() for Visual C++ Hi, I was trying to build GStreamer on Viaual C++ 2008/2010 and found that the code could not be compiled on them for several parts: -First up is that an isnan() implementation is needed. This is done by simply using _isnan() which does the same thing on Visual C++ in gst/math-compat.h
Created attachment 217592 [details] [review] Include gst/math-compat.h instead of math.h Next up is the update to the two source files in GSTController that use isnan(), where they would include gst/math-compat.h instead of math.h, where we added an implementation for isnan() just now.
Created attachment 217593 [details] [review] gstfdsink.c: Do the #ifdef G_OS_WIN32 a bit later Last of this is to move the section "#ifdef G_OS_WIN32" a bit down, to after the inclusion of gstfdsink.h, to ensure that the GLib headers and the definition of G_OS_WIN32 is pulled in, especially for C89 compilers like Visual C++. Thanks for the time, with blessings!
Comment on attachment 217591 [details] [review] Add fallback implementation for isnan() for Visual C++ >+#if defined (__GST_MATH_COMPAT_NEED_ISNAN) && !defined (isnan) && defined (_MSC_VER) >+static inline gboolean >+__gst_math_compat_isnan (double x) >+{ >+ return _isnan (x); >+} >+#endif Why the && defined (_MSC_VER) here? Wouldn't that mean that the #define below might be referencing an inline function that we never define for other compilers where isnan is needed but not available?
(In reply to comment #3) > Why the && defined (_MSC_VER) here? Wouldn't that mean that the #define below > might be referencing an inline function that we never define for other > compilers where isnan is needed but not available? Hi Tim, This was done so as _isnan() is Visual C++ specific-it's not on GCC, and I can't say about the other compilers GStreamer supports, as I don't have them with me.
Created attachment 217801 [details] [review] Add implementation for isnan() when it's not available Hi, Probably for consistency's sake, I should come up with an quick implementation of isnan() when it's needed. With blessings, Thank you!
> Created an attachment (id=217801) [details] [review] > Add implementation for isnan() when it's not available > > Probably for consistency's sake, I should come up with an quick implementation > of isnan() when it's needed. Great, thanks! Just to be sure: you dropped the _isnan() use for MSCV on purpose now, right? :)
Comment on attachment 217593 [details] [review] gstfdsink.c: Do the #ifdef G_OS_WIN32 a bit later commit cb5e8f2bb48549ac25b264db92d949178b735029 Author: Chun-wei Fan <fanchunwei@src.gnome.org> Date: Fri Jun 29 10:56:34 2012 +0800 fdsink.c: fix G_OS_WIN32 #ifdef Postpone the #ifdef to a point after glib.h (via gstfdsink.h) is included so that the needed defines and header includes can be done correctly, especially on Visual C++ builds. https://bugzilla.gnome.org/show_bug.cgi?id=679112
Review of attachment 217801 [details] [review]: Yeah I did drop the _MSC_VER check, but I forgot to update the commit log message. Sorry :)
Comment on attachment 217801 [details] [review] Add implementation for isnan() when it's not available Took the liberty of adding the missing semicolon: commit 02c07aa854f5ca923105a1a16ba5807e66c50069 Author: Chun-wei Fan <fanchunwei@src.gnome.org> Date: Fri Jun 29 16:52:31 2012 +0800 math-compat.h: add implementation for isnan() for Visual C++ Visual C++ does not have isnan(), so add fallback to math-compat.h (could use _isnan() in this case, but this makes it work for all cases where isnan is missing). https://bugzilla.gnome.org/show_bug.cgi?id=679112
Thanks for the patches! commit 94cbf343003b85d58a4d8091e0ab7208f7783580 Author: Chun-wei Fan <fanchunwei@src.gnome.org> Date: Thu Jun 28 16:42:08 2012 +0800 controlbindings: include gst/math-compat.h for isnan() Due to the usage of isnan(), where an implementation is added into gst/math-compat.h. Fixes build on Visual C++. https://bugzilla.gnome.org/show_bug.cgi?id=679112
(In reply to comment #9) > (From update of attachment 217801 [details] [review]) > Took the liberty of adding the missing semicolon: > oops, sorry about that. :) Thank you for your time, with blessings!