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

[Nicolas George]

jack (12023-02-28):
> Signed-off-by: jack <jack.wgm at 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