[FFmpeg-devel] [PATCH] Enable proper IPv6 functions on windows

[Martin Storsjö]

On Sat, 16 Jan 2010, Reimar D?ffinger wrote:

> On Sat, Jan 16, 2010 at 12:49:44AM +0200, Martin Storsj? wrote:
> >
> > Well, IMO setting the define is a good way to be able to use the normal 
> > configure checks to see what's available at the platform version we're 
> > targeting. So instead of littering the actual code with platform specific 
> > hacks, we simply check if function foo is available on the target 
> > platform.
> 
> Except that Windows is not Linux in that you can't expect a binary to (more
> or less) run where it was compiled. It's about as beautiful as a the square
> pressed through the round hole.

Yes, that's true, but when building, one could choose what binary 
compatibility to aim for using the --target-os parameter.

> > > Disabling gettaddrinfo sure is possible, but
> > > a) no need to hack the defines for that
> > > b) what is the big deal with just not enabling getaddrinfo with HAVE_WINSOCK2_H
> > > and start ff_get_procaddres with
> > > #if HAVE_WINSOCK2_H
> > >   int WSAAPI (*wingetaddrinfo)(const char *, const char *, const struct addrinfo *, struct addrinfo **);
> > >   HMODULE ws2mod = GetModuleHandle("ws2_32.dll");
> > >   wingetaddrinfo = GetProcAddress(ws2mod, "getaddrinfo");
> > >   if (wingetaddrinfo)
> > >     return wingetaddrinfo(...);
> > > #endif
> > 
> > I proposed a version of that, too, in 
> > http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2010-January/080687.html
> > but the simpler way to simply link directly to the getaddrinfo function 
> > was preferred by Ronald, in this follow-up:
> > http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2010-January/080708.html
> 
> Your approach uses an actual Load/Unload step in a special function,
> which IMO pointlessly complicates things.
> Even if LoadLibrary was needed you can just do Load, call function unload.
> Also I can not see the thread safety issue with dynamic loading, we are talking
> about a library we are _linking_ to, you're not going to tell me Windows will
> unload a Library you _linked_ to?

Yes, your approach is much simpler in that sense, and shouldn't have any 
thread safety issues either. GetModuleHandle instead of LoadLibrary should 
be ok, too, since we actually link to that dll. The only drawback is that 
it pollutes the generic fallback wrappers with platform specific code.

The thread safety problems I referred to was in my earlier attempt:
http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100107/492cbd8c/$
(This was modelled after similar dynamic loading of the ipv6 functions
that I've done for other projects.)

The original reason for LoadLibrary instead of GetModuleHandle, is that 
if one would want to support IPv6 on win2k, too, where it was an 
optionally installable component, one would need to load the 
getaddrinfo/freeaddrinfo/getnameinfo functions from wship6.dll instead. 
But I skipped that part for ffmpeg since that's undoubtedly unnecessary 
complication.

> > But yes, that would be the most compatible solution, ending up with a 
> > binary compatible with older platforms and still being able to utilize the 
> > newer functions when available.
> 
> Actually I'd be happy to just avoid the impenetrable and in half the cases
> simply working wrong (wince is only one case, and how will your wince hacks
> work if wince ever gets IPv6 support?) configure mess this is accumulating.

WinCE itself actually supports IPv6 already, but the mingw32ce headers may 
need a little fixing (currently that part is unmodified from the normal 
mingw version and only defines getaddrinfo if _WIN32_WINNT >= 0x0501, but 
probably should be improved to have a similar check for _WIN32_WCE 
instead.)

I personally don't really care, it's up to all of you to choose which 
approach you prefer. That is, either setting _WIN32_WINNT conditionally 
depending on the --target-os value and later checking for getaddrinfo 
using normal check_func_headers, or simply adding the 
GetModuleHandle/GetProcAddress routine to each of the three wrappers 
(getaddrinfo, freeaddrinfo, getnameinfo).

As for the first alternative, another option which hopefully wouldn't be 
all that messy and error prone wouuld be:

test $target_os = "mingw32" && add_cflags -D_WIN32_WINNT=0x0501

So that the define would only be added for the normal "mingw32" case; 
mingw32-win2k or -prexp or whatever won't get any such, neither would 
"mingw32ce".

// Martin