[FFmpeg-devel] [PATCH v2 0/9] HTTP rate limiting and retry improvements

[FFmpeg-devel] [PATCH v2 0/9] HTTP rate limiting and retry improvements

[Derek Buitenhuis]

This patch set adds support for properly handling HTTP 429 codes,
and their rate limiting, which is widely used and is standardized.

Changes since first set:
  * Added AVERROR_HTTP_TOO_MANY_REQUESTS top error_entries in error.c, per Andreas' review.
  * Made respect_retry_after unsigned and use strtoull, per James' review.
  * Added docs, as per Stefano's reviews./
  * Added a new option to limit the total reconnect delay.
     * Unfortunate, but HTTP connection management is messy business.

Original set link: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325706.html

Derek Buitenhuis (9):
  avutil/error: Add HTTP 429 Too Many Requests AVERROR code
  avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS
  avformat/http: Don't bail on parsing headers on "bad" HTTP codes
  avformat/http: Add support for Retry-After header
  avformat/http: Rename attempts to auth_attempts
  avformat/http: Add options to set the max number of connection retries
  avformat/http: Add option to limit total reconnect delay
  doc/protocols: Re-order HTTP options to match http.c order
  doc/protocols: Fill in missing HTTP options

 doc/protocols.texi    | 139 ++++++++++++++++++++++++++----------------
 libavformat/http.c    |  77 ++++++++++++++++++-----
 libavformat/version.h |   2 +-
 libavutil/error.c     |   1 +
 libavutil/error.h     |   1 +
 libavutil/version.h   |   2 +-
 6 files changed, 152 insertions(+), 70 deletions(-)

-- 
2.43.0

_______________________________________________
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] [PATCH v2 1/9] avutil/error: Add HTTP 429 Too Many Requests AVERROR code

[Derek Buitenhuis]

This is a common error code from e.g. CDNs or cloud storage, and
it is useful to be able to handle it differently to a generic
4XX code.

Its source is RFC6585.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavutil/error.c   | 1 +
 libavutil/error.h   | 1 +
 libavutil/version.h | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavutil/error.c b/libavutil/error.c
index 938a8bc000..7a9d760e76 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -61,6 +61,7 @@ static const struct error_entry error_entries[] = {
     { ERROR_TAG(HTTP_UNAUTHORIZED),  "Server returned 401 Unauthorized (authorization failed)" },
     { ERROR_TAG(HTTP_FORBIDDEN),     "Server returned 403 Forbidden (access denied)" },
     { ERROR_TAG(HTTP_NOT_FOUND),     "Server returned 404 Not Found"           },
+    { ERROR_TAG(HTTP_TOO_MANY_REQUESTS), "Server returned 404 Too Many Requests"      },
     { ERROR_TAG(HTTP_OTHER_4XX),     "Server returned 4XX Client Error, but not one of 40{0,1,3,4}" },
     { ERROR_TAG(HTTP_SERVER_ERROR),  "Server returned 5XX Server Error reply" },
 #if !HAVE_STRERROR_R
diff --git a/libavutil/error.h b/libavutil/error.h
index 0d3269aa6d..1efa86c4c1 100644
--- a/libavutil/error.h
+++ b/libavutil/error.h
@@ -79,6 +79,7 @@
 #define AVERROR_HTTP_UNAUTHORIZED  FFERRTAG(0xF8,'4','0','1')
 #define AVERROR_HTTP_FORBIDDEN     FFERRTAG(0xF8,'4','0','3')
 #define AVERROR_HTTP_NOT_FOUND     FFERRTAG(0xF8,'4','0','4')
+#define AVERROR_HTTP_TOO_MANY_REQUESTS FFERRTAG(0xF8,'4','2','9')
 #define AVERROR_HTTP_OTHER_4XX     FFERRTAG(0xF8,'4','X','X')
 #define AVERROR_HTTP_SERVER_ERROR  FFERRTAG(0xF8,'5','X','X')
 
diff --git a/libavutil/version.h b/libavutil/version.h
index 1f2bddc022..5de2d92146 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -80,7 +80,7 @@
 
 #define LIBAVUTIL_VERSION_MAJOR  59
 #define LIBAVUTIL_VERSION_MINOR  15
-#define LIBAVUTIL_VERSION_MICRO 100
+#define LIBAVUTIL_VERSION_MICRO 101
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
                                                LIBAVUTIL_VERSION_MINOR, \
-- 
2.43.0

_______________________________________________
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] [PATCH v2 2/9] avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS

[Derek Buitenhuis]

Added in thep previous commit.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/http.c b/libavformat/http.c
index ed20359552..bbace2694f 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -286,6 +286,7 @@ static int http_should_reconnect(HTTPContext *s, int err)
     case AVERROR_HTTP_UNAUTHORIZED:
     case AVERROR_HTTP_FORBIDDEN:
     case AVERROR_HTTP_NOT_FOUND:
+    case AVERROR_HTTP_TOO_MANY_REQUESTS:
     case AVERROR_HTTP_OTHER_4XX:
         status_group = "4xx";
         break;
@@ -522,6 +523,7 @@ int ff_http_averror(int status_code, int default_averror)
         case 401: return AVERROR_HTTP_UNAUTHORIZED;
         case 403: return AVERROR_HTTP_FORBIDDEN;
         case 404: return AVERROR_HTTP_NOT_FOUND;
+        case 429: return AVERROR_HTTP_TOO_MANY_REQUESTS;
         default: break;
     }
     if (status_code >= 400 && status_code <= 499)
