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

Michael Niedermayer michaelni
Sat Jun 6 17:49:45 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

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

[...]


>  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()


[...]

> @@ -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


> +
> +                if (!s->chunksize)
> +                    return 0;
> +                break;
> +            }
> +        }

> +        if (s->chunksize < size)
> +            size = s->chunksize;

size= FFMIN(size, s->chunksize);


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090606/d73a22d7/attachment.pgp>



More information about the ffmpeg-devel mailing list