use bcrypt instead of the old wincrypt API

Message ID 20180330073605.15052-1-robux4@ycbcr.xyz
State New
Headers show
Series
  • use bcrypt instead of the old wincrypt API
Related show

Commit Message

Steve Lhomme March 30, 2018, 7:36 a.m.
When targeting Windows Vista and above
The wincrypt API is deprecated and not allowed for Windows Store apps.
---
 configure               |  4 +++-
 libavutil/random_seed.c | 16 ++++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Diego Biurrun March 30, 2018, 8:46 a.m. | #1
On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
> --- a/configure
> +++ b/configure
> @@ -1708,6 +1708,7 @@ SYSTEM_LIBRARIES="
>      vaapi_x11
>      vdpau_x11
> +    bcrypt
>      wincrypt
>  "

This should be ordered.

> @@ -2611,7 +2612,7 @@ avdevice_extralibs="libm_extralibs"
>  avfilter_extralibs="pthreads_extralibs libm_extralibs"
>  avresample_extralibs="libm_extralibs"
> -avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs wincrypt_extralibs"
> +avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs bcrypt_extralibs wincrypt_extralibs"

same

> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>  check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>  check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>  check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt

Do you really need to check the Vista condition? What about using bcrypt
unconditionally if available? The variable name with an uppercase letter
and a '+' is slightly odd. I'm not sure if it can cause problems but I
cannot rule it out offhand either.

Diego
Steve Lhomme March 30, 2018, 10:38 a.m. | #2
Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>> --- a/configure
>> +++ b/configure
>> @@ -1708,6 +1708,7 @@ SYSTEM_LIBRARIES="
>>       vaapi_x11
>>       vdpau_x11
>> +    bcrypt
>>       wincrypt
>>   "
> This should be ordered.
>
>> @@ -2611,7 +2612,7 @@ avdevice_extralibs="libm_extralibs"
>>   avfilter_extralibs="pthreads_extralibs libm_extralibs"
>>   avresample_extralibs="libm_extralibs"
>> -avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs wincrypt_extralibs"
>> +avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs bcrypt_extralibs wincrypt_extralibs"
> same
>
>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
> Do you really need to check the Vista condition? What about using bcrypt

Yes, you need to use it only on builds that won't run on XP. Otherwise 
it will fail to load the bcrypt.dll and the whole libavutil DLL (or 
whatever its form) will fail to load. It would be possible to do it 
dynamically but IMO it's overkill. It's not really a critical component. 
But with time if XP support is dropped this check can go and wincrypt 
dropped entirely.

> unconditionally if available? The variable name with an uppercase letter
> and a '+' is slightly odd. I'm not sure if it can cause problems but I
> cannot rule it out offhand either.

It seems the same is only used in config.log. And the + didn't cause any 
problem for me.

>
> Diego
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Diego Biurrun March 30, 2018, 1:38 p.m. | #3
On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
> > On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
> > > --- a/configure
> > > +++ b/configure
> > > @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
> > >   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
> > >   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
> > >   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
> > > +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
> > Do you really need to check the Vista condition? What about using bcrypt
> > unconditionally if available?
> 
> Yes, you need to use it only on builds that won't run on XP. Otherwise it
> will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
> its form) will fail to load. It would be possible to do it dynamically but
> IMO it's overkill. It's not really a critical component.

Is bcrypt available on XP? If no then the CPP condition check would seem
unnecessary. You could just check for bcrypt and bcrypt being available
would imply Vista. I think I'm missing something.

> But with time if XP support is dropped this check can go and wincrypt
> dropped entirely.

Is it maybe time to consider dropping XP support?

> > The variable name with an uppercase letter
> > and a '+' is slightly odd. I'm not sure if it can cause problems but I
> > cannot rule it out offhand either.
> 
> It seems the same is only used in config.log. And the + didn't cause any
> problem for me.

I remain sceptical; "it worked for me" is usually not a good argument when
considering edge cases ;)

Diego
James Almer March 30, 2018, 1:43 p.m. | #4
On 3/30/2018 10:38 AM, Diego Biurrun wrote:
> On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
>>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt

