[FFmpeg-devel] [PATCH] http Transfer-Encoding chunked
Michael Niedermayer
michaelni
Mon Jun 1 17:11:50 CEST 2009
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;
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20090601/d385da44/attachment.pgp>
More information about the ffmpeg-devel
mailing list