@@ -558,6 +560,10 @@ static int http_write_reply(URLContext* h, int status_code)
         reply_code = 404;
         reply_text = "Not Found";
         break;
+    case 429:
+        reply_code = 429;
+        reply_text = "Too Many Requests";
+        break;
     case 200:
         reply_code = 200;
         reply_text = "OK";
-- 
2.43.0

_______________________________________________
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] [PATCH v2 3/9] avformat/http: Don't bail on parsing headers on "bad" HTTP codes

[Derek Buitenhuis]

Many "bad" HTTP codes like 429 and 503 may include important info in
their headers.

Also, in general, there is no purpose in bailing here.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index bbace2694f..e7603037f4 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1086,7 +1086,7 @@ static void parse_cache_control(HTTPContext *s, const char *p)
     }
 }
 
-static int process_line(URLContext *h, char *line, int line_count)
+static int process_line(URLContext *h, char *line, int line_count, int *parsed_http_code)
 {
     HTTPContext *s = h->priv_data;
     const char *auto_method =  h->flags & AVIO_FLAG_READ ? "POST" : "GET";
@@ -1166,6 +1166,8 @@ static int process_line(URLContext *h, char *line, int line_count)
 
             av_log(h, AV_LOG_TRACE, "http_code=%d\n", s->http_code);
 
+            *parsed_http_code = 1;
+
             if ((ret = check_http_code(h, s->http_code, end)) < 0)
                 return ret;
         }
@@ -1338,7 +1340,7 @@ static int http_read_header(URLContext *h)
 {
     HTTPContext *s = h->priv_data;
     char line[MAX_URL_SIZE];
-    int err = 0;
+    int err = 0, http_err = 0;
 
     av_freep(&s->new_location);
     s->expires = 0;
@@ -1346,18 +1348,31 @@ static int http_read_header(URLContext *h)
     s->filesize_from_content_range = UINT64_MAX;
 
     for (;;) {
+        int parsed_http_code = 0;
+
         if ((err = http_get_line(s, line, sizeof(line))) < 0)
             return err;
 
         av_log(h, AV_LOG_TRACE, "header='%s'\n", line);
 
-        err = process_line(h, line, s->line_count);
-        if (err < 0)
-            return err;
+        err = process_line(h, line, s->line_count, &parsed_http_code);
+        if (err < 0) {
+            if (parsed_http_code) {
+                http_err = err;
+            } else {
+                /* Prefer to return HTTP code error if we've already seen one. */
+                if (http_err)
+                    return http_err;
+                else
+                    return err;
+            }
+        }
         if (err == 0)
             break;
         s->line_count++;
     }
+    if (http_err)
+        return http_err;
 
     // filesize from Content-Range can always be used, even if using chunked Transfer-Encoding
     if (s->filesize_from_content_range != UINT64_MAX)
-- 
2.43.0

_______________________________________________
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] [PATCH v2 4/9] avformat/http: Add support for Retry-After header

[Derek Buitenhuis]

429 and 503 codes can, and often do (e.g. all Google Cloud
Storage URLs can), return a Retry-After header with the error,
indicating how long to wait, in seconds, before retrying again.
If it is not respected by, for example, using our default backoff
stratetgy instead, chances of success are very unlikely.

This adds an AVOption to respect that header.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c    | 12 ++++++++++++
 libavformat/version.h |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index e7603037f4..5ed481b63a 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -138,6 +138,8 @@ typedef struct HTTPContext {
     char *new_location;
     AVDictionary *redirect_cache;
     uint64_t filesize_from_content_range;
+    int respect_retry_after;
+    unsigned int retry_after;
 } HTTPContext;
 
 #define OFFSET(x) offsetof(HTTPContext, x)
@@ -176,6 +178,7 @@ static const AVOption options[] = {
     { "reconnect_on_http_error", "list of http status codes to reconnect on", OFFSET(reconnect_on_http_error), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
     { "reconnect_streamed", "auto reconnect streamed / non seekable streams", OFFSET(reconnect_streamed), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
     { "reconnect_delay_max", "max reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, UINT_MAX/1000/1000, D },
+    { "respect_retry_after", "respect the Retry-After header when retrying connections", OFFSET(respect_retry_after), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, D },
     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
     { "reply_code", "The http status code to return to a client", OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, INT_MIN, 599, E},
@@ -386,6 +389,13 @@ redo:
             reconnect_delay > s->reconnect_delay_max)
             goto fail;
 
+        if (s->respect_retry_after && s->retry_after > 0) {
+            reconnect_delay = s->retry_after;
+            if (reconnect_delay > s->reconnect_delay_max)
+                goto fail;
+            s->retry_after = 0;
+        }
+
         av_log(h, AV_LOG_WARNING, "Will reconnect at %"PRIu64" in %d second(s).\n", off, reconnect_delay);
         ret = ff_network_sleep_interruptible(1000U * 1000 * reconnect_delay, &h->interrupt_callback);
         if (ret != AVERROR(ETIMEDOUT))
@@ -1231,6 +1241,8 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h
             parse_expires(s, p);
         } else if (!av_strcasecmp(tag, "Cache-Control")) {
             parse_cache_control(s, p);
+        } else if (!av_strcasecmp(tag, "Retry-After")) {
+            s->retry_after = strtoul(p, NULL, 10);
         }
     }
     return 1;
diff --git a/libavformat/version.h b/libavformat/version.h
index 7ff1483912..ee91990360 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 #include "version_major.h"
 
 #define LIBAVFORMAT_VERSION_MINOR   3
-#define LIBAVFORMAT_VERSION_MICRO 100
+#define LIBAVFORMAT_VERSION_MICRO 101
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \
-- 
2.43.0

_______________________________________________
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] [PATCH v2 5/9] avformat/http: Rename attempts to auth_attempts

