[FFmpeg-devel] [PATCH] http: add support for reading streamcast metadata

Stefano Sabatini stefasab at gmail.com
Sat Jun 29 02:06:41 CEST 2013


On date Thursday 2013-06-27 18:52:39 +0200, wm4 encoded:
> On Thu, 27 Jun 2013 15:14:58 +0200
> Stefano Sabatini <stefasab at gmail.com> wrote:
> 
> Thanks for reviewing.
> 
> > > @@ -375,6 +385,16 @@ static int process_line(URLContext *h, char
> > > *line, int line_count, snprintf(s->cookies, str_size, "%s\n%s",
> > > tmp, p); av_free(tmp);
> > >              }
> > > +        } else if (!av_strcasecmp (tag, "Icy-MetaInt")) {
> > > +            s->icy_metaint = strtoll(p, NULL, 10);
> > > +        } else if (!av_strncasecmp(tag, "Icy-", 4)) {
> > > +            AVBPrint bp;
> > > +            av_bprint_init(&bp, 1, -1);
> > > +            if (s->icy_meta_header)
> > > +                av_bprintf(&bp, "%s", s->icy_meta_header);
> > > +            av_bprintf(&bp, "%s: %s\n", tag, p);
> > > +            av_freep(&s->icy_meta_header);
> > > +            av_bprint_finalize(&bp, &s->icy_meta_header);
> > 
> > Nit++: probably easier if you first free, then fill the new value but
> > maybe it's just me
> 
> No, I actually need the old value before that, because all header lines
> starting with icy- are concatenated.
> 
> > > +    if (s->icy_metaint > 0) {
> > > +        int remaining = s->icy_metaint - s->icy_data_read;
> > > +        if (!remaining) {
> > > +            char data[4096];
> > > +            char *buf;
> > > +            int n;
> > > +            int ch = http_getc(s);
> > > +            if (ch < 0)
> > > +                return ch;
> > > +            if (ch > 0) {
> > > +                ch *= 16;
> > > +                for (n = 0; n < ch; n++)
> > > +                    data[n] = http_getc(s);
> > > +                buf = av_mallocz(ch + 1);
> > > +                if (buf)
> > > +                    memcpy(buf, data, ch);
> > > +                av_freep(&s->icy_meta_packet);
> > > +                s->icy_meta_packet = buf;
> > > +            }
> > > +            s->icy_data_read = 0;
> > > +            remaining = s->icy_metaint;
> > > +        }
> > > +        size = FFMIN(size, remaining);
> > > +    }
> > 
> > I can't really comment about this part. Can you add some generic
> > comments to explain what it's doing?
> 
> Tried to do so. Note that I call http_getc() in a loop. This is a bit
> dumb, but saves me from dealing with the buffer management code that is
> in http.c.

> From ebff3e39308b4ba1ea2e09776a021782fdddc347 Mon Sep 17 00:00:00 2001
> From: wm4 <nfxjfg at googlemail.com>
> Date: Wed, 26 Jun 2013 00:53:26 +0200
> Subject: [PATCH] http: add support for reading streamcast metadata
> 
> Allow applications to request reading streamcast metadata. This uses
> AVOptions as API, and requires the application to explicitly request
> and read metadata. Metadata can be updated mid-stream; if an
> application is interested in that, it has to poll for the data by
> reading the "icy_metadata_packet" option in regular intervals.
> 
> There doesn't seem to be a nice way to transfer the metadata in a nicer
> way. Converting the metadata to ID3v2 tags might be a nice idea, but
> the libavformat mp3 demuxer doesn't seem to read these tags mid-stream,
> and even then we couldn't guarantee that tags are not inserted in the
> middle of mp3 packet data.
> 
> This commit provides the minimum to enable applications to retrieve
> this information at all.
> ---
>  doc/protocols.texi | 14 +++++++++++++
>  libavformat/http.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 97ff62d..e8427aa 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -228,6 +228,20 @@ not specified.
>  @item mime_type
>  Set MIME type.
>  
> + at item icy
> +If set to 1 request ICY (SHOUTcast) metadata from the server. If the server
> +supports this, the metadata has to be retrieved by the application by reading
> +the @option{icy_metadata_headers} and @option{icy_metadata_packet} options.
> +The default is 0.
> +
> + at item icy_metadata_headers
> +If the server supports ICY metadata, this contains the ICY specific HTTP reply
> +headers, separated with newline characters.
> +
> + at item icy_metadata_packet
> +If the server supports ICY metadata, and @option{icy} was set to 1, this
> +contains the last non-empty metadata packet sent by the server.
> +
>  @item cookies
>  Set the cookies to be sent in future requests. The format of each cookie is the
>  same as the value of a Set-Cookie HTTP response field. Multiple cookies can be
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 91f8d1f..b8c9324 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -28,6 +28,7 @@
>  #include "httpauth.h"
>  #include "url.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/bprint.h"
>  
>  /* XXX: POST protocol is not completely implemented because ffmpeg uses
>     only a subset of it. */
> @@ -49,6 +50,8 @@ typedef struct {
>      char *content_type;
>      char *user_agent;
>      int64_t off, filesize;
> +    int icy_data_read;      /**< how much data was read since last ICY metadata packet */

> +    int icy_metaint;        /**< new metadata packet after that many bytes of data read */

uh? Still can't get what it is from the description. From my unerstanding:
specify after how many bytes of read data a new metadata packet will be found

Is that correct?

Also nit: ///<
is the prevailing convention

>      char location[MAX_URL_SIZE];
>      HTTPAuthState auth_state;
>      HTTPAuthState proxy_auth_state;
> @@ -65,6 +68,9 @@ typedef struct {
>      int rw_timeout;
>      char *mime_type;
>      char *cookies;          ///< holds newline (\n) delimited Set-Cookie header field values (without the "Set-Cookie: " field name)
> +    int icy;
> +    char *icy_metadata_headers;
> +    char *icy_metadata_packet;
>  } HTTPContext;
>  
>  #define OFFSET(x) offsetof(HTTPContext, x)
> @@ -82,6 +88,9 @@ static const AVOption options[] = {
>  {"timeout", "set timeout of socket I/O operations", OFFSET(rw_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, D|E },
>  {"mime_type", "set MIME type", OFFSET(mime_type), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
>  {"cookies", "set cookies to be sent in applicable future requests, use newline delimited Set-Cookie HTTP field value syntax", OFFSET(cookies), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
> +{"icy", "request ICY metadata", OFFSET(icy), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, D },
> +{"icy_metadata_headers", "return ICY metadata headers", OFFSET(icy_metadata_headers), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
> +{"icy_metadata_packet", "return current ICY metadata packet", OFFSET(icy_metadata_packet), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
>  {NULL}
>  };
>  #define HTTP_CLASS(flavor)\
> @@ -218,6 +227,7 @@ int ff_http_do_new_request(URLContext *h, const char *uri)
>      HTTPContext *s = h->priv_data;
>  
>      s->off = 0;
> +    s->icy_data_read = 0;
>      av_strlcpy(s->location, uri, sizeof(s->location));
>  
>      return http_open_cnx(h);
> @@ -375,6 +385,17 @@ static int process_line(URLContext *h, char *line, int line_count,
>                  snprintf(s->cookies, str_size, "%s\n%s", tmp, p);
>                  av_free(tmp);
>              }
> +        } else if (!av_strcasecmp (tag, "Icy-MetaInt")) {
> +            s->icy_metaint = strtoll(p, NULL, 10);
> +        } else if (!av_strncasecmp(tag, "Icy-", 4)) {
> +            // Concat all Icy- header lines
> +            AVBPrint bp;
> +            av_bprint_init(&bp, 1, -1);
> +            if (s->icy_metadata_headers)
> +                av_bprintf(&bp, "%s", s->icy_metadata_headers);
> +            av_bprintf(&bp, "%s: %s\n", tag, p);
> +            av_freep(&s->icy_metadata_headers);
> +            av_bprint_finalize(&bp, &s->icy_metadata_headers);
>          }
>      }
>      return 1;
> @@ -580,6 +601,10 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>              av_free(cookies);
>          }
>      }

> +    if (!has_header(s->headers, "\r\nIcy-MetaData: ") && s->icy) {
> +        len += av_strlcatf(headers + len, sizeof(headers) - len,
> +                           "Icy-MetaData: %d\r\n", 1);
> +    }

why this?

>  
>      /* now add in custom headers */
>      if (s->headers)
> @@ -613,6 +638,7 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>      s->buf_end = s->buffer;
>      s->line_count = 0;
>      s->off = 0;
> +    s->icy_data_read = 0;
>      s->filesize = -1;
>      s->willclose = 0;
>      s->end_chunked_post = 0;
> @@ -652,6 +678,7 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size)
>      }
>      if (len > 0) {
>          s->off += len;
> +        s->icy_data_read += len;
>          if (s->chunksize > 0)
>              s->chunksize -= len;
>      }
> @@ -693,6 +720,34 @@ static int http_read(URLContext *h, uint8_t *buf, int size)
>          }
>          size = FFMIN(size, s->chunksize);
>      }
> +    if (s->icy_metaint > 0) {
> +        int remaining = s->icy_metaint - s->icy_data_read; /* until next metadata packet */
> +        if (!remaining) {
> +            char data[4096];
> +            char *buf;
> +            int n;
> +            // The metadata packet is variable sized. It has a 1 byte header
> +            // which sets the length of the packet (divided by 16). If it's 0,
> +            // the metadata doesn't change. After the packet, icy_metaint bytes
> +            // of normal data follow.
> +            int ch = http_getc(s);
> +            if (ch < 0)
> +                return ch;
> +            if (ch > 0) {
> +                ch *= 16;

> +                for (n = 0; n < ch; n++)
> +                    data[n] = http_getc(s);
> +                buf = av_mallocz(ch + 1);

shouldn't you abort with AVERROR(ENOMEM) in case !buf?

> +                if (buf)
> +                    memcpy(buf, data, ch);

> +                av_freep(&s->icy_metadata_packet);
> +                s->icy_metadata_packet = buf;

av_opt_set?

> +            }
> +            s->icy_data_read = 0;
> +            remaining = s->icy_metaint;
> +        }
> +        size = FFMIN(size, remaining);
> +    }
>      return http_buf_read(h, buf, size);
>  }
>  
> @@ -744,6 +799,9 @@ static int http_close(URLContext *h)
>      int ret = 0;
>      HTTPContext *s = h->priv_data;
>  
> +    av_freep(&s->icy_metadata_headers);
> +    av_freep(&s->icy_metadata_packet);

Private context options values should be freed automatically.

[...]
-- 
FFmpeg = Fostering and Fierce Mind-dumbing Peaceless Elitarian Gospel


More information about the ffmpeg-devel mailing list