Re: [FFmpeg-devel] [PATCH v3 1/3] avformat/udp: return the error code instead of generic EIO
On Sat, Jan 09, 2021 at 12:23:42AM +0100, Marton Balint wrote: > > > On Fri, 8 Jan 2021, lance.lmw...@gmail.com wrote: > > > From: Limin Wang > > > > Signed-off-by: Limin Wang > > --- > > libavformat/udp.c | 42 ++ > > 1 file changed, 22 insertions(+), 20 deletions(-) > > > > diff --git a/libavformat/udp.c b/libavformat/udp.c > > index 13c346a..28987e0 100644 > > --- a/libavformat/udp.c > > +++ b/libavformat/udp.c > > @@ -633,6 +633,8 @@ static int udp_open(URLContext *h, const char *uri, int > > flags) > > char buf[256]; > > struct sockaddr_storage my_addr; > > socklen_t len; > > +int ret = AVERROR(EIO); > > +int net_ret = 0; > > > > h->is_streamed = 1; > > > > @@ -641,12 +643,12 @@ static int udp_open(URLContext *h, const char *uri, > > int flags) > > s->buffer_size = is_output ? UDP_TX_BUF_SIZE : UDP_RX_BUF_SIZE; > > > > if (s->sources) { > > -if (ff_ip_parse_sources(h, s->sources, >filters) < 0) > > +if ((ret = ff_ip_parse_sources(h, s->sources, >filters)) < 0) > > goto fail; > > } > > > > if (s->block) { > > -if (ff_ip_parse_blocks(h, s->block, >filters) < 0) > > +if ((ret = ff_ip_parse_blocks(h, s->block, >filters)) < 0) > > goto fail; > > } > > > > @@ -712,11 +714,11 @@ static int udp_open(URLContext *h, const char *uri, > > int flags) > > av_strlcpy(localaddr, buf, sizeof(localaddr)); > > } > > if (av_find_info_tag(buf, sizeof(buf), "sources", p)) { > > -if (ff_ip_parse_sources(h, buf, >filters) < 0) > > +if ((ret = ff_ip_parse_sources(h, buf, >filters)) < 0) > > goto fail; > > } > > if (av_find_info_tag(buf, sizeof(buf), "block", p)) { > > -if (ff_ip_parse_blocks(h, buf, >filters) < 0) > > +if ((ret = ff_ip_parse_blocks(h, buf, >filters)) < 0) > > goto fail; > > } > > if (!is_output && av_find_info_tag(buf, sizeof(buf), "timeout", p)) > > @@ -742,7 +744,7 @@ static int udp_open(URLContext *h, const char *uri, int > > flags) > > if (!(flags & AVIO_FLAG_READ)) > > goto fail; > > } else { > > -if (ff_udp_set_remote_url(h, uri) < 0) > > +if ((ret = ff_udp_set_remote_url(h, uri)) < 0) > > goto fail; > > } > > > > @@ -763,13 +765,13 @@ static int udp_open(URLContext *h, const char *uri, > > int flags) > > */ > > if (s->reuse_socket > 0 || (s->is_multicast && s->reuse_socket < 0)) { > > s->reuse_socket = 1; > > -if (setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, > > &(s->reuse_socket), sizeof(s->reuse_socket)) != 0) > > +if ((net_ret = setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, > > &(s->reuse_socket), sizeof(s->reuse_socket))) != 0) > > goto fail; > > } > > > > if (s->is_broadcast) { > > #ifdef SO_BROADCAST > > -if (setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, > > &(s->is_broadcast), sizeof(s->is_broadcast)) != 0) > > +if ((net_ret = setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, > > &(s->is_broadcast), sizeof(s->is_broadcast))) != 0) > > #endif > >goto fail; > > } > > @@ -788,7 +790,7 @@ static int udp_open(URLContext *h, const char *uri, int > > flags) > > > > if (dscp >= 0) { > > dscp <<= 2; > > -if (setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , sizeof(dscp)) > > != 0) > > +if ((net_ret = setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , > > sizeof(dscp))) != 0) > > goto fail; > > } > > > > @@ -802,7 +804,7 @@ static int udp_open(URLContext *h, const char *uri, int > > flags) > > /* bind to the local address if not multicast or if the multicast > > * bind failed */ > > /* the bind is needed to give a port to the socket now */ > > -if (bind_ret < 0 && bind(udp_fd,(struct sockaddr *)_addr, len) < 0) > > { > > +if (bind_ret < 0 && (net_ret = bind(udp_fd,(struct sockaddr > > *)_addr, len)) < 0) { > > ff_log_net_error(h, AV_LOG_ERROR, "bind failed"); > > goto fail; > > } > > @@ -814,28 +816,28 @@ static int udp_open(URLContext *h, const char *uri, > > int flags) > > if (s->is_multicast) { > > if (h->flags & AVIO_FLAG_WRITE) { > > /* output */ > > -if (udp_set_multicast_ttl(udp_fd, s->ttl, (struct sockaddr > > *)>dest_addr) < 0) > > +if ((net_ret = udp_set_multicast_ttl(udp_fd, s->ttl, (struct > > sockaddr *)>dest_addr)) < 0) > > I suggest you make this ret as well and change udp_set_multicast_ttl to > return ff_neterrno() instead of -1 on error. OK, will change by suggestion. > > > goto fail; > > } > > if (h->flags & AVIO_FLAG_READ) { > > /* input */ > > if (s->filters.nb_include_addrs) { > > -if (udp_set_multicast_sources(h, udp_fd, > > +if
Re: [FFmpeg-devel] [PATCH v3 1/3] avformat/udp: return the error code instead of generic EIO
On Fri, 8 Jan 2021, lance.lmw...@gmail.com wrote: From: Limin Wang Signed-off-by: Limin Wang --- libavformat/udp.c | 42 ++ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/libavformat/udp.c b/libavformat/udp.c index 13c346a..28987e0 100644 --- a/libavformat/udp.c +++ b/libavformat/udp.c @@ -633,6 +633,8 @@ static int udp_open(URLContext *h, const char *uri, int flags) char buf[256]; struct sockaddr_storage my_addr; socklen_t len; +int ret = AVERROR(EIO); +int net_ret = 0; h->is_streamed = 1; @@ -641,12 +643,12 @@ static int udp_open(URLContext *h, const char *uri, int flags) s->buffer_size = is_output ? UDP_TX_BUF_SIZE : UDP_RX_BUF_SIZE; if (s->sources) { -if (ff_ip_parse_sources(h, s->sources, >filters) < 0) +if ((ret = ff_ip_parse_sources(h, s->sources, >filters)) < 0) goto fail; } if (s->block) { -if (ff_ip_parse_blocks(h, s->block, >filters) < 0) +if ((ret = ff_ip_parse_blocks(h, s->block, >filters)) < 0) goto fail; } @@ -712,11 +714,11 @@ static int udp_open(URLContext *h, const char *uri, int flags) av_strlcpy(localaddr, buf, sizeof(localaddr)); } if (av_find_info_tag(buf, sizeof(buf), "sources", p)) { -if (ff_ip_parse_sources(h, buf, >filters) < 0) +if ((ret = ff_ip_parse_sources(h, buf, >filters)) < 0) goto fail; } if (av_find_info_tag(buf, sizeof(buf), "block", p)) { -if (ff_ip_parse_blocks(h, buf, >filters) < 0) +if ((ret = ff_ip_parse_blocks(h, buf, >filters)) < 0) goto fail; } if (!is_output && av_find_info_tag(buf, sizeof(buf), "timeout", p)) @@ -742,7 +744,7 @@ static int udp_open(URLContext *h, const char *uri, int flags) if (!(flags & AVIO_FLAG_READ)) goto fail; } else { -if (ff_udp_set_remote_url(h, uri) < 0) +if ((ret = ff_udp_set_remote_url(h, uri)) < 0) goto fail; } @@ -763,13 +765,13 @@ static int udp_open(URLContext *h, const char *uri, int flags) */ if (s->reuse_socket > 0 || (s->is_multicast && s->reuse_socket < 0)) { s->reuse_socket = 1; -if (setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, &(s->reuse_socket), sizeof(s->reuse_socket)) != 0) +if ((net_ret = setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, &(s->reuse_socket), sizeof(s->reuse_socket))) != 0) goto fail; } if (s->is_broadcast) { #ifdef SO_BROADCAST -if (setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, &(s->is_broadcast), sizeof(s->is_broadcast)) != 0) +if ((net_ret = setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, &(s->is_broadcast), sizeof(s->is_broadcast))) != 0) #endif goto fail; } @@ -788,7 +790,7 @@ static int udp_open(URLContext *h, const char *uri, int flags) if (dscp >= 0) { dscp <<= 2; -if (setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , sizeof(dscp)) != 0) +if ((net_ret = setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , sizeof(dscp))) != 0) goto fail; } @@ -802,7 +804,7 @@ static int udp_open(URLContext *h, const char *uri, int flags) /* bind to the local address if not multicast or if the multicast * bind failed */ /* the bind is needed to give a port to the socket now */ -if (bind_ret < 0 && bind(udp_fd,(struct sockaddr *)_addr, len) < 0) { +if (bind_ret < 0 && (net_ret = bind(udp_fd,(struct sockaddr *)_addr, len)) < 0) { ff_log_net_error(h, AV_LOG_ERROR, "bind failed"); goto fail; } @@ -814,28 +816,28 @@ static int udp_open(URLContext *h, const char *uri, int flags) if (s->is_multicast) { if (h->flags & AVIO_FLAG_WRITE) { /* output */ -if (udp_set_multicast_ttl(udp_fd, s->ttl, (struct sockaddr *)>dest_addr) < 0) +if ((net_ret = udp_set_multicast_ttl(udp_fd, s->ttl, (struct sockaddr *)>dest_addr)) < 0) I suggest you make this ret as well and change udp_set_multicast_ttl to return ff_neterrno() instead of -1 on error. goto fail; } if (h->flags & AVIO_FLAG_READ) { /* input */ if (s->filters.nb_include_addrs) { -if (udp_set_multicast_sources(h, udp_fd, +if ((net_ret = udp_set_multicast_sources(h, udp_fd, This is ret as far as I see. (struct sockaddr *)>dest_addr, s->dest_addr_len, >local_addr_storage, s->filters.include_addrs, - s->filters.nb_include_addrs, 1) < 0) + s->filters.nb_include_addrs, 1)) < 0) goto fail; } else { -if (udp_join_multicast_group(udp_fd, (struct