diff mbox

[FFmpeg-devel] Made minor changes to get the decklink avdevice code to build using Visual C++

Message ID e3b72fe4-3db3-0f79-7bcb-ea887e5f14f3@aracnet.com
State Superseded
Headers show

Commit Message

Aaron Levinson May 5, 2017, 7:34 a.m. UTC
On 4/16/2017 4:11 PM, Aaron Levinson wrote:
> On 4/15/2017 6:13 AM, Aaron Levinson wrote:
>> On 4/15/2017 4:19 AM, Marton Balint wrote:
>>>
>>> On Thu, 13 Apr 2017, Aaron Levinson wrote:
>>>
>>>> On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
>>> [...]

Ping, and in addition, I've provided a new patch below since the original won't apply anymore.  I've also adjusted the patch text a little bit.  The original patch went through a few different reviews.  These changes are necessary to get the decklink code to build using MSVC on Windows.

Thanks,
Aaron Levinson

------------------------------------------------------------------------------------

From 90d20d87401584138f6bdea5567b92e7dfe07f1f Mon Sep 17 00:00:00 2001
From: Aaron Levinson <alevinsn@aracnet.com>
Date: Fri, 5 May 2017 00:20:56 -0700
Subject: [PATCH] decklink:  Fixed MSVC build issues

Purpose: Made minor changes to get the decklink avdevice code to build
using Visual C++.

Notes: Made changes to configure per Hendrik Leppkes's review of first
and second versions of patch.

Comments:

-- configure: Added if enabled decklink section and setting
   decklink_indev_extralibs and decklink_outdev_extralibs here for
   both mingw and Windows.  Also eliminated the setting of these
   variables in the mingw section earlier in the file.

-- libavdevice/decklink_common.cpp: Switched the order of the include
   of libavformat/internal.h to workaround build issues with Visual
   C++.  See comment in file for more details.

-- libavdevice/decklink_dec.cpp:
a) Rearranged the include of libavformat/internal.h (for reasons as
   described above).
b) Made slight alteration to an argument for call to av_rescale_q() to
   workaround a compiler error with Visual C++.  This appears to only
   be an issue when building C++ files with Visual C++.  See comment
   in code for more details.

-- libavdevice/decklink_enc.cpp: Rearranged the include of
   libavformat/internal.h (for reasons as described above).

Signed-off-by: Aaron Levinson <alevinsn@aracnet.com>
---
 configure                       | 11 +++++++++--
 libavdevice/decklink_common.cpp |  7 ++++++-
 libavdevice/decklink_dec.cpp    | 17 +++++++++++++++--
 libavdevice/decklink_enc.cpp    |  7 ++++++-
 4 files changed, 36 insertions(+), 6 deletions(-)

Comments

Marton Balint May 5, 2017, 11:57 p.m. UTC | #1
On Fri, 5 May 2017, Aaron Levinson wrote:

