PR #24358 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24358 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24358.patch
Split off from #23934 >From 8cc44df76d991c1426369db72fe66642e8bc904a Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 25 Jun 2026 13:54:35 +0200 Subject: [PATCH 1/6] avformat/libcurl: consolidate s->error and s->eof s->eof can just be AVERROR_EOF. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 44 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index bb5fa0f002..f50b71ed6f 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -150,8 +150,7 @@ struct CurlContext { pthread_cond_t cond; AVFifo *fifo; int paused; /* write callback paused, FIFO was full */ - int eof; /* producer delivered all data */ - int error; /* AVERROR for an unrecoverable failure, or 0 */ + int status; /* current stream status (AVERROR code) */ int aborted; /* transfer should stop (open was interrupted) */ }; @@ -309,8 +308,8 @@ static size_t header_callback(char *ptr, size_t size, size_t nitems, void *userd "with offset %"PRId64" (expected %"PRId64")\n", content_start, c->request_start); c->stream_ok = 0; - if (!c->error) - c->error = AVERROR(EIO); + if (!c->status) + c->status = AVERROR(EIO); pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); return len; @@ -360,8 +359,8 @@ static size_t header_callback(char *ptr, size_t size, size_t nitems, void *userd c->seekable = c->seekable_opt; } else { c->stream_ok = 0; - if (!c->error) - c->error = ff_http_averror(status, AVERROR(EIO)); + if (!c->status) + c->status = ff_http_averror(status, AVERROR(EIO)); } c->probed = 1; pthread_cond_broadcast(&c->cond); @@ -417,8 +416,8 @@ static void start_request(CurlContext *c) curl_multi_strerror(res)); c->active = 0; pthread_mutex_lock(&c->mutex); - if (!c->error) - c->error = AVERROR(EIO); + if (!c->status) + c->status = AVERROR(EIO); pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); } @@ -459,8 +458,8 @@ static void on_done(CurlContext *c, CURLcode code) received = c->request_received; /* Advance past delivered bytes so a retry or seek resumes at the right offset. */ if (received > INT64_MAX - c->request_start) { - if (!c->error) - c->error = AVERROR(EIO); + if (!c->status) + c->status = AVERROR(EIO); received = 0; aborted = 1; pthread_cond_broadcast(&c->cond); @@ -475,8 +474,8 @@ static void on_done(CurlContext *c, CURLcode code) pthread_mutex_lock(&c->mutex); c->probed = 1; c->stream_ok = 0; - if (!c->error) - c->error = curlcode_to_averror(code); + if (!c->status) + c->status = curlcode_to_averror(code); pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); return; @@ -493,7 +492,7 @@ static void on_done(CurlContext *c, CURLcode code) return; } pthread_mutex_lock(&c->mutex); - c->eof = 1; + c->status = AVERROR_EOF; pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); return; @@ -512,8 +511,8 @@ static void on_done(CurlContext *c, CURLcode code) if (!aborted) { pthread_mutex_lock(&c->mutex); - if (!c->error) - c->error = curlcode_to_averror(code); + if (!c->status) + c->status = curlcode_to_averror(code); pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); } @@ -552,8 +551,7 @@ static void execute_command(CurlLoop *loop, CurlCmd *cmd) pthread_mutex_lock(&c->mutex); av_fifo_reset2(c->fifo); c->paused = 0; - c->eof = 0; - c->error = 0; + c->status = 0; pthread_mutex_unlock(&c->mutex); c->request_start = cmd->pos; c->retry_count = 0; @@ -956,7 +954,7 @@ static int wait_for_probe(CurlContext *c) int ret = 0; pthread_mutex_lock(&c->mutex); - while (!c->probed && !c->error) { + while (!c->probed && !c->status) { if (ff_check_interrupt(&h->interrupt_callback)) { c->aborted = 1; ret = AVERROR_EXIT; @@ -966,7 +964,7 @@ static int wait_for_probe(CurlContext *c) } if (!ret) { if (!c->stream_ok) - ret = c->error ? c->error : AVERROR(EIO); + ret = c->status ? c->status : AVERROR(EIO); } pthread_mutex_unlock(&c->mutex); @@ -1072,12 +1070,8 @@ static int libcurl_read(URLContext *h, unsigned char *buf, int size) curl_dispatch(c->loop, CMD_UNPAUSE, c, 0, 0); return n; } - if (c->error) { - ret = c->error; - break; - } - if (c->eof) { - ret = AVERROR_EOF; + if (c->status) { + ret = c->status; break; } if (nonblock) { -- 2.52.0 >From 4fc17664cb55c7e488d00886e24513abc54ead1b Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 9 Jul 2026 16:23:43 +0200 Subject: [PATCH 2/6] avformat/libcurl: early-exit on_done() when aborted Simplifies this logic by not having to embed a check for !aborted in every conditional branch. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index f50b71ed6f..334c33e432 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -481,7 +481,10 @@ static void on_done(CurlContext *c, CURLcode code) return; } - if (code == CURLE_OK && !aborted && c->stream_ok) { + if (aborted) + return; + + if (code == CURLE_OK && c->stream_ok) { c->retry_count = 0; int64_t file_end = c->content_size > 0 ? c->content_size - 1 : -1; if (c->end_off > 0) @@ -499,7 +502,7 @@ static void on_done(CurlContext *c, CURLcode code) } /* Resume seekable transfers after a recoverable error. */ - if (!aborted && c->seekable && is_recoverable(code) && + if (c->seekable && is_recoverable(code) && c->retry_count < c->max_retries) { c->retry_count++; c->loop->num_retries++; @@ -509,13 +512,12 @@ static void on_done(CurlContext *c, CURLcode code) return; } - if (!aborted) { - pthread_mutex_lock(&c->mutex); - if (!c->status) - c->status = curlcode_to_averror(code); - pthread_cond_broadcast(&c->cond); - pthread_mutex_unlock(&c->mutex); - } + /* Unhandled generic curl error */ + pthread_mutex_lock(&c->mutex); + if (!c->status) + c->status = curlcode_to_averror(code); + pthread_cond_broadcast(&c->cond); + pthread_mutex_unlock(&c->mutex); } /* ------------------------------------------------------------------------- */ -- 2.52.0 >From db0943c7f9b8ff68f3470fbc32fa49830f478ece Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 9 Jul 2026 16:19:55 +0200 Subject: [PATCH 3/6] avformat/libcurl: log all unexpected curl errors Excludes the case of !c->stream_ok (e.g. the result of an HTTP error), since in this case the actual curl error code is likely just CURLE_WRITE_ERROR. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index 334c33e432..dcd7d291fc 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -501,13 +501,16 @@ static void on_done(CurlContext *c, CURLcode code) return; } + if (c->stream_ok) + av_log(c->h, AV_LOG_WARNING, "%s\n", curl_easy_strerror(code)); + /* Resume seekable transfers after a recoverable error. */ if (c->seekable && is_recoverable(code) && c->retry_count < c->max_retries) { c->retry_count++; c->loop->num_retries++; - av_log(c->h, AV_LOG_WARNING, "%s, retrying (#%d) from %"PRId64"\n", - curl_easy_strerror(code), c->retry_count, c->request_start); + av_log(c->h, AV_LOG_WARNING, "Retrying (#%d) from %"PRId64"\n", + c->retry_count, c->request_start); start_request(c); return; } -- 2.52.0 >From 9313b14dbe53a76d038fef64c103a6e092b00e8d Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 9 Jul 2026 16:45:58 +0200 Subject: [PATCH 4/6] avformat/libcurl: log number of errors, not retries This is arguably more informative, and also easier to account for, than the retries (which are already indirectly reflected in the number of requests and connections). More importantly, this avoids getting in the way of us refactoring the retry mechanism. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index dcd7d291fc..90d4c27708 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -87,7 +87,7 @@ typedef struct CurlLoop { int num_connections; int num_redirects; int num_requests; - int num_retries; + int num_errors; } CurlLoop; struct CurlContext { @@ -307,6 +307,7 @@ static size_t header_callback(char *ptr, size_t size, size_t nitems, void *userd av_log(c->h, AV_LOG_ERROR, "Server sent back unexpected reply " "with offset %"PRId64" (expected %"PRId64")\n", content_start, c->request_start); + c->loop->num_errors++; c->stream_ok = 0; if (!c->status) c->status = AVERROR(EIO); @@ -358,6 +359,7 @@ static size_t header_callback(char *ptr, size_t size, size_t nitems, void *userd if (c->seekable_opt >= 0) c->seekable = c->seekable_opt; } else { + c->loop->num_errors++; c->stream_ok = 0; if (!c->status) c->status = ff_http_averror(status, AVERROR(EIO)); @@ -476,6 +478,7 @@ static void on_done(CurlContext *c, CURLcode code) c->stream_ok = 0; if (!c->status) c->status = curlcode_to_averror(code); + c->loop->num_errors++; pthread_cond_broadcast(&c->cond); pthread_mutex_unlock(&c->mutex); return; @@ -501,14 +504,15 @@ static void on_done(CurlContext *c, CURLcode code) return; } - if (c->stream_ok) + if (c->stream_ok) { av_log(c->h, AV_LOG_WARNING, "%s\n", curl_easy_strerror(code)); + c->loop->num_errors++; + } /* Resume seekable transfers after a recoverable error. */ if (c->seekable && is_recoverable(code) && c->retry_count < c->max_retries) { c->retry_count++; - c->loop->num_retries++; av_log(c->h, AV_LOG_WARNING, "Retrying (#%d) from %"PRId64"\n", c->retry_count, c->request_start); start_request(c); @@ -712,8 +716,8 @@ static void print_statistics(CurlLoop *loop) loop->total_bytes, time * 1e3, avg / 1e3); av_log(avfc, AV_LOG_VERBOSE, - "libcurl: %d connections, %d redirects, %d requests, %d retries\n", - loop->num_connections, loop->num_redirects, loop->num_requests, loop->num_retries); + "libcurl: %d connections, %d redirects, %d requests, %d errors\n", + loop->num_connections, loop->num_redirects, loop->num_requests, loop->num_errors); } static void curl_loop_destroy(CurlLoop *loop) -- 2.52.0 >From 98f6971ff23cd4b4d7dee41174530eb7ed94fb90 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 9 Jul 2026 19:27:52 +0200 Subject: [PATCH 5/6] avformat/libcurl: also log errors without successful data transfer As an aside, remove a redundant cast. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index 90d4c27708..a2f7e6201e 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -706,18 +706,21 @@ fail: static void print_statistics(CurlLoop *loop) { AVFormatContext *avfc = loop->avfc; - if (!loop->total_bytes) - return; - double time = (double) loop->total_time_us / 1000000.0; - double avg = time ? loop->total_bytes / time : 0; - av_log(avfc, AV_LOG_VERBOSE, - "libcurl: Overall %"PRId64" bytes received in %.0f ms = %.0f kB/s\n", - loop->total_bytes, time * 1e3, avg / 1e3); + if (loop->total_bytes) { + double time = loop->total_time_us / 1000000.0; + double avg = time ? loop->total_bytes / time : 0; + av_log(avfc, AV_LOG_VERBOSE, + "libcurl: Overall %"PRId64" bytes received in %.0f ms = %.0f kB/s\n", + loop->total_bytes, time * 1e3, avg / 1e3); + } - av_log(avfc, AV_LOG_VERBOSE, - "libcurl: %d connections, %d redirects, %d requests, %d errors\n", - loop->num_connections, loop->num_redirects, loop->num_requests, loop->num_errors); + if (loop->num_connections || loop->num_errors) { + av_log(avfc, AV_LOG_VERBOSE, + "libcurl: %d connections, %d redirects, %d requests, %d errors\n", + loop->num_connections, loop->num_redirects, loop->num_requests, + loop->num_errors); + } } static void curl_loop_destroy(CurlLoop *loop) -- 2.52.0 >From 2048588332e34b6163e4beb44ad91abd542b145a Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Thu, 3 Sep 2026 19:12:18 +0200 Subject: [PATCH 6/6] avformat/libcurl: remove libcurl: log prefix This is not really providing relevant information (the HTTP implementation chosen is already readily available from the log), and makes it seem like it's coming from libcurl itself, rather than libavformat. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/libcurl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavformat/libcurl.c b/libavformat/libcurl.c index a2f7e6201e..8c6e164317 100644 --- a/libavformat/libcurl.c +++ b/libavformat/libcurl.c @@ -711,13 +711,13 @@ static void print_statistics(CurlLoop *loop) double time = loop->total_time_us / 1000000.0; double avg = time ? loop->total_bytes / time : 0; av_log(avfc, AV_LOG_VERBOSE, - "libcurl: Overall %"PRId64" bytes received in %.0f ms = %.0f kB/s\n", + "Overall %"PRId64" bytes received in %.0f ms = %.0f kB/s\n", loop->total_bytes, time * 1e3, avg / 1e3); } if (loop->num_connections || loop->num_errors) { av_log(avfc, AV_LOG_VERBOSE, - "libcurl: %d connections, %d redirects, %d requests, %d errors\n", + "%d connections, %d redirects, %d requests, %d errors\n", loop->num_connections, loop->num_redirects, loop->num_requests, loop->num_errors); } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
