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

[Marton Balint]

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.

> @@ -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.

Regards,
Marton