On Thu, Apr 13, 2017 at 2:36 PM, wm4 <nfx...@googlemail.com> wrote:
> On Thu, 13 Apr 2017 02:47:39 -0700
> Aaron Levinson <alevi...@aracnet.com> wrote:
>
>> On 4/13/2017 2:10 AM, Hendrik Leppkes wrote:
>> > On Thu, Apr 13, 2017 at 11:04 AM, Aaron Levinson <alevi...@aracnet.com> 
>> > wrote:
>> >>  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?
>> >
>> > - Hendrik
>>
>> Yes, it is simpler and cleaner to properly set the return value of 
>> GetModuleHandle() to an HMODULE.  Here is what is hopefully the final 
>> version of the patch, which is also much cleaner and simplified from the 
>> original version.
>>
>> Thanks,
>> Aaron
>>
>> ------------------------------------------------------------------
>>
>> From 56f9a4b6c281a0d9382d2b4ec2e29aff5ab69fbc Mon Sep 17 00:00:00 2001
>> From: Aaron Levinson <alevi...@aracnet.com>
>> Date: Thu, 13 Apr 2017 02:38:02 -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.  Also simplified changes
>> to w32pthreads.h per Hendrik's review.
>>
>> 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 | 14 +++++++-------
>>  configure            |  3 +++
>>  2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
>> index 0c9a7fa..eeead60 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);
>> @@ -397,7 +397,7 @@ static av_unused int pthread_cond_signal(pthread_cond_t 
>> *cond)
>>  static av_unused void w32thread_init(void)
>>  {
>>  #if _WIN32_WINNT < 0x0600
>> -    HANDLE kernel_dll = GetModuleHandle(TEXT("kernel32.dll"));
>> +    HMODULE 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");
>> 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
>
> LGTM, although in my opinion it would be better to move the
> implementations to a .c file to avoid accidental breakage of C++ code.
> (But of course that's out of scope of the patch.)

Pushed with one additional hunk cherry-picked from another patch in
this series (after discussion and testing on IRC), and a slightly
revised commit message.

Thanks!

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to