[Derek Buitenhuis]

This accurately reflects what it does, as per
e75bbcf493aeb549d04c56f49406aeee3950d93b.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 5ed481b63a..6927fea2fb 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -359,7 +359,7 @@ static int http_open_cnx(URLContext *h, AVDictionary **options)
 {
     HTTPAuthType cur_auth_type, cur_proxy_auth_type;
     HTTPContext *s = h->priv_data;
-    int ret, attempts = 0, redirects = 0;
+    int ret, auth_attempts = 0, redirects = 0;
     int reconnect_delay = 0;
     uint64_t off;
     char *cached;
@@ -409,10 +409,10 @@ redo:
         goto redo;
     }
 
-    attempts++;
+    auth_attempts++;
     if (s->http_code == 401) {
         if ((cur_auth_type == HTTP_AUTH_NONE || s->auth_state.stale) &&
-            s->auth_state.auth_type != HTTP_AUTH_NONE && attempts < 4) {
+            s->auth_state.auth_type != HTTP_AUTH_NONE && auth_attempts < 4) {
             ffurl_closep(&s->hd);
             goto redo;
         } else
@@ -420,7 +420,7 @@ redo:
     }
     if (s->http_code == 407) {
         if ((cur_proxy_auth_type == HTTP_AUTH_NONE || s->proxy_auth_state.stale) &&
-            s->proxy_auth_state.auth_type != HTTP_AUTH_NONE && attempts < 4) {
+            s->proxy_auth_state.auth_type != HTTP_AUTH_NONE && auth_attempts < 4) {
             ffurl_closep(&s->hd);
             goto redo;
         } else
@@ -449,7 +449,7 @@ redo:
         /* Restart the authentication process with the new target, which
          * might use a different auth mechanism. */
         memset(&s->auth_state, 0, sizeof(s->auth_state));
-        attempts         = 0;
+        auth_attempts         = 0;
         goto redo;
     }
     return 0;
@@ -2082,7 +2082,7 @@ static int http_proxy_open(URLContext *h, const char *uri, int flags)
     char hostname[1024], hoststr[1024];
     char auth[1024], pathbuf[1024], *path;
     char lower_url[100];
-    int port, ret = 0, attempts = 0;
+    int port, ret = 0, auth_attempts = 0;
     HTTPAuthType cur_auth_type;
     char *authstr;
 
@@ -2142,10 +2142,10 @@ redo:
     if (ret < 0)
         goto fail;
 
-    attempts++;
+    auth_attempts++;
     if (s->http_code == 407 &&
         (cur_auth_type == HTTP_AUTH_NONE || s->proxy_auth_state.stale) &&
-        s->proxy_auth_state.auth_type != HTTP_AUTH_NONE && attempts < 2) {
+        s->proxy_auth_state.auth_type != HTTP_AUTH_NONE && auth_attempts < 2) {
         ffurl_closep(&s->hd);
         goto redo;
     }
-- 
2.43.0

_______________________________________________
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] [PATCH v2 6/9] avformat/http: Add options to set the max number of connection retries

[Derek Buitenhuis]

Not every use case benefits from setting retries in terms of the backoff.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c    | 12 +++++++++---
 libavformat/version.h |  2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 6927fea2fb..06bd3e340e 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -140,6 +140,7 @@ typedef struct HTTPContext {
     uint64_t filesize_from_content_range;
     int respect_retry_after;
     unsigned int retry_after;
+    int reconnect_max_retries;
 } HTTPContext;
 
 #define OFFSET(x) offsetof(HTTPContext, x)
