[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