[FFmpeg-devel] [PATCH] urlconcat protocol

[Stefano Sabatini]

On date Sunday 2010-02-07 07:34:32 +0100, Michele Orr? encoded:
> 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.

You can set uc[i] before to call concat_close().
 
> 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.

I see, but it looks a little overkill a realloc in this case.

Anyway since I nitpicketed enough, I applied your patch with some
cosmetics (mainly udata -> data, unodes -> nodes and some empty
newlines addition for enhanced readability).

Thanks for the contribution!
-- 
FFmpeg = Frenzy and Fascinating Mortal Peaceful Elitarian Gnome