[FFmpeg-devel] [PATCH] http Transfer-Encoding chunked

Peter Holik peter
Sat Jun 6 18:24:38 CEST 2009


> On Mon, Jun 01, 2009 at 07:46:25PM +0200, Peter Holik wrote:
>> > On Mon, Jun 01, 2009 at 01:21:07PM +0200, Peter Holik wrote:
>> >> > On Mon, Jun 01, 2009 at 12:20:03PM +0200, Peter Holik wrote:
>> >> >> > On Wed, May 27, 2009 at 08:54:37AM +0200, Peter Holik wrote:
>> >> >> >> > Hi Peter,
>> >> >> >> >
>> >> >> >> > On Tue, May 26, 2009 at 3:20 PM, Peter Holik <peter at holik.at> wrote:
>> >> >> >> >> i used printf with DEBUG like i saw in http.c:
>> >> >> >> >>
>> >> >> >> >> process_line
>> >> >> >> >>
>> >> >> >> >> #ifdef DEBUG
>> >> >> >> >> ? ? ? ?printf("http_code=%d\n", s->http_code);
>> >> >> >> >> #endif
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> http_connect
>> >> >> >> >>
>> >> >> >> >> #ifdef DEBUG
>> >> >> >> >> ? ? ? ? ? ?printf("header='%s'\n", line);
>> >> >> >> >> #endif
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> why now use av_log?
>> >> >> >> >
>> >> >> >> > That's a good catch. These lines of code are rather old, and most
>> >> >> >> > likely predate the "forbidding" of printf(). They were not converted
>> >> >> >> > for the simple reason that the compilation doesn't fail because DEBUG
>> >> >> >> > is, by default, not included in CFLAGS. A separate patch which
>> >> >> >> > converts them to av_log() at debugging-level would be much
>> >> >> >> > appreciated. Alternatively, they could also be removed.
>> >> >> >> >
>> >> >> >>
>> >> >> > [...]
>> >> >> >> +            for(;;) {
>> >> >> >> +                ch = http_getc(s);
>> >> >> >> +                if (ch < 0)
>> >> >> >> +                    return 0;
>> >> >> >> +                if (ch == '\n') {
>> >> >> >> +                    /* process chunk size */
>> >> >> >> +                    if (q > line && q[-1] == '\r')
>> >> >> >> +                         q--;
>> >> >> >> +                    *q = '\0';
>> >> >> >> +                    /* skip CR LF from last chunk */
>> >> >> >> +                    if (!(*line)) continue;
>> >> >> >> +
>> >> >> >> +                    s->chunksize = strtoll(line, NULL, 16);
>> >> >> >> +
>> >> >> >> +                    av_log(NULL, AV_LOG_DEBUG, "Chunked encoding data size:
>> >> %"PRId64"'\n",
>> >> >> >> s->chunksize);
>> >> >> >> +
>> >> >> >> +                    if (!s->chunksize)
>> >> >> >> +                        return 0;
>> >> >> >> +                    break;
>> >> >> >> +                } else
>> >> >> >> +                    if ((q - line) < sizeof(line) - 1)
>> >> >> >> +                        *q++ = ch;
>> >> >> >> +            }
>> >> >> >
>> >> >> > looks like code duplication
>> >> >>
>> >> >> looks like, but it is not exactly the same.
>> >> >
>> >> > can it be factorized?
>> >>
>> >> maybe like this patch?
>> >
>> > factorizing http_get_line() out should be a seperate patch
>> >
>> >
>> > [...]
>> >> @@ -151,6 +152,30 @@ static int http_getc(HTTPContext *s)
>> >>      return *s->buf_ptr++;
>> >>  }
>> >>
>> >> +static int http_get_line(HTTPContext *s, char *line, int line_size)
>> >> +{
>> >> +    int ch;
>> >> +    char *q;
>> >> +
>> >> +    q = line;
>> >> +    for(;;) {
>> >> +        ch = http_getc(s);
>> >> +        if (ch < 0)
>> >> +            return AVERROR(EIO);
>> >> +        if (ch == '\n') {
>> >> +            /* process line */
>> >> +            if (q > line && q[-1] == '\r')
>> >> +                q--;
>> >> +            *q = '\0';
>> >> +
>> >> +            return 0;
>> >> +        } else {
>> >> +            if ((q - line) < line_size - 1)
>> >> +                *q++ = ch;
>> >> +        }
>> >> +    }
>> >> +}
>> >> +
>> >>  static int process_line(URLContext *h, char *line, int line_count,
>> >>                          int *new_location)
>> >>  {
>> > [...]
>> >> @@ -251,30 +279,18 @@ static int http_connect(URLContext *h, const char *path, const char
>> >> *hoststr,
>> >>      }
>> >>
>> >>      /* wait for header */
>> >> -    q = line;
>> >>      for(;;) {
>> >> -        ch = http_getc(s);
>> >> -        if (ch < 0)
>> >> +        if (http_get_line(s, line, sizeof(line)) < 0)
>> >>              return AVERROR(EIO);
>> >> -        if (ch == '\n') {
>> >> -            /* process line */
>> >> -            if (q > line && q[-1] == '\r')
>> >> -                q--;
>> >> -            *q = '\0';
>> >> -#ifdef DEBUG
>> >> -            printf("header='%s'\n", line);
>> >> -#endif
>> >> -            err = process_line(h, line, s->line_count, new_location);
>> >> -            if (err < 0)
>> >> -                return err;
>> >> -            if (err == 0)
>> >> -                break;
>> >> -            s->line_count++;
>> >> -            q = line;
>> >> -        } else {
>> >> -            if ((q - line) < sizeof(line) - 1)
>> >> -                *q++ = ch;
>> >> -        }
>> >> +
>> >> +        av_log(NULL, AV_LOG_DEBUG, "http header='%s'\n", line);
>> >> +
>> >> +        err = process_line(h, line, s->line_count, new_location);
>> >> +        if (err < 0)
>> >> +            return err;
>> >> +        if (err == 0)
>> >> +            break;
>> >> +        s->line_count++;
>> >>      }
>> >>
>> >>      return (off == s->off) ? 0 : -1;
>>
>> now every little bit is separated
>>
>> cu Peter
>
>>  http.c |   24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>> 4c9c8f96e640dad6934d1282c64665ff09241188  0001-introduce-http_get_line.patch
>> >From 44c840033191cd4bed784d68e4a51ee1ccb00976 Mon Sep 17 00:00:00 2001
>> From: Peter Holik <peter at holik.at>
>> Date: Mon, 1 Jun 2009 19:09:36 +0200
>> Subject: [PATCH 1/5] introduce http_get_line
> [...]
>>  http.c |   18 +++---------------
>>  1 file changed, 3 insertions(+), 15 deletions(-)
>> cdf00979442a748962c6f77e33bb58727a121359  0002-http_connect-changed-to-use-http_get_line.patch
>> >From 3f04da28ab2422a59a740175f067793dd1260f5a Mon Sep 17 00:00:00 2001
>> From: Peter Holik <peter at holik.at>
>> Date: Mon, 1 Jun 2009 19:11:28 +0200
>> Subject: [PATCH 2/5] http_connect changed to use http_get_line
>
> above 2 when commited togteher are ok

ok

> also if this can be shared with rtsp, this is of course welcome

no clue

>>  http.c |   14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>> 0dcfc6f947f335e4ee9e82c240db17a51bf11b61  0003-cosmetics-for-http_connect.patch
>> >From fee3eb8edf269cba3bedae0caea4a539d2f1cf76 Mon Sep 17 00:00:00 2001
>> From: Peter Holik <peter at holik.at>
>> Date: Mon, 1 Jun 2009 19:12:32 +0200
>> Subject: [PATCH 3/5] cosmetics for http_connect
>
> ok
>
>
> [...]
>
>>  http.c |   14 ++++++--------
>>  1 file changed, 6 insertions(+), 8 deletions(-)
>> cab2ee8ef3b09c92afc85bd100709c4c6d1816ba  0004-cosmetics-printf-to-av_log.patch
>> >From e491e3b736b9069ea5a2b4d7252f0478345ee8ad Mon Sep 17 00:00:00 2001
>> From: Peter Holik <peter at holik.at>
>> Date: Mon, 1 Jun 2009 19:17:13 +0200
>> Subject: [PATCH 4/5] cosmetics printf to av_log
> [...]
>> @@ -192,9 +190,9 @@ static int process_line(URLContext *h, char *line, int line_count,
>>          while (isspace(*p))
>>              p++;
>>          s->http_code = strtol(p, NULL, 10);
>> -#ifdef DEBUG
>> -        printf("http_code=%d\n", s->http_code);
>> -#endif
>> +
>> +        av_log(NULL, AV_LOG_DEBUG, "http_code=%d\n", s->http_code);
>
> dprintf()

ok

>> @@ -296,6 +302,29 @@ static int http_read(URLContext *h, uint8_t *buf, int size)
>>      HTTPContext *s = h->priv_data;
>>      int len;
>>
>> +    if (s->chunksize >= 0) {
>> +        if (!s->chunksize) {
>> +            char line[32];
>> +
>> +            for(;;) {
>> +                if (http_get_line(s, line, sizeof(line)) < 0)
>> +                    return AVERROR(EIO);
>> +
>> +                /* skip CR LF from last chunk */
>> +                if (!(*line)) continue;
>> +
>> +                s->chunksize = strtoll(line, NULL, 16);
>> +
>
>> +                av_log(NULL, AV_LOG_DEBUG, "Chunked encoding data size: %"PRId64"'\n",
>> s->chunksize);
>
> i think this one should be dprintf() as well

ok

>> +
>> +                if (!s->chunksize)
>> +                    return 0;
>> +                break;
>> +            }
>> +        }
>
>> +        if (s->chunksize < size)
>> +            size = s->chunksize;
>
> size= FFMIN(size, s->chunksize);

ok

here now the new patches

cu Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01-introduce-http_get_line.patch
Type: text/x-diff
Size: 2106 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090606/bb1a962e/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02-cosmetics-for-http_connect.patch
Type: text/x-diff
Size: 864 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090606/bb1a962e/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03-cosmetics-printf-to-dprintf.patch
Type: text/x-diff
Size: 1221 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090606/bb1a962e/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 04-http-Transfer-Encoding-chunked.patch
Type: text/x-diff
Size: 2388 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090606/bb1a962e/attachment-0003.patch>



More information about the ffmpeg-devel mailing list