On Fri, 24 Jul 2015, Luca Barbato wrote:

Make possible to send the custom headers and override the user agent.

Reported-by: BenWonder
---
libavformat/hls.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 83 insertions(+), 7 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 8b52a35..6109602 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -103,6 +103,8 @@ typedef struct HLSContext {
    int64_t seek_timestamp;
    int seek_flags;
    AVIOInterruptCB *interrupt_callback;
+    uint8_t *headers;
+    uint8_t *user_agent;
} HLSContext;

I'd say it'd make more sense to store these as char*; an uint8_t* without an associated length is useless. (Yes, I know that av_opt_get returns an uint8_t*, but that's basically wrong since it always returns a null-terminated string.)


static int read_chomp_line(AVIOContext *s, char *buf, int maxlen)
@@ -199,6 +201,29 @@ static void handle_key_args(struct key_info *info, const 
char *key,
    }
}

+static int open_in(HLSContext *c, AVIOContext **in, const char *url)
+{
+    AVDictionary *opts = NULL;
+    int ret;
+
+    if (c->headers) {
+        ret = av_dict_set(&opts, "headers", c->headers, 0);
+        if (ret < 0)
+            return ret;
+    }
+    if (c->user_agent) {
+        ret = av_dict_set(&opts, "user_agent", c->user_agent, 0);
+        if (ret < 0)
+            goto fail;
+    }
+
+    ret = avio_open2(in, url, AVIO_FLAG_READ, c->interrupt_callback, &opts);
+
+fail:
+    av_dict_free(&opts);
+    return ret;
+}

This kinda looks clumsy, but probably is ok. In principle one might want to store a dict of options instead of handpicking them, but that might be overkill for now.

@@ -214,10 +239,10 @@ static int parse_playlist(HLSContext *c, const char *url,
    uint8_t *new_url = NULL;

    if (!in) {
-        close_in = 1;
-        if ((ret = avio_open2(&in, url, AVIO_FLAG_READ,
-                              c->interrupt_callback, NULL)) < 0)
+        ret = open_in(c, &in, url);
+        if (ret < 0)
            return ret;
+        close_in = 1;
    }

    if (av_opt_get(in, "location", AV_OPT_SEARCH_CHILDREN, &new_url) >= 0)
@@ -329,19 +354,41 @@ fail:
    return ret;
}

+static int open_url(HLSContext *c, URLContext **uc, const char *url)
+{
+    AVDictionary *opts = NULL;
+    int ret;
+
+    if (c->headers) {
+        ret = av_dict_set(&opts, "headers", c->headers, 0);
+        if (ret < 0)
+            return ret;
+    }
+    if (c->user_agent) {
+        ret = av_dict_set(&opts, "user_agent", c->user_agent, 0);
+        if (ret < 0)
+            goto fail;
+    }

Please factorize setting the options, we shouldn't duplicate such things at least, it's already ugly enough.

+    ret = ffurl_open(uc, url, AVIO_FLAG_READ, c->interrupt_callback, &opts);
+
+fail:
+    av_dict_free(&opts);
+
+    return ret;
+}
+
static int open_input(struct variant *var)
{
    struct segment *seg = var->segments[var->cur_seq_no - var->start_seq_no];
    if (seg->key_type == KEY_NONE) {
-        return ffurl_open(&var->input, seg->url, AVIO_FLAG_READ,
-                          &var->parent->interrupt_callback, NULL);
+        return  open_url(var->parent->priv_data, &var->input, seg->url);

Stray double space

    } else if (seg->key_type == KEY_AES_128) {
+        HLSContext *c = var->parent->priv_data;
        char iv[33], key[33], url[MAX_URL_SIZE];
        int ret;
        if (strcmp(seg->key, var->key_url)) {
            URLContext *uc;
-            if (ffurl_open(&uc, seg->key, AVIO_FLAG_READ,
-                           &var->parent->interrupt_callback, NULL) == 0) {
+            if (open_url(var->parent->priv_data, &uc, seg->key) == 0) {
                if (ffurl_read_complete(uc, var->key, sizeof(var->key))
                    != sizeof(var->key)) {
                    av_log(NULL, AV_LOG_ERROR, "Unable to read key file %s\n",
@@ -366,6 +413,10 @@ static int open_input(struct variant *var)
            return ret;
        av_opt_set(var->input->priv_data, "key", key, 0);
        av_opt_set(var->input->priv_data, "iv", iv, 0);
+        if (c->headers)
+            av_opt_set(var->input->priv_data, "headers", c->headers, 0);
+        if (c->user_agent)
+            av_opt_set(var->input->priv_data, "user_agent", c->user_agent, 0);

Should be factorized. Yes, this is av_opt_set instead of av_dict_set, but at least move it to some function close to the rest, so that it's easier to find all the occurrance of headers/user_agent/foo in one place, instead of having them sprinkled all over the place.

        if ((ret = ffurl_connect(var->input, NULL)) < 0) {
            ffurl_close(var->input);
            var->input = NULL;
@@ -449,6 +500,24 @@ reload:
    goto restart;
}

+static int save_avio_options(AVFormatContext *s)
+{
+    HLSContext *c = s->priv_data;
+    int ret = 0;
+
+    ret = av_opt_get(s->pb, "headers",
+                     AV_OPT_SEARCH_CHILDREN, &c->headers);
+    if (ret < 0)
+        return ret;
+
+    ret = av_opt_get(s->pb, "user_agent",
+                     AV_OPT_SEARCH_CHILDREN, &c->user_agent);
+    if (ret < 0)
+        return ret;
+
+    return ret;
+}
+
static int hls_read_header(AVFormatContext *s)
{
    HLSContext *c = s->priv_data;
@@ -459,6 +528,9 @@ static int hls_read_header(AVFormatContext *s)
    if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0)
        goto fail;

+    if ((ret = save_avio_options(s)) < 0)
+        goto fail;
+

Umm, no.

What happens if one of the options isn't found? av_opt_get will return an error, and the HLS demuxer will refuse to work, even though things are just fine. Keep in mind that the HLS demuxer works with any input protocol, not only http. (E.g. you can play back an m3u8 and associated ts files over file:// just fine.)

Ideally, save_avio_options should continue even if one of the options isn't found. Consider if the AVIOContext is some other protocol that e.g. does have the field "headers" but not "user_agent" (or vice versa), then it still makes sense to continue, and store the fields that exist, even if not all of them do.

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to