@@ -178,6 +179,7 @@ static const AVOption options[] = {
     { "reconnect_on_http_error", "list of http status codes to reconnect on", OFFSET(reconnect_on_http_error), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
     { "reconnect_streamed", "auto reconnect streamed / non seekable streams", OFFSET(reconnect_streamed), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
     { "reconnect_delay_max", "max reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, UINT_MAX/1000/1000, D },
+    { "reconnect_max_retries", "the max number of times to retry a connection", OFFSET(reconnect_max_retries), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, D },
     { "respect_retry_after", "respect the Retry-After header when retrying connections", OFFSET(respect_retry_after), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, D },
     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
@@ -359,7 +361,7 @@ static int http_open_cnx(URLContext *h, AVDictionary **options)
 {
     HTTPAuthType cur_auth_type, cur_proxy_auth_type;
     HTTPContext *s = h->priv_data;
-    int ret, auth_attempts = 0, redirects = 0;
+    int ret, conn_attempts = 1, auth_attempts = 0, redirects = 0;
     int reconnect_delay = 0;
     uint64_t off;
     char *cached;
@@ -386,7 +388,8 @@ redo:
     ret = http_open_cnx_internal(h, options);
     if (ret < 0) {
         if (!http_should_reconnect(s, ret) ||
-            reconnect_delay > s->reconnect_delay_max)
+            reconnect_delay > s->reconnect_delay_max ||
+            (s->reconnect_max_retries >= 0 && conn_attempts > s->reconnect_max_retries))
             goto fail;
 
         if (s->respect_retry_after && s->retry_after > 0) {
@@ -401,6 +404,7 @@ redo:
         if (ret != AVERROR(ETIMEDOUT))
             goto fail;
         reconnect_delay = 1 + 2 * reconnect_delay;
+        conn_attempts++;
 
         /* restore the offset (http_connect resets it) */
         s->off = off;
@@ -1706,6 +1710,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
     int err, read_ret;
     int64_t seek_ret;
     int reconnect_delay = 0;
+    int conn_attempt = 1;
 
     if (!s->hd)
         return AVERROR_EOF;
@@ -1734,7 +1739,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
             !(s->reconnect_at_eof && read_ret == AVERROR_EOF))
             break;
 
-        if (reconnect_delay > s->reconnect_delay_max)
+        if (reconnect_delay > s->reconnect_delay_max || (s->reconnect_max_retries >= 0 && conn_attempt > s->reconnect_max_retries))
             return AVERROR(EIO);
 
         av_log(h, AV_LOG_WARNING, "Will reconnect at %"PRIu64" in %d second(s), error=%s.\n", s->off, reconnect_delay, av_err2str(read_ret));
@@ -1742,6 +1747,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
         if (err != AVERROR(ETIMEDOUT))
             return err;
         reconnect_delay = 1 + 2*reconnect_delay;
+        conn_attempt++;
         seek_ret = http_seek_internal(h, target, SEEK_SET, 1);
         if (seek_ret >= 0 && seek_ret != target) {
             av_log(h, AV_LOG_ERROR, "Failed to reconnect at %"PRIu64".\n", target);
diff --git a/libavformat/version.h b/libavformat/version.h
index ee91990360..41dbd4ad01 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 #include "version_major.h"
 
 #define LIBAVFORMAT_VERSION_MINOR   3
-#define LIBAVFORMAT_VERSION_MICRO 101
+#define LIBAVFORMAT_VERSION_MICRO 102
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \
-- 
2.43.0

_______________________________________________
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] [PATCH v2 7/9] avformat/http: Add option to limit total reconnect delay

[Derek Buitenhuis]

The existing option only allows users to set the max delay for a
single attempt, rather than the total allowed delay, which is both
pretty unintitive, and only applicable when exponential backoff is
used.

The default for this option is set to 256, which is just above the
effective total delay accomplished by the the existing
reconnect_delay_max default of 120.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/http.c    | 12 ++++++++++--
 libavformat/version.h |  2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 06bd3e340e..930c115ec3 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -141,6 +141,7 @@ typedef struct HTTPContext {
     int respect_retry_after;
     unsigned int retry_after;
     int reconnect_max_retries;
+    int reconnect_delay_total_max;
 } HTTPContext;
 
 #define OFFSET(x) offsetof(HTTPContext, x)
@@ -180,6 +181,7 @@ static const AVOption options[] = {
     { "reconnect_streamed", "auto reconnect streamed / non seekable streams", OFFSET(reconnect_streamed), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
     { "reconnect_delay_max", "max reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, UINT_MAX/1000/1000, D },
     { "reconnect_max_retries", "the max number of times to retry a connection", OFFSET(reconnect_max_retries), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, D },
+    { "reconnect_delay_total_max", "max total reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_total_max), AV_OPT_TYPE_INT, { .i64 = 256 }, 0, UINT_MAX/1000/1000, D },
     { "respect_retry_after", "respect the Retry-After header when retrying connections", OFFSET(respect_retry_after), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, D },
     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
@@ -363,6 +365,7 @@ static int http_open_cnx(URLContext *h, AVDictionary **options)
     HTTPContext *s = h->priv_data;
     int ret, conn_attempts = 1, auth_attempts = 0, redirects = 0;
     int reconnect_delay = 0;
+    int reconnect_delay_total = 0;
     uint64_t off;
     char *cached;
 
@@ -389,7 +392,8 @@ redo:
     if (ret < 0) {
         if (!http_should_reconnect(s, ret) ||
             reconnect_delay > s->reconnect_delay_max ||
-            (s->reconnect_max_retries >= 0 && conn_attempts > s->reconnect_max_retries))
+            (s->reconnect_max_retries >= 0 && conn_attempts > s->reconnect_max_retries) ||
+            reconnect_delay_total > s->reconnect_delay_total_max)
             goto fail;
 
         if (s->respect_retry_after && s->retry_after > 0) {
@@ -403,6 +407,7 @@ redo:
         ret = ff_network_sleep_interruptible(1000U * 1000 * reconnect_delay, &h->interrupt_callback);
         if (ret != AVERROR(ETIMEDOUT))
             goto fail;
+        reconnect_delay_total += reconnect_delay;
         reconnect_delay = 1 + 2 * reconnect_delay;
         conn_attempts++;
 
@@ -1710,6 +1715,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
     int err, read_ret;
     int64_t seek_ret;
     int reconnect_delay = 0;
+    int reconnect_delay_total = 0;
     int conn_attempt = 1;
 
     if (!s->hd)
@@ -1739,13 +1745,15 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
             !(s->reconnect_at_eof && read_ret == AVERROR_EOF))
             break;
 
-        if (reconnect_delay > s->reconnect_delay_max || (s->reconnect_max_retries >= 0 && conn_attempt > s->reconnect_max_retries))
+        if (reconnect_delay > s->reconnect_delay_max || (s->reconnect_max_retries >= 0 && conn_attempt > s->reconnect_max_retries) ||
+            reconnect_delay_total > s->reconnect_delay_total_max)
             return AVERROR(EIO);
 
         av_log(h, AV_LOG_WARNING, "Will reconnect at %"PRIu64" in %d second(s), error=%s.\n", s->off, reconnect_delay, av_err2str(read_ret));
         err = ff_network_sleep_interruptible(1000U*1000*reconnect_delay, &h->interrupt_callback);
         if (err != AVERROR(ETIMEDOUT))
             return err;
+        reconnect_delay_total += reconnect_delay;
         reconnect_delay = 1 + 2*reconnect_delay;
         conn_attempt++;
         seek_ret = http_seek_internal(h, target, SEEK_SET, 1);
diff --git a/libavformat/version.h b/libavformat/version.h
index 41dbd4ad01..5310326bda 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 #include "version_major.h"
 
 #define LIBAVFORMAT_VERSION_MINOR   3
-#define LIBAVFORMAT_VERSION_MICRO 102
+#define LIBAVFORMAT_VERSION_MICRO 103
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \
-- 
2.43.0

_______________________________________________
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] [PATCH v2 8/9] doc/protocols: Re-order HTTP options to match http.c order

[Derek Buitenhuis]

This makes the list easier to maintain.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 doc/protocols.texi | 112 ++++++++++++++++++++++-----------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/doc/protocols.texi b/doc/protocols.texi
index f54600b846..5ce1ddc8f4 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -442,9 +442,6 @@ value is -1.
 @item chunked_post
 If set to 1 use chunked Transfer-Encoding for posts, default is 1.
 
-@item content_type
-Set a specific content type for the POST messages or for listen mode.
-
 @item http_proxy
 set HTTP proxy to tunnel through e.g. http://example.com:1234
 
@@ -452,35 +449,21 @@ set HTTP proxy to tunnel through e.g. http://example.com:1234
 Set custom HTTP headers, can override built in default headers. The
 value must be a string encoding the headers.
 
-@item multiple_requests
-Use persistent connections if set to 1, default is 0.
-
-@item post_data
-Set custom HTTP post data.
-
-@item referer
-Set the Referer header. Include 'Referer: URL' header in HTTP request.
+@item content_type
+Set a specific content type for the POST messages or for listen mode.
 
 @item user_agent
 Override the User-Agent header. If not specified the protocol will use a
 string describing the libavformat build. ("Lavf/<version>")
 
-@item reconnect_at_eof
-If set then eof is treated like an error and causes reconnection, this is useful
-for live / endless streams.
-
-@item reconnect_streamed
-If set then even streamed/non seekable streams will be reconnected on errors.
-
-@item reconnect_on_network_error
-Reconnect automatically in case of TCP/TLS errors during connect.
+@item referer
+Set the Referer header. Include 'Referer: URL' header in HTTP request.
 
-@item reconnect_on_http_error
-A comma separated list of HTTP status codes to reconnect on. The list can
-include specific status codes (e.g. '503') or the strings '4xx' / '5xx'.
+@item multiple_requests
+Use persistent connections if set to 1, default is 0.
 
-@item reconnect_delay_max
-Sets the maximum delay in seconds after which to give up reconnecting
+@item post_data
+Set custom HTTP post data.
 
 @item mime_type
 Export the MIME type.
@@ -488,6 +471,11 @@ Export the MIME type.
 @item http_version
 Exports the HTTP response version number. Usually "1.0" or "1.1".
 
+@item cookies
+Set the cookies to be sent in future requests. The format of each cookie is the
+same as the value of a Set-Cookie HTTP response field. Multiple cookies can be
+delimited by a newline character.
+
 @item icy
 If set to 1 request ICY (SHOUTcast) metadata from the server. If the server
 supports this, the metadata has to be retrieved by the application by reading
@@ -504,10 +492,32 @@ contains the last non-empty metadata packet sent by the server. It should be
 polled in regular intervals by applications interested in mid-stream metadata
 updates.
 
-@item cookies
-Set the cookies to be sent in future requests. The format of each cookie is the
-same as the value of a Set-Cookie HTTP response field. Multiple cookies can be
-delimited by a newline character.
+@item auth_type
+
+Set HTTP authentication type. No option for Digest, since this method requires
+getting nonce parameters from the server first and can't be used straight away like
+Basic.
+
+@table @option
+@item none
+Choose the HTTP authentication type automatically. This is the default.
+@item basic
+
+Choose the HTTP basic authentication.
+
+Basic authentication sends a Base64-encoded string that contains a user name and password
+for the client. Base64 is not a form of encryption and should be considered the same as
+sending the user name and password in clear text (Base64 is a reversible encoding).
+If a resource needs to be protected, strongly consider using an authentication scheme
+other than basic authentication. HTTPS/TLS should be used with basic authentication.
+Without these additional security enhancements, basic authentication should not be used
+to protect sensitive or valuable information.
+@end table
+
+@item send_expect_100
+Send an Expect: 100-continue header for POST. If set to 1 it will send, if set
+to 0 it won't, if set to -1 it will try to send if it is applicable. Default
+value is -1.
 
 @item offset
 Set initial byte offset.
@@ -525,6 +535,23 @@ be given a Bad Request response.
 When unset the HTTP method is not checked for now. This will be replaced by
 autodetection in the future.
 
+@item reconnect_at_eof
+If set then eof is treated like an error and causes reconnection, this is useful
+for live / endless streams.
+
+@item reconnect_on_network_error
+Reconnect automatically in case of TCP/TLS errors during connect.
+
+@item reconnect_on_http_error
+A comma separated list of HTTP status codes to reconnect on. The list can
+include specific status codes (e.g. '503') or the strings '4xx' / '5xx'.
+
+@item reconnect_streamed
+If set then even streamed/non seekable streams will be reconnected on errors.
+
+@item reconnect_delay_max
+Sets the maximum delay in seconds after which to give up reconnecting
+
 @item listen
 If set to 1 enables experimental HTTP server. This can be used to send data when
 used as an output option, or read data from a client with HTTP POST when used as
@@ -551,33 +578,6 @@ ffmpeg -i somefile.ogg -chunked_post 0 -c copy -f ogg http://@var{server}:@var{p
 wget --post-file=somefile.ogg http://@var{server}:@var{port}
 @end example
 
-@item send_expect_100
-Send an Expect: 100-continue header for POST. If set to 1 it will send, if set
-to 0 it won't, if set to -1 it will try to send if it is applicable. Default
-value is -1.
-
-@item auth_type
-
-Set HTTP authentication type. No option for Digest, since this method requires
-getting nonce parameters from the server first and can't be used straight away like
-Basic.
-
-@table @option
-@item none
-Choose the HTTP authentication type automatically. This is the default.
-@item basic
-
-Choose the HTTP basic authentication.
-
-Basic authentication sends a Base64-encoded string that contains a user name and password
-for the client. Base64 is not a form of encryption and should be considered the same as
-sending the user name and password in clear text (Base64 is a reversible encoding).
-If a resource needs to be protected, strongly consider using an authentication scheme
-other than basic authentication. HTTPS/TLS should be used with basic authentication.
-Without these additional security enhancements, basic authentication should not be used
-to protect sensitive or valuable information.
-@end table
-
 @end table
 
 @subsection HTTP Cookies
-- 
2.43.0

_______________________________________________
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] [PATCH v2 9/9] doc/protocols: Fill in missing HTTP options

[Derek Buitenhuis]

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 doc/protocols.texi | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/doc/protocols.texi b/doc/protocols.texi
index 5ce1ddc8f4..ed70af4b33 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -492,6 +492,10 @@ contains the last non-empty metadata packet sent by the server. It should be
 polled in regular intervals by applications interested in mid-stream metadata
 updates.
 
+@item metadata
+Set an exported dictionary containing Icecast metadata from the bitstream, if present.
+Only useful with the C API.
+
 @item auth_type
 
 Set HTTP authentication type. No option for Digest, since this method requires
@@ -519,6 +523,10 @@ Send an Expect: 100-continue header for POST. If set to 1 it will send, if set
 to 0 it won't, if set to -1 it will try to send if it is applicable. Default
 value is -1.
 
+@item location
+An exported dictionary containing the content location. Only useful with the C
+API.
+
 @item offset
 Set initial byte offset.
 
@@ -535,6 +543,9 @@ be given a Bad Request response.
 When unset the HTTP method is not checked for now. This will be replaced by
 autodetection in the future.
 
+@item reconnect
+Reconnect automatically when disconnected before EOF is hit.
+
 @item reconnect_at_eof
 If set then eof is treated like an error and causes reconnection, this is useful
 for live / endless streams.
@@ -550,7 +561,18 @@ include specific status codes (e.g. '503') or the strings '4xx' / '5xx'.
 If set then even streamed/non seekable streams will be reconnected on errors.
 
 @item reconnect_delay_max
-Sets the maximum delay in seconds after which to give up reconnecting
+Set the maximum delay in seconds after which to give up reconnecting.
+
+@item reconnect_max_retries
+Set the maximum number of times to retry a connection. Default unset.
+
+@item reconnect_delay_total_max
+Set the maximum total delay in seconds after which to give up reconnecting.
+
+@item respect_retry_after
+If enabled, and a Retry-After header is encountered, its requested reconnection
+delay will be honored, rather than using exponential backoff. Useful for 429 and
+503 errors. Default enabled.
 
 @item listen
 If set to 1 enables experimental HTTP server. This can be used to send data when
@@ -578,6 +600,17 @@ ffmpeg -i somefile.ogg -chunked_post 0 -c copy -f ogg http://@var{server}:@var{p
 wget --post-file=somefile.ogg http://@var{server}:@var{port}
 @end example
 
+@item resource
+The resource requested by a client, when the experimental HTTP server is in use.
+
+@item reply_code
+The HTTP code returned to the client, when the experimental HTTP server is in use.
+
+@item short_seek_size
+Set the threshold, in bytes, for when a readahead should be prefered over a seek and
+new HTTP request. This is useful, for example, to make sure the same connection
+is used for reading large video packets with small audio packets in between.
+
 @end table
 
 @subsection HTTP Cookies
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/9] avutil/error: Add HTTP 429 Too Many Requests AVERROR code

[Derek Buitenhuis]

On 4/22/2024 3:25 PM, Derek Buitenhuis wrote:
> +    { ERROR_TAG(HTTP_TOO_MANY_REQUESTS), "Server returned 404 Too Many Requests"      },

Derp.

Change locally to "Server returned 429 Too Many Requests".

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/9] avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS

[Martin Storsjö]

On Mon, 22 Apr 2024, Derek Buitenhuis wrote:

> Added in thep previous commit.

Typo in the commit message

// Martin

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/9] avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS

[Martin Storsjö]

On Mon, 22 Apr 2024, Derek Buitenhuis wrote:

> Added in thep previous commit.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> libavformat/http.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index ed20359552..bbace2694f 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -286,6 +286,7 @@ static int http_should_reconnect(HTTPContext *s, int err)
>     case AVERROR_HTTP_UNAUTHORIZED:
>     case AVERROR_HTTP_FORBIDDEN:
>     case AVERROR_HTTP_NOT_FOUND:
> +    case AVERROR_HTTP_TOO_MANY_REQUESTS:
>     case AVERROR_HTTP_OTHER_4XX:
>         status_group = "4xx";
>         break;
> @@ -522,6 +523,7 @@ int ff_http_averror(int status_code, int default_averror)
>         case 401: return AVERROR_HTTP_UNAUTHORIZED;
>         case 403: return AVERROR_HTTP_FORBIDDEN;
>         case 404: return AVERROR_HTTP_NOT_FOUND;
> +        case 429: return AVERROR_HTTP_TOO_MANY_REQUESTS;
>         default: break;
>     }
>     if (status_code >= 400 && status_code <= 499)
> @@ -558,6 +560,10 @@ static int http_write_reply(URLContext* h, int status_code)
>         reply_code = 404;
>         reply_text = "Not Found";
>         break;
> +    case 429:
> +        reply_code = 429;
> +        reply_text = "Too Many Requests";
> +        break;
>     case 200:

This function seems to handle both the literal status codes, like 429, and 
also AVERROR style error codes, as when called from handle_http_errors, so 
perhaps it would be good for consistency to add the AVERROR here too.

// Martin

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 4/9] avformat/http: Add support for Retry-After header

[Martin Storsjö]

On Mon, 22 Apr 2024, Derek Buitenhuis wrote:

> 429 and 503 codes can, and often do (e.g. all Google Cloud
> Storage URLs can), return a Retry-After header with the error,
> indicating how long to wait, in seconds, before retrying again.
> If it is not respected by, for example, using our default backoff
> stratetgy instead, chances of success are very unlikely.
>
> This adds an AVOption to respect that header.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> libavformat/http.c    | 12 ++++++++++++
> libavformat/version.h |  2 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)

Is this feature standardized in a RFC, or is it some other spec somewhere? 
I think it would be nice with a link to a spec in the commit message here.

>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index e7603037f4..5ed481b63a 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -138,6 +138,8 @@ typedef struct HTTPContext {
>     char *new_location;
>     AVDictionary *redirect_cache;
>     uint64_t filesize_from_content_range;
> +    int respect_retry_after;
> +    unsigned int retry_after;
> } HTTPContext;
>
> #define OFFSET(x) offsetof(HTTPContext, x)
> @@ -176,6 +178,7 @@ static const AVOption options[] = {
>     { "reconnect_on_http_error", "list of http status codes to reconnect on", OFFSET(reconnect_on_http_error), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
>     { "reconnect_streamed", "auto reconnect streamed / non seekable streams", OFFSET(reconnect_streamed), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
>     { "reconnect_delay_max", "max reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, UINT_MAX/1000/1000, D },
> +    { "respect_retry_after", "respect the Retry-After header when retrying connections", OFFSET(respect_retry_after), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, D },
>     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
>     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
>     { "reply_code", "The http status code to return to a client", OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, INT_MIN, 599, E},
> @@ -386,6 +389,13 @@ redo:
>             reconnect_delay > s->reconnect_delay_max)
>             goto fail;
>
> +        if (s->respect_retry_after && s->retry_after > 0) {
> +            reconnect_delay = s->retry_after;

It'd be nice with a comment to clarify the units of both values here, 
which apparently both happen to be integer seconds?

> +            if (reconnect_delay > s->reconnect_delay_max)
> +                goto fail;
> +            s->retry_after = 0;
> +        }
> +
>         av_log(h, AV_LOG_WARNING, "Will reconnect at %"PRIu64" in %d second(s).\n", off, reconnect_delay);
>         ret = ff_network_sleep_interruptible(1000U * 1000 * reconnect_delay, &h->interrupt_callback);
>         if (ret != AVERROR(ETIMEDOUT))
> @@ -1231,6 +1241,8 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h
>             parse_expires(s, p);
>         } else if (!av_strcasecmp(tag, "Cache-Control")) {
>             parse_cache_control(s, p);
> +        } else if (!av_strcasecmp(tag, "Retry-After")) {
> +            s->retry_after = strtoul(p, NULL, 10);

Can you add a comment here, to clarify what unit the value is expressed 
in?

// Martin

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 6/9] avformat/http: Add options to set the max number of connection retries

[Martin Storsjö]

On Mon, 22 Apr 2024, Derek Buitenhuis wrote:

> Not every use case benefits from setting retries in terms of the backoff.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> libavformat/http.c    | 12 +++++++++---
> libavformat/version.h |  2 +-
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 6927fea2fb..06bd3e340e 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -140,6 +140,7 @@ typedef struct HTTPContext {
>     uint64_t filesize_from_content_range;
>     int respect_retry_after;
>     unsigned int retry_after;
> +    int reconnect_max_retries;
> } HTTPContext;
>
> #define OFFSET(x) offsetof(HTTPContext, x)
> @@ -178,6 +179,7 @@ static const AVOption options[] = {
>     { "reconnect_on_http_error", "list of http status codes to reconnect on", OFFSET(reconnect_on_http_error), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
>     { "reconnect_streamed", "auto reconnect streamed / non seekable streams", OFFSET(reconnect_streamed), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
>     { "reconnect_delay_max", "max reconnect delay in seconds after which to give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, UINT_MAX/1000/1000, D },
> +    { "reconnect_max_retries", "the max number of times to retry a connection", OFFSET(reconnect_max_retries), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, D },
>     { "respect_retry_after", "respect the Retry-After header when retrying connections", OFFSET(respect_retry_after), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, D },
>     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
>     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
> @@ -359,7 +361,7 @@ static int http_open_cnx(URLContext *h, AVDictionary **options)
> {
>     HTTPAuthType cur_auth_type, cur_proxy_auth_type;
>     HTTPContext *s = h->priv_data;
> -    int ret, auth_attempts = 0, redirects = 0;
> +    int ret, conn_attempts = 1, auth_attempts = 0, redirects = 0;
>     int reconnect_delay = 0;
>     uint64_t off;
>     char *cached;
> @@ -386,7 +388,8 @@ redo:
>     ret = http_open_cnx_internal(h, options);
>     if (ret < 0) {
>         if (!http_should_reconnect(s, ret) ||
> -            reconnect_delay > s->reconnect_delay_max)
> +            reconnect_delay > s->reconnect_delay_max ||
> +            (s->reconnect_max_retries >= 0 && conn_attempts > s->reconnect_max_retries))
>             goto fail;
>
>         if (s->respect_retry_after && s->retry_after > 0) {
> @@ -401,6 +404,7 @@ redo:
>         if (ret != AVERROR(ETIMEDOUT))
>             goto fail;
>         reconnect_delay = 1 + 2 * reconnect_delay;
> +        conn_attempts++;
>
>         /* restore the offset (http_connect resets it) */
>         s->off = off;
> @@ -1706,6 +1710,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size)
>     int err, read_ret;
>     int64_t seek_ret;
>     int reconnect_delay = 0;
> +    int conn_attempt = 1;

Minor inconsistency; the corresponding variable in the other function was 
called conn_attempts, as a plural.

// Martin

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 0/9] HTTP rate limiting and retry improvements

[Martin Storsjö]

On Mon, 22 Apr 2024, Derek Buitenhuis wrote:

> This patch set adds support for properly handling HTTP 429 codes,
> and their rate limiting, which is widely used and is standardized.
>
> Changes since first set:
>  * Added AVERROR_HTTP_TOO_MANY_REQUESTS top error_entries in error.c, per Andreas' review.
>  * Made respect_retry_after unsigned and use strtoull, per James' review.
>  * Added docs, as per Stefano's reviews./
>  * Added a new option to limit the total reconnect delay.
>     * Unfortunate, but HTTP connection management is messy business.

I had a look over this patchset, and I had a handful of minor comments, 
but overall, the patchset seems fine to me. Thanks!

// Martin

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 4/9] avformat/http: Add support for Retry-After header

[Derek Buitenhuis]

On 4/24/2024 12:06 PM, Martin Storsjö wrote:
> Is this feature standardized in a RFC, or is it some other spec somewhere? 
> I think it would be nice with a link to a spec in the commit message here.

It is in the RFC for 429 I noted in the commit I added that: RFC6585. It is also
probably in the 503 and 301 RFCs.

See also: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After

I will add those.

>> +        if (s->respect_retry_after && s->retry_after > 0) {
>> +            reconnect_delay = s->retry_after;
> 
> It'd be nice with a comment to clarify the units of both values here, 
> which apparently both happen to be integer seconds?

Yes, seconds.

I have added:
    /* Both the Retry-After header and the option are in seconds. */


>> +        } else if (!av_strcasecmp(tag, "Retry-After")) {
>> +            s->retry_after = strtoul(p, NULL, 10);
> 
> Can you add a comment here, to clarify what unit the value is expressed 
> in?

Added:
    /* Specifies how long to wait before retrying in second. */

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/9] avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS

[Derek Buitenhuis]

On 4/24/2024 11:53 AM, Martin Storsjö wrote:
> Typo in the commit message

Fixed locally.

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/9] avformat/http: Use AVERROR_HTTP_TOO_MANY_REQUESTS

[Derek Buitenhuis]

On 4/24/2024 11:58 AM, Martin Storsjö wrote:
> This function seems to handle both the literal status codes, like 429, and 
> also AVERROR style error codes, as when called from handle_http_errors, so 
> perhaps it would be good for consistency to add the AVERROR here too.

Good catch. Added.

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 6/9] avformat/http: Add options to set the max number of connection retries

[Derek Buitenhuis]

On 4/24/2024 12:08 PM, Martin Storsjö wrote:
> Minor inconsistency; the corresponding variable in the other function was 
> called conn_attempts, as a plural.

Renamed to the plural version.

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 0/9] HTTP rate limiting and retry improvements

[Derek Buitenhuis]

On 4/24/2024 12:13 PM, Martin Storsjö wrote:
> I had a look over this patchset, and I had a handful of minor comments, 
> but overall, the patchset seems fine to me. Thanks!

Applied all your comments.

Will wait a day and then push if no others appear.

- Derek
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 0/9] HTTP rate limiting and retry improvements

[Derek Buitenhuis]

On 4/24/2024 8:43 PM, Derek Buitenhuis wrote:
> Applied all your comments.
> 
> Will wait a day and then push if no others appear.

Pushed all except Retry-After support, which I need to change.

While adding RFC references I noticed it can be in seconds, *or*
a date... fun.

Sending an updated patch for that in a bit in a new thread.

- Derek
_______________________________________________
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".