Re: [FFmpeg-devel] [PATCH] libavformat/tcp: add local_addr/local_port for network option

[jackarain <jack.wgm@gmail.com>]

Done.

Nicolas George <george@nsup.org> 于2023年2月28日周二 18:57写道：

> jack (12023-02-28):
> > Signed-off-by: jack <jack.wgm@gmail.com>
> > ---
> >  libavformat/tcp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
>
> Thanks for the patch
>
> You neglected to update the documentation, and a few remarks below.
>
> >
> > diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> > index a11ccbb913..598d61067e 100644
> > --- a/libavformat/tcp.c
> > +++ b/libavformat/tcp.c
> > @@ -36,6 +36,8 @@ typedef struct TCPContext {
> >      const AVClass *class;
> >      int fd;
> >      int listen;
> > +    int local_port;
> > +    char *local_addr;
> >      int open_timeout;
> >      int rw_timeout;
> >      int listen_timeout;
> > @@ -52,6 +54,8 @@ typedef struct TCPContext {
> >  #define E AV_OPT_FLAG_ENCODING_PARAM
> >  static const AVOption options[] = {
> >      { "listen",          "Listen for incoming connections",
> OFFSET(listen),         AV_OPT_TYPE_INT, { .i64 = 0 },     0,       2,
>  .flags = D|E },
> > +    { "local_port",      "Local port",
>        OFFSET(local_port),     AV_OPT_TYPE_INT,    { .i64 = -1 },      -1,
> INT_MAX, .flags = D|E },
> > +    { "local_addr",      "Local address",
>         OFFSET(local_addr),     AV_OPT_TYPE_STRING, { .str = NULL },
>  0,       0, .flags = D|E },
> >      { "timeout",     "set timeout (in microseconds) of socket I/O
> operations", OFFSET(rw_timeout),     AV_OPT_TYPE_INT, { .i64 = -1 },
>  -1, INT_MAX, .flags = D|E },
> >      { "listen_timeout",  "Connection awaiting timeout (in
> milliseconds)",      OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1
> },         -1, INT_MAX, .flags = D|E },
> >      { "send_buffer_size", "Socket send buffer size (in bytes)",
>         OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },
>  -1, INT_MAX, .flags = D|E },
> > @@ -73,6 +77,39 @@ static const AVClass tcp_class = {
> >  static void customize_fd(void *ctx, int fd)
> >  {
> >      TCPContext *s = ctx;
> > +
>
> > +    if (s->local_addr) {
>
> s->local_addr || s->local_port
>
> > +        struct addrinfo hints = { 0 }, *ai;
> > +        int ret;
> > +        int port = 0;
> > +
> > +        hints.ai_family = AF_UNSPEC;
> > +        hints.ai_socktype = SOCK_STREAM;
> > +
> > +        if (s->local_port > 0)
> > +            port = s->local_port;
> > +
>
> > +        ret = getaddrinfo(s->local_addr, "0", &hints, &ai);
>
> The provided port should be used instead of this "0", and be a string,
> and named service.
>
> (The remote port should be a string too, alas.)
>
> > +        if (ret) {
>
> > +            av_log(ctx, AV_LOG_WARNING,
> > +               "Failed to bind local addr: %s port: %d err: %s\n",
> > +                s->local_addr, s->local_port, gai_strerror(ret));
>
> Should be an error: failing to bind on the specified address can be a
> security issue. And the error message is misleading.
>
> > +        } else {
>
> > +            if (ai->ai_family == AF_INET6) {
> > +                struct sockaddr_in6 * sockaddr_v6 = (struct
> sockaddr_in6 *)ai->ai_addr;
> > +                sockaddr_v6->sin6_port = htons(port);
> > +            } else {
> > +                struct sockaddr_in * sockaddr = (struct sockaddr_in
> *)ai->ai_addr;
> > +                sockaddr->sin_port = htons(port);
> > +            }
>
> Do not distinguish between protocols: ai_addr already contains all that
> is needed.
>
> > +            ret = bind(fd, (struct sockaddr *)ai->ai_addr,
> (int)ai->ai_addrlen);
> > +            if (ret) {
> > +                av_log(ctx, AV_LOG_WARNING,
> > +                "Failed to bind local addr: %s port: %d err: %s\n",
> > +                    s->local_addr, s->local_port, gai_strerror(ret));
> > +            }
>
> You should be looping over the returned addresses.
>
> > +        }
> > +    }
> >      /* Set the socket's send or receive buffer sizes, if specified.
> >         If unspecified or setting fails, system default is used. */
> >      if (s->recv_buffer_size > 0) {
> > @@ -129,6 +166,13 @@ static int tcp_open(URLContext *h, const char *uri,
> int flags)
> >              if (buf == endptr)
> >                  s->listen = 1;
> >          }
> > +        if (av_find_info_tag(buf, sizeof(buf), "local_port", p)) {
> > +            s->local_port = strtol(buf, NULL, 10);
> > +        }
> > +        if (av_find_info_tag(buf, sizeof(buf), "local_addr", p)) {
> > +            av_freep(&s->local_addr);
> > +            s->local_addr = av_strndup(buf, strlen(buf));
> > +        }
> >          if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
> >              s->rw_timeout = strtol(buf, NULL, 10);
> >          }
>
> Regards,
>
> --
>   Nicolas George
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".