[FFmpeg-devel] [PATCH] Made minor changes to get the decklink avdevice code to build using Visual C++
Aaron Levinson
alevinsn at aracnet.com
Mon Apr 17 02:11:04 EEST 2017
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:
>> [...]
>>
>>> --------------------------------------------------------------------------------------------
>>>
>>>
>>> From 00fdc9d15414a92a155eb7d1394bac3736dc9405 Mon Sep 17 00:00:00 2001
>>> From: Aaron Levinson <alevinsn at aracnet.com>
>>> Date: Thu, 13 Apr 2017 14:22:19 -0700
>>> Subject: [PATCH] Made minor changes to get the decklink avdevice code
>>> to build using Visual C++.
>>>
>>
>> Maybe it just me, but as I mentioned earlier, I don't like too verbose
>> comments in the code, see below:
>>
>>> diff --git a/libavdevice/decklink_common.cpp
>>> b/libavdevice/decklink_common.cpp
>>> index f01fba9..523217c 100644
>>> --- a/libavdevice/decklink_common.cpp
>>> +++ b/libavdevice/decklink_common.cpp
>>> @@ -19,6 +19,15 @@
>>> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> 02110-1301 USA
>>> */
>>>
>>> +// Moved inclusion of internal.h here in order to get it to build
>>> successfully
>>> +// when using Visual C++ to build--otherwise, compilation errors result
>>> +// due to winsock.h (which is included indirectly by DeckLinkAPI.h and
>>> +// DeckLinkAPI_i.c) conflicting with winsock2.h, which is included by
>>> +// internal.h. If winsock2.h is included first, then the conflict is
>>> resolved.
>>
>> This can be as short as this:
>>
>> /* Include internal.h first to avoid conflict of winsock.h (used by
>> * DeckLink) and winsock2.h (used by libavformat) in MSVC++ builds */
>>
>> (for multiline comments I think /* */ is preferred)
>>
>> Although since you do this in multiple headers, maybe it is enough if
>> you specify the reason in the commit message, and delete the comment
>> from here entirely.
>
> I think it is a good idea to have a comment in the code, as then it is
> front in center if someone in the future wonders why internal.h precedes
> the other includes and decides to move it because it will look cleaner,
> thereby breaking the MSVC build. Although, in that case, maybe it would
> be preferable to have the same comment in each of the three places, as
> opposed to just one.
>
> A shorter comment is fine, and your example comment explains things well
> enough, although I tend to think that more information is better than
> less for comments in code. From my perspective, "used by DeckLink" is a
> bit vague, since technically, DeckLinkAPI.h and DeckLinkAPI_i.c would be
> generated by the user if the actual Blackmagic DeckLink SDK were used
> (these files are not actually supplied with the Blackmagic DeckLink
> SDK), which is how I got DeckLinkAPI.h and DeckLinkAPI_c.h when I built
> ffmpeg. Well, DeckLinkAPI.h is included in the Linux header files in
> the Blackmagic DeckLink SDK, but the _i.c file is not, and on Windows,
> neither file is provided.
>
> Regarding multi-line comments being wrapped in /* */ vs using // on each
> line, it doesn't so much matter in this case, but I personally find //
> more versatile because then it makes it easier to comment out blocks of
> code. But, if that's the standard for the ffmpeg code base, then I'll
> make that change.
>
>>> @@ -262,8 +265,18 @@ 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);
>>> + {
>>> + // doing the following rather than using AV_TIME_BASE_Q
>>> because
>>> + // AV_TIME_BASE_Q doesn't work when building with Visual
>>> C++
>>> + // for C++ files (works for C files). When building C++
>>> files,
>>> + // it results in compiler error C4576. At least, this is
>>> the case
>>> + // with Visual C++ 2015.
>>
>> And this is:
>>
>> // MSVC does not support compound literals like AV_TIME_BASE_Q in C++
>> code
>>
>>> + AVRational timebase;
>>> + timebase.num = 1;
>>> + timebase.den = AV_TIME_BASE;
>>> + pts = av_rescale_q(wallclock, timebase, time_base);
>>> break;
>>> + }
>>
>> This whole block needs to be indented 1 column more I think.
>
> I'll double-check later today to make sure that it is indented properly,
> adjust the comment, and submit a new patch then.
>
>> Regards,
>> Marton
>
> Thanks,
> Aaron
A new version of the patch (generated against the latest code in git) can be found below. Basically, I just improved the comments. There was nothing wrong with the spacing--I think it just looks off because the "break;" line is left alone.
Thanks,
Aaron Levinson
----------------------------------------------------------------------
>From f62e33366bd38e3eb15c37d6363d1862b34f5b9e Mon Sep 17 00:00:00 2001
From: Aaron Levinson <alevinsn at aracnet.com>
Date: Sun, 16 Apr 2017 16:03:56 -0700
Subject: [PATCH] Made minor changes to get the decklink avdevice code to
build using Visual C++.
Purpose: Made minor changes to get the decklink avdevice code to build
using Visual C++.
Notes: Used Visual Studio 2015 (with update 3) for this. Also 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).
---
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 ec4e016..0d63c3d 100755
--- a/configure
+++ b/configure
@@ -4872,8 +4872,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"
@@ -5975,6 +5973,15 @@ if ! disabled sdl2; then
fi
enabled sdl2 && enable sdl && add_cflags $sdl2_cflags && add_extralibs $sdl2_libs
+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 "Security/SecureTransport.h Security/Security.h" "SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
enable securetransport; }
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index f01fba9..cbb591c 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 67eaf97..69f790d 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 5105967..be01bcd 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"
}
--
2.10.1.windows.1
More information about the ffmpeg-devel
mailing list