[FFmpeg-devel] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

On 4/13/2017 12:21 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 2:16 AM, Aaron Levinson <alevinsn@aracnet.com> wrote:
>> diff --git a/configure b/configure
>> index d13d60b..b0f7b1a 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3635,10 +3635,13 @@ case "$toolchain" in
>>          # successfully parses the version number of existing supported
>>          # versions that require the converter (MSVC 2010 and 2012).
>>          cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version \([[:digit:]]\{1,\}\)\..*/\1/p')
>> +        CXX_O='-Fo$@'
>>          if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
>>              cc_default="cl"
>> +            cxx_default="cl"
>>          else
>>              cc_default="c99wrap cl"
>> +            cxx_default="c99wrap cl"
>>          fi
>>          ld_default="$source_path/compat/windows/mslink"
>>          nm_default="dumpbin -symbols"
> 
> CXX_O is in the wrong spot, it should be set at the same place where
> CC_O is beint set, and not here.
> 
> - Hendrik

I believe that I've addressed your comment, and a new version of the patch can be found below.

Thanks,
Aaron Levinson

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

From 764aed11d179fd42d1aa8c1c507d7660386cfde6 Mon Sep 17 00:00:00 2001
From: Aaron Levinson <alevinsn@aracnet.com>
Date: Thu, 13 Apr 2017 01:13:07 -0700
Subject: [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows

Purpose: Made appropriate changes to be able to successfully
 build C++ files using a Visual C++ build on Windows.  Note that this is a 
 continuation of the "fixes w32pthreads to support C++" aspect of Kyle 
 Schwarz's "Remove pthread dependency" patch that is available at 
 https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
 as far as I can tell, there was no follow-up after it was rejected.

Notes: Used Visual Studio 2015 (with update 3) for this.  Altered
approach for specifying -Fo$@ in configure based on code review from
Hendrik Leppkes for first version of patch.

Comments:

-- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
   get it to build when it is being included in a C++ file and built
   with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
   described above.

-- configure: Made minor modifications to MSVC (Microsoft Visual C++)
   toolchain section to make sure that -Fo$@ is used when building
   object files but when the C++ compiler is used and also to use cl
   for the default C++ compiler.  This is currently only relevant for
   building the Blackmagic/Decklink-related files under avdevice.
---
 compat/w32pthreads.h | 24 ++++++++++++------------
 configure            |  6 +++++-
 2 files changed, 17 insertions(+), 13 deletions(-)

On Thu, Apr 13, 2017 at 10:23 AM, Aaron Levinson <alevinsn@aracnet.com> wrote:
> On 4/13/2017 12:21 AM, Hendrik Leppkes wrote:
>> On Thu, Apr 13, 2017 at 2:16 AM, Aaron Levinson <alevinsn@aracnet.com> wrote:
>>> diff --git a/configure b/configure
>>> index d13d60b..b0f7b1a 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3635,10 +3635,13 @@ case "$toolchain" in
>>>          # successfully parses the version number of existing supported
>>>          # versions that require the converter (MSVC 2010 and 2012).
>>>          cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version \([[:digit:]]\{1,\}\)\..*/\1/p')
>>> +        CXX_O='-Fo$@'
>>>          if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
>>>              cc_default="cl"
>>> +            cxx_default="cl"
>>>          else
>>>              cc_default="c99wrap cl"
>>> +            cxx_default="c99wrap cl"
>>>          fi
>>>          ld_default="$source_path/compat/windows/mslink"
>>>          nm_default="dumpbin -symbols"
>>
>> CXX_O is in the wrong spot, it should be set at the same place where
>> CC_O is beint set, and not here.
>>
>> - Hendrik
>
> I believe that I've addressed your comment, and a new version of the patch can be found below.
>
> Thanks,
> Aaron Levinson
>
> -----------------------------------------------------------------------------------------
>
> From 764aed11d179fd42d1aa8c1c507d7660386cfde6 Mon Sep 17 00:00:00 2001
> From: Aaron Levinson <alevinsn@aracnet.com>
> Date: Thu, 13 Apr 2017 01:13:07 -0700
> Subject: [PATCH] Made appropriate changes to be able to successfully build C++ files using a Visual C++ build on Windows
>
> Purpose: Made appropriate changes to be able to successfully
>  build C++ files using a Visual C++ build on Windows.  Note that this is a
>  continuation of the "fixes w32pthreads to support C++" aspect of Kyle
>  Schwarz's "Remove pthread dependency" patch that is available at
>  https://patchwork.ffmpeg.org/patch/2654/ .  This patch wasn't accepted, and
>  as far as I can tell, there was no follow-up after it was rejected.
>
> Notes: Used Visual Studio 2015 (with update 3) for this.  Altered
> approach for specifying -Fo$@ in configure based on code review from
> Hendrik Leppkes for first version of patch.
>
> Comments:
>
> -- compat/w32pthreads.h: Made appropriate changes to w32pthreads.h to
>    get it to build when it is being included in a C++ file and built
>    with Visual C++.  This is mostly a copy of Kyle Schwarz's patch as
>    described above.
>
> -- configure: Made minor modifications to MSVC (Microsoft Visual C++)
>    toolchain section to make sure that -Fo$@ is used when building
>    object files but when the C++ compiler is used and also to use cl
>    for the default C++ compiler.  This is currently only relevant for
>    building the Blackmagic/Decklink-related files under avdevice.
> ---
>  compat/w32pthreads.h | 24 ++++++++++++------------
>  configure            |  6 +++++-
>  2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
> index 0c9a7fa..a6c699b 100644
> --- a/compat/w32pthreads.h
> +++ b/compat/w32pthreads.h
> @@ -77,7 +77,7 @@ typedef struct pthread_cond_t {
>
>  static av_unused unsigned __stdcall attribute_align_arg win32thread_worker(void *arg)
>  {
> -    pthread_t *h = arg;
> +    pthread_t *h = (pthread_t*)arg;
>      h->ret = h->func(h->arg);
>      return 0;
>  }
> @@ -270,7 +270,7 @@ static av_unused int pthread_cond_init(pthread_cond_t *cond, const void *unused_
>      }
>
>      /* non native condition variables */
> -    win32_cond = av_mallocz(sizeof(win32_cond_t));
> +    win32_cond = (win32_cond_t*)av_mallocz(sizeof(win32_cond_t));
>      if (!win32_cond)
>          return ENOMEM;
>      cond->Ptr = win32_cond;
> @@ -288,7 +288,7 @@ static av_unused int pthread_cond_init(pthread_cond_t *cond, const void *unused_
>
>  static av_unused int pthread_cond_destroy(pthread_cond_t *cond)
>  {
> -    win32_cond_t *win32_cond = cond->Ptr;
> +    win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
>      /* native condition variables do not destroy */
>      if (cond_init)
>          return 0;
> @@ -305,7 +305,7 @@ static av_unused int pthread_cond_destroy(pthread_cond_t *cond)
>
>  static av_unused int pthread_cond_broadcast(pthread_cond_t *cond)
>  {
> -    win32_cond_t *win32_cond = cond->Ptr;
> +    win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
>      int have_waiter;
>
>      if (cond_broadcast) {
> @@ -337,7 +337,7 @@ static av_unused int pthread_cond_broadcast(pthread_cond_t *cond)
>
>  static av_unused int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
>  {
> -    win32_cond_t *win32_cond = cond->Ptr;
> +    win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
>      int last_waiter;
>      if (cond_wait) {
>          cond_wait(cond, mutex, INFINITE);
> @@ -369,7 +369,7 @@ static av_unused int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mu
>
>  static av_unused int pthread_cond_signal(pthread_cond_t *cond)
>  {
> -    win32_cond_t *win32_cond = cond->Ptr;
> +    win32_cond_t *win32_cond = (win32_cond_t*)cond->Ptr;
>      int have_waiter;
>      if (cond_signal) {
>          cond_signal(cond);
> @@ -400,17 +400,17 @@ static av_unused void w32thread_init(void)
>      HANDLE kernel_dll = GetModuleHandle(TEXT("kernel32.dll"));
>      /* if one is available, then they should all be available */
>      cond_init      = (void (WINAPI*)(pthread_cond_t *))
> -        GetProcAddress(kernel_dll, "InitializeConditionVariable");
> +        GetProcAddress((HMODULE)kernel_dll, "InitializeConditionVariable");
>      cond_broadcast = (void (WINAPI*)(pthread_cond_t *))
> -        GetProcAddress(kernel_dll, "WakeAllConditionVariable");
> +        GetProcAddress((HMODULE)kernel_dll, "WakeAllConditionVariable");
>      cond_signal    = (void (WINAPI*)(pthread_cond_t *))
> -        GetProcAddress(kernel_dll, "WakeConditionVariable");
> +        GetProcAddress((HMODULE)kernel_dll, "WakeConditionVariable");
>      cond_wait      = (BOOL (WINAPI*)(pthread_cond_t *, pthread_mutex_t *, DWORD))
> -        GetProcAddress(kernel_dll, "SleepConditionVariableCS");
> +        GetProcAddress((HMODULE)kernel_dll, "SleepConditionVariableCS");
>      initonce_begin = (BOOL (WINAPI*)(pthread_once_t *, DWORD, BOOL *, void **))
> -        GetProcAddress(kernel_dll, "InitOnceBeginInitialize");
> +        GetProcAddress((HMODULE)kernel_dll, "InitOnceBeginInitialize");
>      initonce_complete = (BOOL (WINAPI*)(pthread_once_t *, DWORD, void *))
> -        GetProcAddress(kernel_dll, "InitOnceComplete");
> +        GetProcAddress((HMODULE)kernel_dll, "InitOnceComplete");
>  #endif
>
>  }
> diff --git a/configure b/configure
> index b2fc781..6112b9b 100755
> --- a/configure
> +++ b/configure
> @@ -3637,8 +3637,10 @@ case "$toolchain" in
>          cl_major_ver=$(cl 2>&1 | sed -n 's/.*Version \([[:digit:]]\{1,\}\)\..*/\1/p')
>          if [ -z "$cl_major_ver" ] || [ $cl_major_ver -ge 18 ]; then
>              cc_default="cl"
> +            cxx_default="cl"
>          else
>              cc_default="c99wrap cl"
> +            cxx_default="c99wrap cl"
>          fi
>          ld_default="$source_path/compat/windows/mslink"
>          nm_default="dumpbin -symbols"
> @@ -3983,7 +3985,7 @@ probe_cc(){
>      _cc=$2
>      first=$3
>
> -    unset _type _ident _cc_c _cc_e _cc_o _flags _cflags
> +    unset _type _ident _cc_c _cc_e _cc_o _cxx_o _flags _cflags
>      unset _ld_o _ldflags _ld_lib _ld_path
>      unset _depflags _DEPCMD _DEPFLAGS
>      _flags_filter=echo
> @@ -4156,6 +4158,7 @@ probe_cc(){
>          fi
>          _cc_o='-Fo$@'
>          _cc_e='-P -Fi$@'
> +        _cxx_o='-Fo$@'
>          _flags_filter=msvc_flags
>          _ld_lib='lib%.a'
>          _ld_path='-libpath:'
> @@ -4196,6 +4199,7 @@ cflags_noopt=$_cflags_noopt
>  add_cflags $_flags $_cflags
>  cc_ldflags=$_ldflags
>  set_ccvars CC
> +set_ccvars CXX
>
>  probe_cc hostcc "$host_cc"
>  host_cflags_filter=$_flags_filter

Technically this just happens to work by accident because CC_O and
CXX_O get the same value, which is probably fine for all C++ compilers
we currently support.
set_ccvars always uses the _cc_o variable to set whatever namespace
you requested (_cxx_o is never used), so you could remove the _cxx_o
handling from probe_cc entirely and it would still work.

Full CXX probing through probe_cc would be the most complete solution,
but probably overkill?

- Hendrik