diff mbox

[FFmpeg-devel] http connect: retry five times before reporting about error

Message ID 20180916154154.5526-1-akawolf0@gmail.com
State New
Headers show

Commit Message

Artjom Vejsel Sept. 16, 2018, 3:41 p.m. UTC
Signed-off-by: Artjom Vejsel <akawolf0@gmail.com>
---

Hello.

I've faced an error while playing when downloading just stopped. It can be even few times at one film.
After digging into source I've finished with knowledge that HTTP connection made only once and if it was not successfull, HTTP interface just report
with an error.
I tried to change this behaviour to try few times and that solve my problem.

 libavformat/http.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Carl Eugen Hoyos Sept. 16, 2018, 5:57 p.m. UTC | #1
2018-09-16 17:41 GMT+02:00, Artjom Vejsel <akawolf0@gmail.com>:
> Signed-off-by: Artjom Vejsel <akawolf0@gmail.com>
> ---
>
> Hello.
>
> I've faced an error while playing when downloading just stopped. It can be
> even few times at one film.
> After digging into source I've finished with knowledge that HTTP connection
> made only once and if it was not successfull, HTTP interface just report
> with an error.
> I tried to change this behaviour to try few times and that solve my problem.
>
>  libavformat/http.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 4fdb2f13f2..9afa885423 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -259,11 +259,19 @@ redo:
>      cur_auth_type       = s->auth_state.auth_type;
>      cur_proxy_auth_type = s->auth_state.auth_type;
>
> +    attempts++;

Maybe this is just me but I believe the code gets clearer
if you move this into the if() below.

> +
>      location_changed = http_open_cnx_internal(h, options);
> -    if (location_changed < 0)
> -        goto fail;
> +    if (location_changed < 0) {
> +        if (attempts < 5) {

> +            av_log(h, AV_LOG_ERROR, "Couldn't get HTTP resource,
> retrying...\n");

Should be a warning imo.

> +            if (s->hd)
> +                ffurl_closep(&s->hd);
> +            goto redo;
> +        } else
> +            goto fail;
> +    }
>
> -    attempts++;

Carl Eugen
Derek Buitenhuis Sept. 18, 2018, 1:44 p.m. UTC | #2
On 16/09/2018 23:01, Artjom Vejsel wrote:
> Signed-off-by: Artjom Vejsel<akawolf0@gmail.com>
> ---
>   libavformat/http.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)

This should be an AVOption instead of a hardcoded number.

- Derek
Artjom Vejsel Sept. 19, 2018, 10:26 a.m. UTC | #3
> This should be an AVOption instead of a hardcoded number.

I didn't change mechanism of attempts: there was attempts variable for auth with magical max attempts constant 3, I just add open attempts with the same 3 max attempts...

Regards, Artjom.
diff mbox

Patch

diff --git a/libavformat/http.c b/libavformat/http.c
index 4fdb2f13f2..9afa885423 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -259,11 +259,19 @@  redo:
     cur_auth_type       = s->auth_state.auth_type;
     cur_proxy_auth_type = s->auth_state.auth_type;
 
+    attempts++;
+
     location_changed = http_open_cnx_internal(h, options);
-    if (location_changed < 0)
-        goto fail;
+    if (location_changed < 0) {
+        if (attempts < 5) {
+            av_log(h, AV_LOG_ERROR, "Couldn't get HTTP resource, retrying...\n");
+            if (s->hd)
+                ffurl_closep(&s->hd);
+            goto redo;
+        } else
+            goto fail;
+    }
 
-    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) {