[FFmpeg-devel] [PATCH] Use getaddrinfo in resolve_host (ipv6 support)

[Ronald S. Bultje]

Hi,

On 8/23/07, Michael Niedermayer <michaelni at gmx.at> wrote:

> > Index: ffmpeg/libavformat/avformat.h
> > ===================================================================
> > --- ffmpeg.orig/libavformat/avformat.h        2007-08-14 09:38:
> 26.000000000 -0400
> > +++ ffmpeg/libavformat/avformat.h     2007-08-19 16:21:56.000000000-0400
> > @@ -27,6 +27,7 @@
> >
> >  #define LIBAVFORMAT_IDENT       "Lavf"
> AV_STRINGIFY(LIBAVFORMAT_VERSION)
> >
> > +#include <sys/socket.h> /* struct sockaddr */
>
> ugly ...


Actually, that was when I embedded struct sockaddr in AVInetAddr, which
isn't right since the size is larger for ipv6, so I made it a pointer later
and forgot to remove this. It's gone now.

> +/**
> > + * Structure containing internet hostname lookup data in a way that
> > + * can be used to create sockets for use with connect().
> > + */
> > +struct AVInetAddr {
> > +     int family, socket_type, protocol, addr_len;
> > +     struct sockaddr *addr;
> > +     AVInetAddr *next;
>
> tabs


Removed.

> +int av_resolve_host(AVInetAddr **in_addr, int port, int passive,
> > +                    int socket_stream, const char *hostname);
>
> can we keep this stuff please private to av*/ffserver for the moment
> instead of providing it in libavformats public API


OK, it always used to be in avformat.h so I kept it there, but removed the
av_* prefix which suggests that it is public. Eventually, when people are ok
with the API and udp.c is fixed, I'd like it public.

> +    for (res = resl; res != NULL; res = res->ai_next) {
>
> superflous != NULL


Removed.

> +    for (n = 0, res = resl, addri = addr; res != NULL;
> > +         addr = addr->next, res = res->ai_next) {
>
> no please, dont put 6 statements in a for and then split the
> line randomly


I changed this a bit, it's one line now.

> Index: ffmpeg/ffserver.c
> > ===================================================================
> > --- ffmpeg.orig/ffserver.c    2007-08-14 09:38:26.000000000 -0400
> > +++ ffmpeg/ffserver.c 2007-08-19 16:47:57.000000000 -0400
> > @@ -3792,6 +3792,9 @@
> >      FFStream **last_feed, *feed;
> >      AVCodecContext audio_enc, video_enc;
> >      int audio_id, video_id;
> > +    char http_host[1024] = {'\0'}, rtsp_host[1024] = {'\0'};
>
> '\0' == 0 IIRC


I guess you mean the default initialization values should be NULL already,
right? I removed the explicit initialization, I hope that's what you meant.

New patch attached, ffserver is still not fixed (should that be in here or
in a separate patch?), all the rest should be ok. I also modified the
!CONFIG_IPV6 a bit so that if it returns multiple addresses (h_addr_list),
we use each of them. Not sure if that needs specific configure checks, or if
you want that in a separate patch also (in that case, I'll just revert it
for now). It works for me both wth and without CONFIG_IPV6, so should be
ready for applying except for the ffserver mess.

Ronald
-------------- next part --------------
A non-text attachment was scrubbed...
Name: resolvehost-ipv6.patch
Type: application/octet-stream
Size: 13414 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070825/91ee8364/attachment.obj>