From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 2FADC45407 for ; Tue, 28 Feb 2023 10:57:33 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4400968A799; Tue, 28 Feb 2023 12:57:30 +0200 (EET) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 47FC6688386 for ; Tue, 28 Feb 2023 12:57:24 +0200 (EET) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 31SAvNWv008430 ; Tue, 28 Feb 2023 11:57:23 +0100 Received: by phare.normalesup.org (Postfix, from userid 1001) id 0FAD7EB5BD; Tue, 28 Feb 2023 11:57:23 +0100 (CET) Date: Tue, 28 Feb 2023 11:57:23 +0100 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <20230228101640.3337990-1-jack.wgm@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230228101640.3337990-1-jack.wgm@gmail.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Tue, 28 Feb 2023 11:57:23 +0100 (CET) Subject: Re: [FFmpeg-devel] [PATCH] libavformat/tcp: add local_addr/local_port for network option X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: jack Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: jack (12023-02-28): > Signed-off-by: jack > --- > 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".