[FFmpeg-devel] [PATCH] IPv6 support v.2

Luca Abeni lucabe72
Mon Nov 26 16:56:47 CET 2007


Hi Ronald,

Ronald S. Bultje wrote:
> Hi Luca,
> 
> On Nov 26, 2007 2:41 AM, Luca Abeni <lucabe72 at email.it> wrote:
> 
>> My understanding was that you was going to post some updated patches after
>> I committed my ones (you said: "I'll say it this way: there's multiple
>> ways to get there, and I think your patches are a good start in one such
>> direction to fix the issue, and I'll gladly pick up from there and resolve
>> the whole thing").
>>
> 
> Ok, but I still need to know how:
> 
> I could duplicate udp.c's ipv6_resolve_host() everywhere

I think duplication is not an option.


> generalize it and
> keep it either in udp.c and export it or move it back into os_support.c

I believe that if if can be generalized without increasing its complexity
too much, then it should be generalized and moved in a generic "network.c"
file, which contains all the networking code shared between UDP and TCP.
I also believe that os_support.c should disappear...

> or I could remove it completely and integrate
> the call into the function caller in udp.c and duplicate that code into
> tcp.c.

I do not like this solution (because of code duplication, and because I
believe that functions should be reasonably small).


> For udp.c, I would highly prefer to not only remove the ipv4-code path, but
> also integrate all the single-use functions that we have now back into their
> respective callers (udp_port(), udp_set_url()); this would basically undo
> most of the work that you did so far, but I'd consider it a cleanup, since
> if you remove the ipv4-code, the result is a bit like spaghetti...

When I posted my patches, I clearly stated that I was only trying to help,
and if the patches were not useful, I did not want to insist for having them
applied.
It seemed to me that people agreed about the patches. Now, you basically
want to revert them. If this was your goal, you could have stated it since
the beginning, so that I could have avoided the work of writing the patches,
testing them, and committing them.
We clearly have different opinions about spaghetti programming, and I believe
that splitting a big function in smaller ones having meaningful names improves
the readability of the code. But again, this is just my opinion.


> Then there's a tenfold of code in sdp.c, ffserver.c and rtsp.c involving
> changing in_addr / sockaddr_in into generalized containers holders  and
> inet_aton -> inet_pton or getnameinfo; my patches handled most of these
> already and would probably still be useful afterwards (although I'd need to
> change them to use addrinfo directly instead of this avnet stuff).

Yes, I think so.


> In the end, a lot of things and various ways to go, since I did 3 approaches
> so far (uses error code from gethostbyname, switch to gethostbyname_r and
> switch to getaddrinfo in a wrapper) and none of them got me anywhere, I'd
> like a clear path for me before I start, I'm not going to waste another
> month on these patches and see them rejected again.

Maybe the point is that we should proceed by small steps:
1) ensure that the "getaddrinfo" code works well on the most used OSs (hint:
    mingw seems to provide getaddrinfo(), but the configure script is not able
    to detect it, and when compiling with mingw CONFIG_IPV6 is disabled).
2) remove the IPV4-only code from udp.c
3) wait some time, and see if people complain. Implement a getaddrinfo replacement
    in an external library (maybe the libossupport - or whatever is the name - that
    Michael is requesting?), for the (hopefully few) systems that do not provide it.
    Someone said that this should be fairly easy... Maybe he can provide some help on this?
4) convert tcp.c
5) convert rtsp.c and sdp.c
6) fix ffserver.c

Again, this is just my opinion. I do not want to force anyone to follow it.
If people agree that this is the way to go, I can help following the path I
indicated.


				Luca




More information about the ffmpeg-devel mailing list