> On 4/16/2017 4:11 PM, Aaron Levinson wrote:
>> On 4/15/2017 6:13 AM, Aaron Levinson wrote:
>>> On 4/15/2017 4:19 AM, Marton Balint wrote:
>>>>
>>>> On Thu, 13 Apr 2017, Aaron Levinson wrote:
>>>>
>>>>> On 4/13/2017 1:23 PM, Hendrik Leppkes wrote:
>>>> [...]
>
> Ping, and in addition, I've provided a new patch below since the 
> original won't apply anymore.  I've also adjusted the patch text a 
> little bit.  The original patch went through a few different reviews. 
> These changes are necessary to get the decklink code to build using MSVC 
> on Windows.
>
> Thanks,
> Aaron Levinson
>
> ------------------------------------------------------------------------------------
>
> From 90d20d87401584138f6bdea5567b92e7dfe07f1f Mon Sep 17 00:00:00 2001
> From: Aaron Levinson <alevinsn@aracnet.com>
> Date: Fri, 5 May 2017 00:20:56 -0700
> Subject: [PATCH] decklink:  Fixed MSVC build issues
>
> Purpose: Made minor changes to get the decklink avdevice code to build
> using Visual C++.
>
> Notes: Made changes to configure per Hendrik Leppkes's review of first
> and second versions of patch.
>
> Comments:
>
> -- configure: Added if enabled decklink section and setting
>   decklink_indev_extralibs and decklink_outdev_extralibs here for
>   both mingw and Windows.  Also eliminated the setting of these
>   variables in the mingw section earlier in the file.
>
> -- libavdevice/decklink_common.cpp: Switched the order of the include
>   of libavformat/internal.h to workaround build issues with Visual
>   C++.  See comment in file for more details.
>
> -- libavdevice/decklink_dec.cpp:
> a) Rearranged the include of libavformat/internal.h (for reasons as
>   described above).
> b) Made slight alteration to an argument for call to av_rescale_q() to
>   workaround a compiler error with Visual C++.  This appears to only
>   be an issue when building C++ files with Visual C++.  See comment
>   in code for more details.
>
> -- libavdevice/decklink_enc.cpp: Rearranged the include of
>   libavformat/internal.h (for reasons as described above).
>
> Signed-off-by: Aaron Levinson <alevinsn@aracnet.com>
> ---
> configure                       | 11 +++++++++--
> libavdevice/decklink_common.cpp |  7 ++++++-
> libavdevice/decklink_dec.cpp    | 17 +++++++++++++++--
> libavdevice/decklink_enc.cpp    |  7 ++++++-
> 4 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/configure b/configure
> index 883ec390d6..bbb9fab738 100755
> --- a/configure
> +++ b/configure
> @@ -4847,8 +4847,6 @@ case $target_os in
>         else
>             target_os=mingw32
>         fi
> -        decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 -loleaut32"
> -        decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
>         LIBTARGET=i386
>         if enabled x86_64; then
>             LIBTARGET="i386:x86-64"
> @@ -5947,6 +5945,15 @@ if ! disabled sdl2; then
> fi
> enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs $sdl2_extralibs
> 
> +if enabled decklink; then
> +    case $target_os in
> +        mingw32*|mingw64*|win32|win64)
> +            decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 -loleaut32"
> +            decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
> +            ;;
> +    esac
> +fi
> +
> disabled securetransport || { check_func SecIdentityCreate "-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
>     check_lib securetransport "Security/SecureTransport.h Security/Security.h" "SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation -Wl,-framework,Security"; }
> 
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index f01fba953e..cbb591ce64 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -19,6 +19,12 @@
>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>  */
> 
> +/* Include internal.h first to avoid conflict between winsock.h (used by
> + * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
> +
> #include <DeckLinkAPI.h>
> #ifdef _WIN32
> #include <DeckLinkAPI_i.c>
> @@ -28,7 +34,6 @@
> 
> extern "C" {
> #include "libavformat/avformat.h"
> -#include "libavformat/internal.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/intreadwrite.h"
> #include "libavutil/bswap.h"
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 67eaf97e89..69f790d606 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -19,12 +19,17 @@
>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>  */
> 
> +/* Include internal.h first to avoid conflict between winsock.h (used by
> + * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
> +
> #include <DeckLinkAPI.h>
> 
> extern "C" {
> #include "config.h"
> #include "libavformat/avformat.h"
> -#include "libavformat/internal.h"
> #include "libavutil/avutil.h"
> #include "libavutil/common.h"
> #include "libavutil/imgutils.h"
> @@ -262,8 +267,16 @@ static int64_t get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
>                 res = videoFrame->GetHardwareReferenceTimestamp(time_base.den, &bmd_pts, &bmd_duration);
>             break;
>         case PTS_SRC_WALLCLOCK:
> -            pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
> +        {
> +            /* MSVC does not support compound literals like AV_TIME_BASE_Q
> +             * in C++ code (compiler error C4576) */
> +            // pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);

