mbox series

[FFmpeg-devel,0/6] HTTP rate limiting and retry improvements

Message ID 20240415162741.110374-1-derek.buitenhuis@gmail.com
Headers show
Series HTTP rate limiting and retry improvements | expand

Message

Derek Buitenhuis April 15, 2024, 4:27 p.m. UTC 
This patch set adds support for properly handling HTTP 429 codes,
and their rate limiting, which is widely used and is standardized.

Derek Buitenhuis (6):
  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

 libavformat/http.c    | 69 +++++++++++++++++++++++++++++++++----------
 libavformat/version.h |  2 +-
 libavutil/error.h     |  1 +
 libavutil/version.h   |  2 +-
 4 files changed, 57 insertions(+), 17 deletions(-)

Comments

James Almer April 15, 2024, 4:35 p.m. UTC | #1 
On 4/15/2024 1:27 PM, 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(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index e7603037f4..8f092f108d 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;
> +    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 = strtoull(p, NULL, 10);

Why strtoull for an int? If the value can't be negative, then make it 
unsigned and use strtoul instead.

>           }
>       }
>       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, \