[FFmpeg-devel] [PATCH] urlconcat protocol

[Michele Orrù]

We've already discussed them both.


>
> > +        } 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().
>
Nope, uc isn't in unodes, so we have to close it manually.

Michael Niedermayer :
"""
> +        /* creating URLContext */
> +        if ((err = url_open(&uc, urinod, flags)) < 0)
> +            break;
> +        /* creating size */
> +        if ((size = url_filesize(uc)) < 0) {
> +            err = AVERROR(ENOSYS);
> +            break;
> +        }

uc isnt closed from this break
"""

>
> > +            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).
>
Me:
"""
Nope.
 At line 75 we make an an an array enough long to contain all urls , but
using as size as many times AV_CAT_SEPARATOR is present on *uri (+1).
 At line 100, instead, we move to the next url (uri += len + strspn()). So,
strings like cat:foo||bar are accepted.

 In the end, we realloc *nodes in order to free all the useless space that
could be allocated due to multiple consecutive AV_CAT_SEPARATOR.
"""