I'd rather remove the old code, i see no point in keeping it as a comment.

> +            AVRational timebase;
> +            timebase.num = 1;
> +            timebase.den = AV_TIME_BASE;
> +            pts = av_rescale_q(wallclock, timebase, time_base);
>             break;
> +        }
>     }
>     if (res == S_OK)
>         pts = bmd_pts / time_base.num;
> diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
> index 5105967101..be01bcd64c 100644
> --- a/libavdevice/decklink_enc.cpp
> +++ b/libavdevice/decklink_enc.cpp
> @@ -22,11 +22,16 @@
> #include <atomic>
> using std::atomic;
> 
> +/* Include internal.h first to avoid conflict between winsock.h (used by
> + * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
> +
> #include <DeckLinkAPI.h>
> 
> extern "C" {
> #include "libavformat/avformat.h"
> -#include "libavformat/internal.h"
> #include "libavutil/imgutils.h"
> }
>

The decklink part seems fine otherwise, maybe you should get an ACK from 
Hendrik for the configure part, which I have no opinion about.

Regards,
Marton
diff mbox

Patch

diff --git a/configure b/configure
index 883ec390d6..bbb9fab738 100755
--- a/configure
+++ b/configure
@@ -4847,8 +4847,6 @@  case $target_os in
         else
             target_os=mingw32
         fi
-        decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 -loleaut32"
-        decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
         LIBTARGET=i386
         if enabled x86_64; then
             LIBTARGET="i386:x86-64"
@@ -5947,6 +5945,15 @@  if ! disabled sdl2; then
 fi
 enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs $sdl2_extralibs
 
+if enabled decklink; then
+    case $target_os in
+        mingw32*|mingw64*|win32|win64)
+            decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 -loleaut32"
+            decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
+            ;;
+    esac
+fi
+
 disabled securetransport || { check_func SecIdentityCreate "-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
     check_lib securetransport "Security/SecureTransport.h Security/Security.h" "SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation -Wl,-framework,Security"; }
 
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index f01fba953e..cbb591ce64 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -19,6 +19,12 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* Include internal.h first to avoid conflict between winsock.h (used by
+ * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include <DeckLinkAPI.h>
 #ifdef _WIN32
 #include <DeckLinkAPI_i.c>
@@ -28,7 +34,6 @@ 
 
 extern "C" {
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/bswap.h"
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 67eaf97e89..69f790d606 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -19,12 +19,17 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+/* Include internal.h first to avoid conflict between winsock.h (used by
+ * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include <DeckLinkAPI.h>
 
 extern "C" {
 #include "config.h"
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/avutil.h"
 #include "libavutil/common.h"
 #include "libavutil/imgutils.h"
@@ -262,8 +267,16 @@  static int64_t get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
                 res = videoFrame->GetHardwareReferenceTimestamp(time_base.den, &bmd_pts, &bmd_duration);
             break;
         case PTS_SRC_WALLCLOCK:
-            pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
+        {
+            /* MSVC does not support compound literals like AV_TIME_BASE_Q
+             * in C++ code (compiler error C4576) */
+            // pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
+            AVRational timebase;
+            timebase.num = 1;
+            timebase.den = AV_TIME_BASE;
+            pts = av_rescale_q(wallclock, timebase, time_base);
             break;
+        }
     }
     if (res == S_OK)
         pts = bmd_pts / time_base.num;
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index 5105967101..be01bcd64c 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -22,11 +22,16 @@ 
 #include <atomic>
 using std::atomic;
 
+/* Include internal.h first to avoid conflict between winsock.h (used by
+ * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
+extern "C" {
+#include "libavformat/internal.h"
+}
+
 #include <DeckLinkAPI.h>
 
 extern "C" {
 #include "libavformat/avformat.h"
-#include "libavformat/internal.h"
 #include "libavutil/imgutils.h"
 }