| Message ID | 20200813052814.4300-1-nsugino@3way.com.ar |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avformat/libsrt: Close listen fd in listener mode | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Thu, 13 Aug 2020, Nicolas Sugino wrote: > In listener mode the first fd is not closed when libsrt_close() is called because it is overwritten by the new accept fd. > Added the listen_fd to the context to properly close it when libsrt_close() is called. This fixes trac ticket #8372, right? Add a reference to it to the commit message. > > Signed-off-by: Nicolas Sugino <nsugino@3way.com.ar> > --- > libavformat/libsrt.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c > index 4719ce0d4b..02dab64e36 100644 > --- a/libavformat/libsrt.c > +++ b/libavformat/libsrt.c > @@ -53,6 +53,7 @@ enum SRTMode { > typedef struct SRTContext { > const AVClass *class; > int fd; > + int listen_fd; > int eid; > int64_t rw_timeout; > int64_t listen_timeout; > @@ -362,7 +363,7 @@ static int libsrt_set_options_pre(URLContext *h, int fd) > static int libsrt_setup(URLContext *h, const char *uri, int flags) > { > struct addrinfo hints = { 0 }, *ai, *cur_ai; > - int port, fd = -1; > + int port, fd = -1, listen_fd = -1; > SRTContext *s = h->priv_data; > const char *p; > char buf[256]; > @@ -439,6 +440,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) > // multi-client > if ((ret = libsrt_listen(s->eid, fd, cur_ai->ai_addr, cur_ai->ai_addrlen, h, s->listen_timeout)) < 0) > goto fail1; > + listen_fd = fd; > fd = ret; > } else { > if (s->mode == SRT_MODE_RENDEZVOUS) { > @@ -471,6 +473,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) > > h->is_streamed = 1; > s->fd = fd; > + s->listen_fd = listen_fd > -1 ? listen_fd : -1; Why not simply: s->listen_fd = listen_fd; Also note that listen_fd should also be cleaned up in both the failure paths same way as fd is cleaned up now. > > freeaddrinfo(ai); > return 0; > @@ -676,6 +679,9 @@ static int libsrt_close(URLContext *h) > > srt_close(s->fd); > > + if (s->listen_fd > -1) >= 0 to be consistent with other similar code. Thanks, Marton > + srt_close(s->listen_fd); > + > srt_epoll_release(s->eid); > > srt_cleanup(); > -- > 2.17.1 > > _______________________________________________ > 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".
El jue., 13 ago. 2020 a las 3:56, Marton Balint (<cus@passwd.hu>) escribió: > > > > On Thu, 13 Aug 2020, Nicolas Sugino wrote: > > > In listener mode the first fd is not closed when libsrt_close() is called because it is overwritten by the new accept fd. > > Added the listen_fd to the context to properly close it when libsrt_close() is called. > > This fixes trac ticket #8372, right? Add a reference to it to the commit > message. > > > > > Signed-off-by: Nicolas Sugino <nsugino@3way.com.ar> > > --- > > libavformat/libsrt.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c > > index 4719ce0d4b..02dab64e36 100644 > > --- a/libavformat/libsrt.c > > +++ b/libavformat/libsrt.c > > @@ -53,6 +53,7 @@ enum SRTMode { > > typedef struct SRTContext { > > const AVClass *class; > > int fd; > > + int listen_fd; > > int eid; > > int64_t rw_timeout; > > int64_t listen_timeout; > > @@ -362,7 +363,7 @@ static int libsrt_set_options_pre(URLContext *h, int fd) > > static int libsrt_setup(URLContext *h, const char *uri, int flags) > > { > > struct addrinfo hints = { 0 }, *ai, *cur_ai; > > - int port, fd = -1; > > + int port, fd = -1, listen_fd = -1; > > SRTContext *s = h->priv_data; > > const char *p; > > char buf[256]; > > @@ -439,6 +440,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) > > // multi-client > > if ((ret = libsrt_listen(s->eid, fd, cur_ai->ai_addr, cur_ai->ai_addrlen, h, s->listen_timeout)) < 0) > > goto fail1; > > + listen_fd = fd; > > fd = ret; > > } else { > > if (s->mode == SRT_MODE_RENDEZVOUS) { > > @@ -471,6 +473,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) > > > > h->is_streamed = 1; > > s->fd = fd; > > + s->listen_fd = listen_fd > -1 ? listen_fd : -1; > > Why not simply: > > s->listen_fd = listen_fd; > > Also note that listen_fd should also be cleaned up in both the failure > paths same way as fd is cleaned up now. > > > > > freeaddrinfo(ai); > > return 0; > > @@ -676,6 +679,9 @@ static int libsrt_close(URLContext *h) > > > > srt_close(s->fd); > > > > + if (s->listen_fd > -1) > > >= 0 to be consistent with other similar code. > > Thanks, > Marton > > > + srt_close(s->listen_fd); > > + > > srt_epoll_release(s->eid); > > > > srt_cleanup(); > > -- > > 2.17.1 > > > > _______________________________________________ > > 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". > _______________________________________________ > 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". Thanks for the comments, I've just sent a v2 including them
diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c index 4719ce0d4b..02dab64e36 100644 --- a/libavformat/libsrt.c +++ b/libavformat/libsrt.c @@ -53,6 +53,7 @@ enum SRTMode { typedef struct SRTContext { const AVClass *class; int fd; + int listen_fd; int eid; int64_t rw_timeout; int64_t listen_timeout; @@ -362,7 +363,7 @@ static int libsrt_set_options_pre(URLContext *h, int fd) static int libsrt_setup(URLContext *h, const char *uri, int flags) { struct addrinfo hints = { 0 }, *ai, *cur_ai; - int port, fd = -1; + int port, fd = -1, listen_fd = -1; SRTContext *s = h->priv_data; const char *p; char buf[256]; @@ -439,6 +440,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) // multi-client if ((ret = libsrt_listen(s->eid, fd, cur_ai->ai_addr, cur_ai->ai_addrlen, h, s->listen_timeout)) < 0) goto fail1; + listen_fd = fd; fd = ret; } else { if (s->mode == SRT_MODE_RENDEZVOUS) { @@ -471,6 +473,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int flags) h->is_streamed = 1; s->fd = fd; + s->listen_fd = listen_fd > -1 ? listen_fd : -1; freeaddrinfo(ai); return 0; @@ -676,6 +679,9 @@ static int libsrt_close(URLContext *h) srt_close(s->fd); + if (s->listen_fd > -1) + srt_close(s->listen_fd); + srt_epoll_release(s->eid); srt_cleanup();
In listener mode the first fd is not closed when libsrt_close() is called because it is overwritten by the new accept fd. Added the listen_fd to the context to properly close it when libsrt_close() is called. Signed-off-by: Nicolas Sugino <nsugino@3way.com.ar> --- libavformat/libsrt.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)