[FFmpeg-devel] [PATCH] SHOUTcast HTTP Support

[Michael Niedermayer]

On Wed, Mar 31, 2010 at 06:43:10PM -0400, Micah F. Galizia wrote:

[...]
> @@ -453,3 +497,298 @@
>      http_close,
>      .url_get_file_handle = http_get_file_handle,
>  };
> +
> +
> +#if CONFIG_SHOUTCAST_DEMUXER
> +
> +#define SHOUTCAST_DEMUXER_NAME "shoutcast"
> +

> +typedef struct {
> +    unsigned long chapters;
> +    unsigned long metaint;
> +    unsigned long datapos;

why long?


> +    uint64_t start_time;
> +    AVRational time_base;
> +    AVInputFormat *child;
> +    void *child_priv;
> +} SHOUTcast;
> +
> +static int shoutcast_parse_metadata(AVChapter *c, uint8_t *buffer, int size)

some doxy documentation could possibly help readability of all new functions


> +{
> +    int i = 0, start = 0, mid = 0, inquote = 0;
> +
> +    for (i = 0; i < size; i++) {
> +        switch (buffer[i]) {
> +        case '\'':
> +            inquote = inquote ? 0 : 1;

inquote = !inquote


[...]

> +    if (!(sc->child)) {

unneeded ()


> +        av_log(s, AV_LOG_ERROR, "ERROR: Unable to get child demuxer.\n");
> +        return AVERROR_IO;
> +    }
> +

> +    /* there is no shoutcast in shoutcast */
> +    if (!strcmp(sc->child->name, SHOUTCAST_DEMUXER_NAME)) {
> +        av_log(s, AV_LOG_ERROR, "ERROR: Child demuxer is %s\n",
> +               sc->child->name);
> +        return AVERROR_IO;
> +    }

thats fragile, we just need a second shoutcast like demxuer and
each with such check would not be enough


> +
> +    if (!(sc->child_priv = av_mallocz(sc->child->priv_data_size))) {
> +        av_log(s, AV_LOG_ERROR,
> +               "Unable to allocate SHOUTcast child private data.\n");
> +        return AVERROR_NOMEM;
> +    }
> +
> +    /* read past the HTTP header (after probing the stream is reset) */
> +    url_fseek(s->pb, offset, SEEK_CUR);
> +

> +    /* ensure that we set the data position ahead if the child demuxer moves
> +     * the buffer pointer forward. TODO Use pb->pos if possible. */
> +    offset = (unsigned long)(s->pb->buf_ptr);

whatever this is it has undefined behavior


> +    s->priv_data = sc->child_priv;
> +    ret = sc->child->read_header(s, ap);
> +    s->priv_data = sc;
> +    sc->datapos = (unsigned long)(s->pb->buf_ptr) - offset;
> +
> +    return ret;
> +}
> +

> +static int shoutcast_read_packet(struct AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret;
> +    ByteIOContext *pb = s->pb;
> +    SHOUTcast *sc = s->priv_data;
> +
> +    /* allow the child demuxer to read a packet. */
> +    s->priv_data = sc->child_priv;
> +    if ((ret = sc->child->read_packet(s, pkt)) < 0) {
> +        return ret;
> +    }
> +    s->priv_data = sc;

> +    sc->datapos += pkt->size;

what is datapos supposed to be?
this just looks wrong, it surely is not the "file" position


> +
> +    if (sc->metaint && (sc->datapos > sc->metaint)) {
> +        unsigned int lm_size = 0, md_size = 0, rm_size = 0, rd_size = 0;
> +        unsigned int ld_size = pkt->size - sc->datapos + sc->metaint;
> +        AVPacket pkt2;
> +
> +        /* store the metadata segment size */
> +        md_size = pkt->data[ld_size] * 16;
> +        lm_size = (ld_size + md_size + 1 > pkt->size) ?
> +                  sc->datapos - sc->metaint - 1 : md_size;
> +
> +        /* get remaining metadata size and right data size */
> +        rm_size = md_size - lm_size;
> +        rd_size = (md_size > lm_size) ? 0 : pkt->size - ld_size - md_size - 1;

lm,md,rm,rd are what?


> +
> +        /* read remaining metadata */
> +        av_get_packet(pb, &pkt2, rm_size);
> +
> +        /* copy the metadata */
> +        if (md_size) {
> +            char *md;
> +            AVChapter *chap;
> +
> +            if ((md = av_malloc(sizeof(uint8_t)*(md_size + 1)))) {

> +                uint64_t start = av_gettime() - sc->start_time;

thats wrong, the chapter starts dont change through network delay


[...]
> +static int shoutcast_read_close(struct AVFormatContext *s)
> +{
> +    int ret = 0;
> +    SHOUTcast *sc = s->priv_data;
> +
> +    if (sc->child->read_close) {
> +        s->priv_data = sc->child_priv;
> +        ret = sc->child->read_close(s);
> +        s->priv_data = sc;
> +    }
> +

> +    av_free(sc->child_priv);
> +    sc->child_priv = NULL;

av_freep()

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

Thouse who are best at talking, realize last or never when they are wrong.
-------------- 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/20100407/a360de94/attachment.pgp>