If you don't need to set any variable then just use test_cpp_condition()

>>> Do you really need to check the Vista condition? What about using bcrypt
>>> unconditionally if available?
>>
>> Yes, you need to use it only on builds that won't run on XP. Otherwise it
>> will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
>> its form) will fail to load. It would be possible to do it dynamically but
>> IMO it's overkill. It's not really a critical component.
> 
> Is bcrypt available on XP? If no then the CPP condition check would seem
> unnecessary. You could just check for bcrypt and bcrypt being available
> would imply Vista. I think I'm missing something.

check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt

Seems to succeed even if targeting XP, at least on mingw-w64.

> 
>> But with time if XP support is dropped this check can go and wincrypt
>> dropped entirely.
> 
> Is it maybe time to consider dropping XP support?
> 
>>> The variable name with an uppercase letter
>>> and a '+' is slightly odd. I'm not sure if it can cause problems but I
>>> cannot rule it out offhand either.
>>
>> It seems the same is only used in config.log. And the + didn't cause any
>> problem for me.
> 
> I remain sceptical; "it worked for me" is usually not a good argument when
> considering edge cases ;)
> 
> Diego
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
>
Diego Biurrun March 30, 2018, 1:54 p.m. | #5
On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
> >>>> --- a/configure
> >>>> +++ b/configure
> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
> 
> If you don't need to set any variable then just use test_cpp_condition()

Yes, good point.

> >>> Do you really need to check the Vista condition? What about using bcrypt
> >>> unconditionally if available?
> >>
> >> Yes, you need to use it only on builds that won't run on XP. Otherwise it
> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
> >> its form) will fail to load. It would be possible to do it dynamically but
> >> IMO it's overkill. It's not really a critical component.
> > 
> > Is bcrypt available on XP? If no then the CPP condition check would seem
> > unnecessary. You could just check for bcrypt and bcrypt being available
> > would imply Vista. I think I'm missing something.
> 
> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
> 
> Seems to succeed even if targeting XP, at least on mingw-w64.

Isn't that wrong then?

Diego
Martin Storsjö March 30, 2018, 1:57 p.m. | #6
On Fri, 30 Mar 2018, Diego Biurrun wrote:

> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>> >>>> --- a/configure
>> >>>> +++ b/configure
>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>> 
>> If you don't need to set any variable then just use test_cpp_condition()
>
> Yes, good point.
>
>> >>> Do you really need to check the Vista condition? What about using bcrypt
>> >>> unconditionally if available?
>> >>
>> >> Yes, you need to use it only on builds that won't run on XP. Otherwise it
>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
>> >> its form) will fail to load. It would be possible to do it dynamically but
>> >> IMO it's overkill. It's not really a critical component.
>> > 
>> > Is bcrypt available on XP? If no then the CPP condition check would seem
>> > unnecessary. You could just check for bcrypt and bcrypt being available
>> > would imply Vista. I think I'm missing something.
>> 
>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>> 
>> Seems to succeed even if targeting XP, at least on mingw-w64.
>
> Isn't that wrong then?

I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs 
guarding the availability of this function in the headers. (The official 
windows SDK might, although that SDK also have dropped XP support long ago 
iirc.)

// Martin
Martin Storsjö March 30, 2018, 1:58 p.m. | #7
On Fri, 30 Mar 2018, Diego Biurrun wrote:

> On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>> > On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>> > > --- a/configure
>> > > +++ b/configure
>> > > @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>> > >   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>> > >   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>> > >   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
>> > > +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>> > Do you really need to check the Vista condition? What about using bcrypt
>> > unconditionally if available?
>> 
>> Yes, you need to use it only on builds that won't run on XP. Otherwise it
>> will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
>> its form) will fail to load. It would be possible to do it dynamically but
>> IMO it's overkill. It's not really a critical component.
>
> Is bcrypt available on XP? If no then the CPP condition check would seem
> unnecessary. You could just check for bcrypt and bcrypt being available
> would imply Vista. I think I'm missing something.
>
>> But with time if XP support is dropped this check can go and wincrypt
>> dropped entirely.
>
> Is it maybe time to consider dropping XP support?

