[FFmpeg-devel] [PATCH] HLS fixes: better relative path handling, larger buffer for long URIs, only send range header when necessary

[Duncan Salerno]

On Sun, Sep 23, 2012 at 2:24 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Sun, Sep 23, 2012 at 01:44:33PM +0200, Michael Niedermayer wrote:
>> On Sun, Sep 23, 2012 at 10:50:05AM +0200, Reimar Döffinger wrote:
>> > On 23 Sep 2012, at 02:51, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > > Hi
>> > >
>> > > On Sat, Sep 22, 2012 at 09:17:36PM +0100, Duncan Salerno wrote:
>> > >> ---
>> > >> libavformat/http.c  |    6 +++---
>> > >> libavformat/utils.c |   27 +++++++++++++++++++++++----
>> > >> 2 files changed, 26 insertions(+), 7 deletions(-)
>> > >>
>> > >> diff --git a/libavformat/http.c b/libavformat/http.c
>> > >> index 376ff9e..669fdf4 100644
>> > >> --- a/libavformat/http.c
>> > >> +++ b/libavformat/http.c
>> > >> @@ -33,7 +33,7 @@
>> > >>    only a subset of it. */
>> > >>
>> > >> /* used for protocol handling */
>> > >> -#define BUFFER_SIZE 1024
>> > >> +#define BUFFER_SIZE 4096
>> > >> #define MAX_REDIRECTS 8
>> > >>
>> > >> typedef struct {
>> > >> @@ -380,7 +380,7 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>> > >> {
>> > >>     HTTPContext *s = h->priv_data;
>> > >>     int post, err;
>> > >> -    char headers[1024] = "";
>> > >> +    char headers[4096] = "";
>> > >>     char *authstr = NULL, *proxyauthstr = NULL;
>> > >>     int64_t off = s->off;
>> > >>     int len = 0;
>> > >> @@ -411,7 +411,7 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>> > >>     if (!has_header(s->headers, "\r\nAccept: "))
>> > >>         len += av_strlcpy(headers + len, "Accept: */*\r\n",
>> > >>                           sizeof(headers) - len);
>> > >> -    if (!has_header(s->headers, "\r\nRange: ") && !post)
>> > >> +    if (!has_header(s->headers, "\r\nRange: ") && !post && s->off > 0)
>> > >>         len += av_strlcatf(headers + len, sizeof(headers) - len,
>> > >>                            "Range: bytes=%"PRId64"-\r\n", s->off);
>> > >>
>> > >
>> > > split and applied above
>> >
>> > May I ask if there is a specific reason for that change?
>> > I haven't double-checked if we use it that way, but detecting if seeking works can be difficult because many servers set the wrong things in response headers.
>> > I believe setting a Range in the request makes those give you a few more hints whether they support it or not.
>>
>> i thought the same yesterday but then after a quick look in the http
>> spec thought its ok, a longer look in the spec now and actual testing
>> though confirms that we loose some hints on seekability with this
>> change, thus
>> reverted
>
> Yes, the problem is that the spec doesn't help much because so many
> people hack their own web servers for special purposes without either
> the will or ability to do it right (including companies like Google, so
> it's not just your typical 3rd rate companies).

I've been testing HLS with a few commercial services, and comparing
with iPhone. I'll send a patch with my URL testcases from the RFC -
the current code thinks a backslash in the query part is in the path,
so breaks some providers I've seen, for example. The iPhone, i guess
the canonical implemantation of HLS, does not send Range headers, in
fact I've seen one provider will always return 403 if you try (hence
my change) - seeking in HLS doesn't really make any sense. Can you
suggest a simple way of omitting the header when the caller is HLS?