[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