I wouldn't mind.

See e.g. 9b121dfc32810250938021952aab4172a988cb56 in ffmpeg; dropping XP 
support simplifies the w32pthreads wrapper and allows using better 
synchronization primitives, that allow e.g. static initialization of 
mutexes.

// Martin
James Almer March 30, 2018, 2:14 p.m. | #8
On 3/30/2018 10:57 AM, Martin Storsjö wrote:
> On Fri, 30 Mar 2018, Diego Biurrun wrote:
> 
>> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>> >>>> --- a/configure
>>> >>>> +++ b/configure
>>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"           
>>> CoTaskMemFree        -lole32
>>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW  
>>> -lshell32
>>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom      
>>> -ladvapi32
>>> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo
>>> -lpsapi
>>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600"
>>> && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>
>>> If you don't need to set any variable then just use test_cpp_condition()
>>
>> Yes, good point.
>>
>>> >>> Do you really need to check the Vista condition? What about using
>>> bcrypt
>>> >>> unconditionally if available?
>>> >>
>>> >> Yes, you need to use it only on builds that won't run on XP.
>>> Otherwise it
>>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or
>>> whatever
>>> >> its form) will fail to load. It would be possible to do it
>>> dynamically but
>>> >> IMO it's overkill. It's not really a critical component.
>>> > > Is bcrypt available on XP? If no then the CPP condition check
>>> would seem
>>> > unnecessary. You could just check for bcrypt and bcrypt being
>>> available
>>> > would imply Vista. I think I'm missing something.
>>>
>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>
>>> Seems to succeed even if targeting XP, at least on mingw-w64.
>>
>> Isn't that wrong then?
> 
> I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs
> guarding the availability of this function in the headers. (The official
> windows SDK might, although that SDK also have dropped XP support long
> ago iirc.)

bcrypt.h on mingw-w64 is completely wrapped in checks like

#if WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) || _WIN32_WINNT
>= 0x0A00

The former is the reason it succeeds in XP, seeing the latter is
checking for Windows 10 or newer.
Diego Biurrun March 30, 2018, 2:41 p.m. | #9
On Fri, Mar 30, 2018 at 04:58:29PM +0300, Martin Storsjö wrote:
> On Fri, 30 Mar 2018, Diego Biurrun wrote:
> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
> > > Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
> > > > On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
> > > > >   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
> > > > >   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
> > > > >   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
> > > > > +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
> > > > Do you really need to check the Vista condition? What about using bcrypt
> > > > unconditionally if available?
> > > 
> > > Yes, you need to use it only on builds that won't run on XP. Otherwise it
> > > will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
> > > its form) will fail to load. It would be possible to do it dynamically but
> > > IMO it's overkill. It's not really a critical component.
> > 
> > Is bcrypt available on XP? If no then the CPP condition check would seem
> > unnecessary. You could just check for bcrypt and bcrypt being available
> > would imply Vista. I think I'm missing something.
> > 
> > > But with time if XP support is dropped this check can go and wincrypt
> > > dropped entirely.
> > 
> > Is it maybe time to consider dropping XP support?
> 
> I wouldn't mind.

Let's go ahead then.

> See e.g. 9b121dfc32810250938021952aab4172a988cb56 in ffmpeg; dropping XP
> support simplifies the w32pthreads wrapper and allows using better
> synchronization primitives, that allow e.g. static initialization of
> mutexes.

Do we need to do more changes apart from importing that commit?

Diego
Martin Storsjö March 30, 2018, 6:13 p.m. | #10
On Fri, 30 Mar 2018, James Almer wrote:

