On Thu, Apr 13, 2017 at 11:04 AM, Aaron Levinson <alevi...@aracnet.com> wrote:
> On 4/13/2017 1:27 AM, Hendrik Leppkes wrote:
>> On Thu, Apr 13, 2017 at 10:23 AM, Aaron Levinson <alevi...@aracnet.com>
>> wrote:
>>> 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
>
> Yes, I realize now that my use of _cxx_o had no effect, and it was simply the
> use of set_ccvars CXX that caused the CXX_ variables to be updated. I would
> tend to agree that full CXX probing is probably overkill given that there are
> only are few C++ files in ffmpeg, and those files have to do with Decklink,
> which is only supported on a few operating systems as well.
>
> I also realize now that this patch is more general than one that just applies
> to MSVC, although, in practice, the change will only benefit the Intel
> compiler, in addition to MSVC. Hopefully, I won't be asked to separate out
> the set_ccvars CXX line into a separate patch :-)
>
> Here is hopefully the final version of the patch with updated comments to
> reflect the more general nature of the patch.
>
> Thanks,
> Aaron Levinson
>
> ----------------------------------------------------------------------
>
> From 415f1f233eb963318d436fe272c3b80dda9d2d4d Mon Sep 17 00:00:00 2001
> From: Aaron Levinson <alevi...@aracnet.com>
> Date: Thu, 13 Apr 2017 01:45:23 -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:
> a) Now calling set_ccvars CXX to cause the various CXX_ variables to
> be setup properly. For example, with MSVC (Microsoft Visual C++),
> this causes CXX_O to be set to -Fo$@ instead of using the default
> value. The default value does not work with Visual C++. This
> change will also have the impact of correcting CXX_O (and possibly
> CXX_C) for other compilers, although this is really only relevant
> for the Intel compiler, in addition to MSVC.
> b) Now using cl for the C++ compiler for the MSVC toolchain. This is
> currently only relevant for building the
> Blackmagic/Decklink-related files under avdevice.
> ---
> compat/w32pthreads.h | 24 ++++++++++++------------
> configure | 3 +++
> 2 files changed, 15 insertions(+), 12 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
>
One more comment that I missed earlier (sorry), GetModuleHandle
returns the type HMODULE, can't you just change the type of kernel_dll
to match that, and avoid the casts here?
> }
> diff --git a/configure b/configure
> index b2fc781..43f0938 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"
> @@ -4196,6 +4198,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
This looks ok now, thanks.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
