[FFmpeg-devel] [PATCH] urlconcat protocol

[Stefano Sabatini]

On date Saturday 2010-02-06 21:36:03 +0100, Michele Orr? encoded:
> >
> >
> > I don't like it very much, an url/uri is an identifier/locator of a
> > resource (which can be retrieved using some protocol, yes a normal
> > file can be identified by the url "file:/FILE"), what the protocol
> > allows to concatenate is the physical content of the resources
> > referenced by the urls, so I continue to prefer "concat" / "cat"
> > as names.
> >
> > The term "url" in urlconcat is misleading, there is nothing special
> > about urls in this protocol as all the files/streams in ffmpeg are
> > referenced using uris (e.g. ffplay file:/in.avi).
> >
> > Also having the name of the protocol equal to the prefix used
> > (e.g. concat protocol -> "concat:") would be nice as this is what
> > happens for all the other supported protocols.
> >
> >
> Choose the patch you prefer. The previous uses "urlconcat", this one
> instead "concat".

> Index: libavformat/concat.c
[...]
> +static int concat_open(URLContext *h, const char *uri, int flags)
> +{
> +    char *node_uri = NULL, *tmp_uri;
> +    int err = 0;
> +    int64_t size;
> +    size_t  len, i;
> +    URLContext *uc;
> +    struct concat_data *udata;
> +    struct concat_nodes *unodes;
> +
> +    av_strstart(uri, "concat:", &uri);
> +
> +    /* creating udata */
> +    if (!(udata = av_mallocz(sizeof(*udata))))
> +        return AVERROR(ENOMEM);
> +    h->priv_data = udata;
> +
> +    for (i = 0, len = 1; uri[i]; i++)  /* cat:[url]|[url] -> nodes = sep+1 */
> +        if (uri[i] == *AV_CAT_SEPARATOR)
> +            /* integer overflow */
> +            if (++len == UINT_MAX / sizeof(*unodes)) {
> +                av_freep(&h->priv_data);
> +                return AVERROR(ENAMETOOLONG);
> +            }
> +    if (!(unodes = av_malloc(sizeof(*unodes) * len))) {
> +        av_freep(&h->priv_data);
> +        return AVERROR(ENOMEM);
> +    } else
> +        udata->nodes = unodes;
> +
> +    /* handle input */
> +    if (!*uri)
> +        err = AVERROR(ENOENT);
> +    for (i = 0; *uri; i++) {
> +        /* parsing uri */
> +        len = strcspn(uri, AV_CAT_SEPARATOR);
> +        if (!(tmp_uri = av_realloc(node_uri, len+1))) {
> +            err = AVERROR(ENOMEM);
> +            break;
> +        } else
> +            node_uri = tmp_uri;
> +        av_strlcpy(node_uri, uri, len+1);
> +        uri += len + strspn(uri+len, AV_CAT_SEPARATOR);
> +        /* creating URLContext */
> +        if ((err = url_open(&uc, node_uri, flags)) < 0)
> +            break;
> +        /* creating size */
> +        if ((size = url_filesize(uc)) < 0) {

> +            url_close(uc);

This shouldn't be necessary, the url context is closed in concat_close().

> +            err = AVERROR(ENOSYS);
> +            break;
> +        }
> +        /* assembling */
> +        unodes[i].uc   = uc;
> +        unodes[i].size = size;
> +    }
> +    av_free(node_uri);
> +    udata->length = i;
> +    if (err < 0)
> +        concat_close(h);

> +    else if (!(unodes = av_realloc(unodes, udata->length * sizeof(*unodes)))) {
> +        concat_close(h);
> +        err = AVERROR(ENOMEM);
> +    } else
> +        udata->nodes = unodes;

Why this? I mean nodes is already alloc'ed, no need to reallocate it
(noticed when I was almost committing).

> +    return err;
> +}

Regards.
-- 
FFmpeg = Friendly and Friendly Mournful Powerful Extravagant Gadget