> On 3/30/2018 10:57 AM, Martin Storsjö wrote:
>> On Fri, 30 Mar 2018, Diego Biurrun wrote:
>> 
>>> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>>>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>>>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>>>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>>> >>>> --- a/configure
>>>> >>>> +++ b/configure
>>>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"           
>>>> CoTaskMemFree        -lole32
>>>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW  
>>>> -lshell32
>>>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom      
>>>> -ladvapi32
>>>> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo
>>>> -lpsapi
>>>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600"
>>>> && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>
>>>> If you don't need to set any variable then just use test_cpp_condition()
>>>
>>> Yes, good point.
>>>
>>>> >>> Do you really need to check the Vista condition? What about using
>>>> bcrypt
>>>> >>> unconditionally if available?
>>>> >>
>>>> >> Yes, you need to use it only on builds that won't run on XP.
>>>> Otherwise it
>>>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or
>>>> whatever
>>>> >> its form) will fail to load. It would be possible to do it
>>>> dynamically but
>>>> >> IMO it's overkill. It's not really a critical component.
>>>> > > Is bcrypt available on XP? If no then the CPP condition check
>>>> would seem
>>>> > unnecessary. You could just check for bcrypt and bcrypt being
>>>> available
>>>> > would imply Vista. I think I'm missing something.
>>>>
>>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>
>>>> Seems to succeed even if targeting XP, at least on mingw-w64.
>>>
>>> Isn't that wrong then?
>> 
>> I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs
>> guarding the availability of this function in the headers. (The official
>> windows SDK might, although that SDK also have dropped XP support long
>> ago iirc.)
>
> bcrypt.h on mingw-w64 is completely wrapped in checks like
>
> #if WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) || _WIN32_WINNT
>> = 0x0A00
>
> The former is the reason it succeeds in XP, seeing the latter is
> checking for Windows 10 or newer.

Hmm, ok. I guess the correct form would be something like 
"(WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) && _WIN32_WINNT >= 
0x0600) || _WIN32_WINNT >= 0x0A00" then.

// Martin
Martin Storsjö March 30, 2018, 6:13 p.m. | #11
On Fri, 30 Mar 2018, Diego Biurrun wrote:

> On Fri, Mar 30, 2018 at 04:58:29PM +0300, Martin Storsjö wrote:
>> On Fri, 30 Mar 2018, Diego Biurrun wrote:
>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>> > > Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>> > > > On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>> > > > > --- a/configure
>> > > > > +++ b/configure
>> > > > > @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
>> > > > >   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
>> > > > >   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
>> > > > >   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
>> > > > > +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>> > > > Do you really need to check the Vista condition? What about using bcrypt
>> > > > unconditionally if available?
>> > > 
>> > > Yes, you need to use it only on builds that won't run on XP. Otherwise it
>> > > will fail to load the bcrypt.dll and the whole libavutil DLL (or whatever
>> > > its form) will fail to load. It would be possible to do it dynamically but
>> > > IMO it's overkill. It's not really a critical component.
>> > 
>> > Is bcrypt available on XP? If no then the CPP condition check would seem
>> > unnecessary. You could just check for bcrypt and bcrypt being available
>> > would imply Vista. I think I'm missing something.
>> > 
>> > > But with time if XP support is dropped this check can go and wincrypt
>> > > dropped entirely.
>> > 
>> > Is it maybe time to consider dropping XP support?
>> 
>> I wouldn't mind.
>
> Let's go ahead then.
>
>> See e.g. 9b121dfc32810250938021952aab4172a988cb56 in ffmpeg; dropping XP
>> support simplifies the w32pthreads wrapper and allows using better
>> synchronization primitives, that allow e.g. static initialization of
>> mutexes.
>
> Do we need to do more changes apart from importing that commit?

Don't think so, except for whatever configure differences there are.

