HLS (EXT-X-BYTERANGE) and persistent connection how Range header is replaced ?

[e2iplayer]

Summary of the bug:
During analysis of previous reported by me bug #9070

I found that HLS segments in (EXT-X-BYTERANGE) format are handled wrongly when persistent connection is enabled.

To be able to correctly set Range header in http.c there is need to pass options with:

av_dict_set_int(&opts, "offset", seg->url_offset, 0);
av_dict_set_int(&opts, "end_offset", seg->url_offset + seg->size, 0);

But as you can see in hls.c if connection is re-used the "offset" and "end_offset" are not replaced by new values.

What causes that wrong Range header will be send.

In ff_http_do_new_request only s->off will be set to 0, so if connection will be reused second segment will be requested with Range header == "bytes=0-valueFromPrevRequest"

Is there something I don't understand?

[Steven Liu]

Looks you have known how to fix it, can you send a patch to maillist please?

[e2iplayer]

Unfortunately not. I tried to made some investigation but I don't understand how options are propagated between contexts.

Please take a look:

diff -x '*.[^ch]' -uNr ffmpeg2/libavformat/hls.c ffmpeg/libavformat/hls.c
--- ffmpeg-4.2.2_http/ffmpeg-4.2.2/libavformat/hls.c	2021-01-19 10:18:44.494479917 +0100
+++ ffmpeg-4.2.2/libavformat/hls.c	2021-01-20 13:01:54.113543376 +0100
@@ -598,8 +598,10 @@
     return 0;
 }
 
+int ff_http_do_new_request2(URLContext *h, const char *uri, AVDictionary *opts);
+
 static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
-                              const char *url)
+                              const char *url, AVDictionary *opts)
 {
 #if !CONFIG_HTTP_PROTOCOL
     return AVERROR_PROTOCOL_NOT_FOUND;
@@ -608,7 +610,7 @@
     URLContext *uc = ffio_geturlcontext(*pb);
     av_assert0(uc);
     (*pb)->eof_reached = 0;
-    ret = ff_http_do_new_request(uc, url);
+    ret = ff_http_do_new_request2(uc, url, opts);
     if (ret < 0) {
         ff_format_io_close(s, pb);
     }
@@ -661,7 +663,7 @@
         return AVERROR_INVALIDDATA;
 
     if (is_http && c->http_persistent && *pb) {
-        ret = open_url_keepalive(c->ctx, pb, url);
+        ret = open_url_keepalive(c->ctx, pb, url, tmp);
         if (ret == AVERROR_EXIT) {
             return ret;
         } else if (ret < 0) {
@@ -718,7 +720,10 @@
 
     if (is_http && !in && c->http_persistent && c->playlist_pb) {
         in = c->playlist_pb;
-        ret = open_url_keepalive(c->ctx, &c->playlist_pb, url);
+        AVDictionary *opts = NULL;
+        av_dict_copy(&opts, c->avio_opts, 0);
+        ret = open_url_keepalive(c->ctx, &c->playlist_pb, url, opts);
+        av_dict_free(&opts);
         if (ret == AVERROR_EXIT) {
             return ret;
         } else if (ret < 0) {
diff -x '*.[^ch]' -uNr ffmpeg2/libavformat/http.c ffmpeg/libavformat/http.c
--- ffmpeg-4.2.2_http/ffmpeg-4.2.2/libavformat/http.c	2021-01-19 10:20:05.614484136 +0100
+++ ffmpeg-4.2.2/libavformat/http.c	2021-01-20 14:07:25.875704582 +0100
@@ -242,6 +242,7 @@
 
     err = http_connect(h, path, local_path, hoststr,
                        auth, proxyauth, &location_changed);
+
     if (err < 0)
         return err;
 
@@ -304,8 +305,9 @@
         return location_changed;
     return ff_http_averror(s->http_code, AVERROR(EIO));
 }
+int ff_http_do_new_request2(URLContext *h, const char *uri, AVDictionary *opts);
 
-int ff_http_do_new_request(URLContext *h, const char *uri)
+int ff_http_do_new_request2(URLContext *h, const char *uri, AVDictionary *opts)
 {
     HTTPContext *s = h->priv_data;
     AVDictionary *options = NULL;
@@ -350,12 +352,32 @@
     if (!s->location)
         return AVERROR(ENOMEM);
 

     av_log(s, AV_LOG_INFO, "Opening \'%s\' for %s\n", uri, h->flags & AVIO_FLAG_WRITE ? "writing" : "reading");
+    if (opts) {
+        AVDictionaryEntry *t = NULL;
+        while ((t = av_dict_get(opts, "", t, AV_DICT_IGNORE_SUFFIX))) {
+            if (!strcmp(t->key, "offset")) {
+                s->off = strtoull(t->value, NULL, 10);
+            }
+            if (!strcmp(t->key, "end_offset")) {
+                s->end_off = strtoull(t->value, NULL, 10);
+            }
+        }
+        av_dict_copy(&options, opts, 0);
+    }
     ret = http_open_cnx(h, &options);
     av_dict_free(&options);
     return ret;
 }
 
+int ff_http_do_new_request(URLContext *h, const char *uri)
+{
+    return ff_http_do_new_request2(h, uri, NULL);
+}
+

I do not undestand why this:

+        AVDictionaryEntry *t = NULL;
+        while ((t = av_dict_get(opts, "", t, AV_DICT_IGNORE_SUFFIX))) {
+            if (!strcmp(t->key, "offset")) {
+                s->off = strtoull(t->value, NULL, 10);
+            }
+            if (!strcmp(t->key, "end_offset")) {
+                s->end_off = strtoull(t->value, NULL, 10);
+            }
+        }

is needed? Why this is not automaticly applied to URLContext in the function
ret = http_open_cnx(h, &options);

do you have any idea?