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

["Martin Storsjö" <martin@martin.st>]

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