// Martin
James Almer March 30, 2018, 7 p.m. | #12
On 3/30/2018 3:13 PM, Martin Storsjö wrote:
> On Fri, 30 Mar 2018, James Almer wrote:
> 
>> On 3/30/2018 10:57 AM, Martin Storsjö wrote:
>>> On Fri, 30 Mar 2018, Diego Biurrun wrote:
>>>
>>>> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>>>>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>>>>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>>>>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>>>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>>>> >>>> --- a/configure
>>>>> >>>> +++ b/configure
>>>>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"           
>>>>> CoTaskMemFree        -lole32
>>>>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW  
>>>>> -lshell32
>>>>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom      
>>>>> -ladvapi32
>>>>> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo
>>>>> -lpsapi
>>>>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600"
>>>>> && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>>
>>>>> If you don't need to set any variable then just use
>>>>> test_cpp_condition()
>>>>
>>>> Yes, good point.
>>>>
>>>>> >>> Do you really need to check the Vista condition? What about using
>>>>> bcrypt
>>>>> >>> unconditionally if available?
>>>>> >>
>>>>> >> Yes, you need to use it only on builds that won't run on XP.
>>>>> Otherwise it
>>>>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or
>>>>> whatever
>>>>> >> its form) will fail to load. It would be possible to do it
>>>>> dynamically but
>>>>> >> IMO it's overkill. It's not really a critical component.
>>>>> > > Is bcrypt available on XP? If no then the CPP condition check
>>>>> would seem
>>>>> > unnecessary. You could just check for bcrypt and bcrypt being
>>>>> available
>>>>> > would imply Vista. I think I'm missing something.
>>>>>
>>>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>>
>>>>> Seems to succeed even if targeting XP, at least on mingw-w64.
>>>>
>>>> Isn't that wrong then?
>>>
>>> I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs
>>> guarding the availability of this function in the headers. (The official
>>> windows SDK might, although that SDK also have dropped XP support long
>>> ago iirc.)
>>
>> bcrypt.h on mingw-w64 is completely wrapped in checks like
>>
>> #if WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) || _WIN32_WINNT
>>> = 0x0A00
>>
>> The former is the reason it succeeds in XP, seeing the latter is
>> checking for Windows 10 or newer.
> 
> Hmm, ok. I guess the correct form would be something like
> "(WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) && _WIN32_WINNT >=
> 0x0600) || _WIN32_WINNT >= 0x0A00" then.
> 
> // Martin

The WINAPI_PARTITION_DESKTOP check is already done in configure to
enable or disable the uwp variable.

In any case, does this mean that on uwp neither BCryptGenRandom or
CryptGenRandom are available/allowed?
Martin Storsjö March 30, 2018, 7:07 p.m. | #13
On Fri, 30 Mar 2018, James Almer wrote:

> On 3/30/2018 3:13 PM, Martin Storsjö wrote:
>> On Fri, 30 Mar 2018, James Almer wrote:
>> 
>>> On 3/30/2018 10:57 AM, Martin Storsjö wrote:
>>>> On Fri, 30 Mar 2018, Diego Biurrun wrote:
>>>>
>>>>> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>>>>>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>>>>>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>>>>>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>>>>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>>>>> >>>> --- a/configure
>>>>>> >>>> +++ b/configure
>>>>>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"           
>>>>>> CoTaskMemFree        -lole32
>>>>>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW  
>>>>>> -lshell32
>>>>>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom      
>>>>>> -ladvapi32
>>>>>> >>>>   check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo
>>>>>> -lpsapi
>>>>>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600"
>>>>>> && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>>>
>>>>>> If you don't need to set any variable then just use
>>>>>> test_cpp_condition()
>>>>>
>>>>> Yes, good point.
>>>>>
>>>>>> >>> Do you really need to check the Vista condition? What about using
>>>>>> bcrypt
>>>>>> >>> unconditionally if available?
>>>>>> >>
>>>>>> >> Yes, you need to use it only on builds that won't run on XP.
>>>>>> Otherwise it
>>>>>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or
>>>>>> whatever
>>>>>> >> its form) will fail to load. It would be possible to do it
>>>>>> dynamically but
>>>>>> >> IMO it's overkill. It's not really a critical component.
>>>>>> > > Is bcrypt available on XP? If no then the CPP condition check
>>>>>> would seem
>>>>>> > unnecessary. You could just check for bcrypt and bcrypt being
>>>>>> available
>>>>>> > would imply Vista. I think I'm missing something.
>>>>>>
>>>>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>>>
>>>>>> Seems to succeed even if targeting XP, at least on mingw-w64.
>>>>>
>>>>> Isn't that wrong then?
>>>>
>>>> I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs
>>>> guarding the availability of this function in the headers. (The official
>>>> windows SDK might, although that SDK also have dropped XP support long
>>>> ago iirc.)
>>>
>>> bcrypt.h on mingw-w64 is completely wrapped in checks like
>>>
>>> #if WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) || _WIN32_WINNT
>>>> = 0x0A00
>>>
>>> The former is the reason it succeeds in XP, seeing the latter is
>>> checking for Windows 10 or newer.
>> 
>> Hmm, ok. I guess the correct form would be something like
>> "(WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) && _WIN32_WINNT >=
>> 0x0600) || _WIN32_WINNT >= 0x0A00" then.
>> 
>> // Martin
>
> The WINAPI_PARTITION_DESKTOP check is already done in configure to
> enable or disable the uwp variable.

