[PR] avformat/rtspdec: send TEARDOWN on close despite user interrupt (PR #23406)
PR #23406 opened by Thomas Devoogdt (ThomasDevoogdt) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23406 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23406.patch When ffplay exits via SIGTERM/Ctrl-C, the AVFormatContext interrupt callback is set. The RTSP demuxer skipped TEARDOWN entirely, leaving the server session alive (~10 s), which caused "453 Not Enough Bandwidth" on immediate reconnect. Fix the problem at every layer it can occur: fftools/ffplay: make sigterm_handler() set a flag instead of calling exit() directly. The main event loop checks the flag and routes through do_exit() -> avformat_close_input() so that TEARDOWN is reached during normal shutdown. avformat/rtspdec: in rtsp_read_close(), replace the user interrupt callback with a 500 ms deadline callback on both the TLS URLContext and its TCP child before sending TEARDOWN. Without patching the TCP child, GnuTLS/OpenSSL push callbacks receive AVERROR_EXIT from the TCP write and previously returned 0 ("sent 0 bytes, try again"), causing an infinite busy-spin. Switch to ff_rtsp_send_cmd() (blocking) instead of ff_rtsp_send_cmd_async() so the client waits for the server's 200 OK before closing, guaranteeing the session is freed. avformat/tls: add ff_tls_get_underlying() to expose the TCP/UDP child URLContext of a TLS context through a proper API, avoiding direct access to private priv_data. avformat/tls_gnutls, tls_openssl: fix the AVERROR_EXIT -> 0 mapping in the TLS push callbacks. Returning 0 on interrupt signals "sent 0 bytes" to the TLS library and causes a busy-spin; use EINTR / hard error instead. Fixes: #23405 Signed-off-by: Thomas Devoogdt <thomas@devoogdt.com> # Summary of changes Briefly describe what this PR does and why. <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> From 236cba060fd6120ec804ace01df0b1cf7a19b138 Mon Sep 17 00:00:00 2001 From: Thomas Devoogdt <thomas@devoogdt.com> Date: Mon, 8 Jun 2026 10:58:52 +0200 Subject: [PATCH] avformat/rtspdec: send TEARDOWN on close despite user interrupt When ffplay exits via SIGTERM/Ctrl-C, the AVFormatContext interrupt callback is set. The RTSP demuxer skipped TEARDOWN entirely, leaving the server session alive (~10 s), which caused "453 Not Enough Bandwidth" on immediate reconnect. Fix the problem at every layer it can occur: fftools/ffplay: make sigterm_handler() set a flag instead of calling exit() directly. The main event loop checks the flag and routes through do_exit() -> avformat_close_input() so that TEARDOWN is reached during normal shutdown. avformat/rtspdec: in rtsp_read_close(), replace the user interrupt callback with a 500 ms deadline callback on both the TLS URLContext and its TCP child before sending TEARDOWN. Without patching the TCP child, GnuTLS/OpenSSL push callbacks receive AVERROR_EXIT from the TCP write and previously returned 0 ("sent 0 bytes, try again"), causing an infinite busy-spin. Switch to ff_rtsp_send_cmd() (blocking) instead of ff_rtsp_send_cmd_async() so the client waits for the server's 200 OK before closing, guaranteeing the session is freed. avformat/tls: add ff_tls_get_underlying() to expose the TCP/UDP child URLContext of a TLS context through a proper API, avoiding direct access to private priv_data. avformat/tls_gnutls, tls_openssl: fix the AVERROR_EXIT -> 0 mapping in the TLS push callbacks. Returning 0 on interrupt signals "sent 0 bytes" to the TLS library and causes a busy-spin; use EINTR / hard error instead. Fixes: #23405 Signed-off-by: Thomas Devoogdt <thomas@devoogdt.com> --- fftools/ffplay.c | 6 +++++- libavformat/rtspdec.c | 34 ++++++++++++++++++++++++++++++++-- libavformat/tls.c | 7 +++++++ libavformat/tls.h | 5 +++++ libavformat/tls_gnutls.c | 7 ++++--- libavformat/tls_openssl.c | 7 ++++--- 6 files changed, 57 insertions(+), 9 deletions(-) diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 28a83e079f..46ba85212a 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -361,6 +361,8 @@ static int64_t audio_callback_time; #define FF_QUIT_EVENT (SDL_USEREVENT + 2) +static volatile sig_atomic_t request_quit; + static SDL_Window *window; static SDL_Renderer *renderer; static SDL_RendererInfo renderer_info = {0}; @@ -1372,7 +1374,7 @@ static void do_exit(VideoState *is) static void sigterm_handler(int sig) { - exit(123); + request_quit = 1; } static void set_default_window_size(int width, int height, AVRational sar) @@ -3403,6 +3405,8 @@ static void refresh_loop_wait_event(VideoState *is, SDL_Event *event) { double remaining_time = 0.0; SDL_PumpEvents(); while (!SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT)) { + if (request_quit) + do_exit(is); if (!cursor_hidden && av_gettime_relative() - cursor_last_shown > CURSOR_HIDE_DELAY) { SDL_ShowCursor(0); cursor_hidden = 1; diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c index e0bdf9d4ac..32c38738c4 100644 --- a/libavformat/rtspdec.c +++ b/libavformat/rtspdec.c @@ -59,12 +59,42 @@ static const struct RTSPStatusMessage { { 0, "NULL" } }; +static int rtsp_teardown_interrupt_cb(void *opaque) +{ + return av_gettime_relative() >= *(int64_t *)opaque; +} + static int rtsp_read_close(AVFormatContext *s) { RTSPState *rt = s->priv_data; - if (!(rt->rtsp_flags & RTSP_FLAG_LISTEN)) - ff_rtsp_send_cmd_async(s, "TEARDOWN", rt->control_uri, NULL); + if (!(rt->rtsp_flags & RTSP_FLAG_LISTEN)) { + /* Replace the interrupt callback with a short timeout so TEARDOWN + * can go out despite a pending Ctrl-C. For RTSPS both the TLS + * URLContext and its TCP child need patching (each holds an + * independent copy of the interrupt callback). */ + int64_t deadline = av_gettime_relative() + 500000LL; /* 500 ms */ + AVIOInterruptCB timeout_cb = { rtsp_teardown_interrupt_cb, &deadline }; + URLContext *tcp_child = NULL; + AVIOInterruptCB saved_tls = { NULL, NULL }; + AVIOInterruptCB saved_tcp = { NULL, NULL }; + + if (rt->rtsp_hd_out) { + saved_tls = rt->rtsp_hd_out->interrupt_callback; + rt->rtsp_hd_out->interrupt_callback = timeout_cb; + /* For TLS, also patch the underlying TCP child. */ + tcp_child = ff_tls_get_underlying(rt->rtsp_hd_out); + if (tcp_child) { + saved_tcp = tcp_child->interrupt_callback; + tcp_child->interrupt_callback = timeout_cb; + } + } + ff_rtsp_send_cmd(s, "TEARDOWN", rt->control_uri, NULL, &(RTSPMessageHeader){0}, NULL); + if (rt->rtsp_hd_out) + rt->rtsp_hd_out->interrupt_callback = saved_tls; + if (tcp_child) + tcp_child->interrupt_callback = saved_tcp; + } ff_rtsp_close_streams(s); ff_rtsp_close_connections(s); diff --git a/libavformat/tls.c b/libavformat/tls.c index 3eab305f56..f027076b78 100644 --- a/libavformat/tls.c +++ b/libavformat/tls.c @@ -122,6 +122,13 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV return ret; } +URLContext *ff_tls_get_underlying(URLContext *h) +{ + if (!h || !h->prot || strcmp(h->prot->name, "tls") != 0) + return NULL; + return ((TLSShared *)h->priv_data)->tcp; +} + /** * Read all data from the given URL url and store it in the given buffer bp. */ diff --git a/libavformat/tls.h b/libavformat/tls.h index 971ae5c7a5..5f14bd5fe4 100644 --- a/libavformat/tls.h +++ b/libavformat/tls.h @@ -119,6 +119,11 @@ int ff_tls_parse_host(TLSShared *s, char *hostname, int hostname_size, int *port int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AVDictionary **options); +/** + * Return the underlying TCP/UDP URLContext of a TLS URLContext, or NULL. + */ +URLContext *ff_tls_get_underlying(URLContext *h); + int ff_url_read_all(const char *url, AVBPrint *bp); int ff_tls_set_external_socket(URLContext *h, URLContext *sock); diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c index aedbc66e56..c40c2cd7f3 100644 --- a/libavformat/tls_gnutls.c +++ b/libavformat/tls_gnutls.c @@ -475,9 +475,10 @@ static ssize_t gnutls_url_push(gnutls_transport_ptr_t transport, int ret = ffurl_write(uc, buf, len); if (ret >= 0) return ret; - if (ret == AVERROR_EXIT) - return 0; - if (ret == AVERROR(EAGAIN)) { + if (ret == AVERROR_EXIT) { + /* Use EINTR, not 0: returning 0 would cause GnuTLS to busy-spin. */ + errno = EINTR; + } else if (ret == AVERROR(EAGAIN)) { errno = EAGAIN; } else { errno = EIO; diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c index 7c1289c595..81540cfeeb 100644 --- a/libavformat/tls_openssl.c +++ b/libavformat/tls_openssl.c @@ -570,9 +570,10 @@ static int url_bio_bwrite(BIO *b, const char *buf, int len) if (ret >= 0) return ret; BIO_clear_retry_flags(b); - if (ret == AVERROR_EXIT) - return 0; - if (ret == AVERROR(EAGAIN)) + if (ret == AVERROR_EXIT) { + /* Don't return 0: that signals success and silently drops the data. */ + c->io_err = ret; + } else if (ret == AVERROR(EAGAIN)) BIO_set_retry_write(b); else c->io_err = ret; -- 2.52.0
participants (1)
-
Thomas Devoogdt