Not sure I see how that relates... that part of the header guard makes it 
visible on and makes the check succeed when targeting XP, even though it 
really isn't available there according to Steve.

> In any case, does this mean that on uwp neither BCryptGenRandom or
> CryptGenRandom are available/allowed?

The way I read that, for UWP on Win10, the bcrypt.h stuff should be fine, 
no? (Based on the mingw-w64 header guards, it might not be for win8/8.1 
RT/store/UWP/whatever apps, although MSDN doesn't seem to say anything 
about it.)

// Martin
Steve Lhomme April 3, 2018, 9:43 a.m. | #14
Le 30/03/2018 à 21:07, Martin Storsjö a écrit :
> On Fri, 30 Mar 2018, James Almer wrote:
>
>> On 3/30/2018 3:13 PM, Martin Storsjö wrote:
>>> On Fri, 30 Mar 2018, James Almer wrote:
>>>
>>>> On 3/30/2018 10:57 AM, Martin Storsjö wrote:
>>>>> On Fri, 30 Mar 2018, Diego Biurrun wrote:
>>>>>
>>>>>> On Fri, Mar 30, 2018 at 10:43:27AM -0300, James Almer wrote:
>>>>>>> On 3/30/2018 10:38 AM, Diego Biurrun wrote:
>>>>>>> > On Fri, Mar 30, 2018 at 12:38:05PM +0200, Steve Lhomme wrote:
>>>>>>> >> Le 30/03/2018 à 10:46, Diego Biurrun a écrit :
>>>>>>> >>> On Fri, Mar 30, 2018 at 09:36:05AM +0200, Steve Lhomme wrote:
>>>>>>> >>>> --- a/configure
>>>>>>> >>>> +++ b/configure
>>>>>>> >>>> @@ -4581,6 +4582,7 @@ check_lib ole32    "windows.h"
>>>>>>> CoTaskMemFree        -lole32
>>>>>>> >>>>   check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW
>>>>>>> -lshell32
>>>>>>> >>>>   check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom
>>>>>>> -ladvapi32
>>>>>>> >>>>   check_lib psapi    "windows.h psapi.h"    
>>>>>>> GetProcessMemoryInfo
>>>>>>> -lpsapi
>>>>>>> >>>> +check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600"
>>>>>>> && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
>>>>>>>
>>>>>>> If you don't need to set any variable then just use
>>>>>>> test_cpp_condition()
>>>>>>
>>>>>> Yes, good point.
>>>>>>
>>>>>>> >>> Do you really need to check the Vista condition? What about 
>>>>>>> using
>>>>>>> bcrypt
>>>>>>> >>> unconditionally if available?
>>>>>>> >>
>>>>>>> >> Yes, you need to use it only on builds that won't run on XP.
>>>>>>> Otherwise it
>>>>>>> >> will fail to load the bcrypt.dll and the whole libavutil DLL (or
>>>>>>> whatever
>>>>>>> >> its form) will fail to load. It would be possible to do it
>>>>>>> dynamically but
>>>>>>> >> IMO it's overkill. It's not really a critical component.
>>>>>>> > > Is bcrypt available on XP? If no then the CPP condition check
>>>>>>> would seem
>>>>>>> > unnecessary. You could just check for bcrypt and bcrypt being
>>>>>>> available
>>>>>>> > would imply Vista. I think I'm missing something.
>>>>>>>
>>>>>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt
>>>>>>>
>>>>>>> Seems to succeed even if targeting XP, at least on mingw-w64.
>>>>>>
>>>>>> Isn't that wrong then?
>>>>>
>>>>> I guess it just means that mingw-w64 doesn't have _WIN32_WINNT ifdefs
>>>>> guarding the availability of this function in the headers. (The 
>>>>> official
>>>>> windows SDK might, although that SDK also have dropped XP support 
>>>>> long
>>>>> ago iirc.)
>>>>
>>>> bcrypt.h on mingw-w64 is completely wrapped in checks like
>>>>
>>>> #if WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) || _WIN32_WINNT
>>>>> = 0x0A00
>>>>
>>>> The former is the reason it succeeds in XP, seeing the latter is
>>>> checking for Windows 10 or newer.
>>>
>>> Hmm, ok. I guess the correct form would be something like
>>> "(WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP) && _WIN32_WINNT >=
>>> 0x0600) || _WIN32_WINNT >= 0x0A00" then.
>>>
>>> // Martin
>>
>> The WINAPI_PARTITION_DESKTOP check is already done in configure to
>> enable or disable the uwp variable.
>
> Not sure I see how that relates... that part of the header guard makes 
> it visible on and makes the check succeed when targeting XP, even 
> though it really isn't available there according to Steve.
>
>> In any case, does this mean that on uwp neither BCryptGenRandom or
>> CryptGenRandom are available/allowed?
>
> The way I read that, for UWP on Win10, the bcrypt.h stuff should be 
> fine, no? (Based on the mingw-w64 header guards, it might not be for 
> win8/8.1 RT/store/UWP/whatever apps, although MSDN doesn't seem to say 
> anything about it.)

It's available for 8.1 and even before for Winstore apps. That's why I 
only added the Vista check. Everything above, on all possible targets, 
is supported.

>
> // Martin
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel

Patch

diff --git a/configure b/configure
index 7612a6052c..60bca6ff25 100755
--- a/configure
+++ b/configure
@@ -1708,6 +1708,7 @@  SYSTEM_LIBRARIES="
     vaapi_drm
     vaapi_x11
     vdpau_x11
+    bcrypt
     wincrypt
 "
 
@@ -2611,7 +2612,7 @@  avdevice_extralibs="libm_extralibs"
 avformat_extralibs="libm_extralibs"
 avfilter_extralibs="pthreads_extralibs libm_extralibs"
 avresample_extralibs="libm_extralibs"
-avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs wincrypt_extralibs"
+avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs bcrypt_extralibs wincrypt_extralibs"
 swscale_extralibs="libm_extralibs"
 
 # programs
@@ -4581,6 +4582,7 @@  check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
 check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
 check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
 check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
+check_cpp_condition Vista+ windows.h "_WIN32_WINNT >= 0x0600" && check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom  -lbcrypt
 
 check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
 
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 089d883916..f9686dfae2 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -23,7 +23,9 @@ 
 #if HAVE_UNISTD_H
 #include <unistd.h>
 #endif
-#if HAVE_WINCRYPT
+#if HAVE_BCRYPT
+#include <bcrypt.h>
+#elif HAVE_WINCRYPT
 #include <windows.h>
 #include <wincrypt.h>
 #endif
@@ -96,7 +98,17 @@  uint32_t av_get_random_seed(void)
 {
     uint32_t seed;
 
-#if HAVE_WINCRYPT
+#if HAVE_BCRYPT
+    BCRYPT_ALG_HANDLE algo_handle;
+    NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
+                                               MS_PRIMITIVE_PROVIDER, 0);
+    if (BCRYPT_SUCCESS(ret)) {
+        NTSTATUS ret = BCryptGenRandom(algo_handle, &seed, sizeof(seed), 0);
+        BCryptCloseAlgorithmProvider(algo_handle, 0);
+        if (BCRYPT_SUCCESS(ret))
+            return seed;
+    }
+#elif HAVE_WINCRYPT
     HCRYPTPROV provider;
     if (CryptAcquireContext(&provider, NULL, NULL, PROV_RSA_FULL,
                             CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) {