[FFmpeg-devel] [PATCH] File concat protocol

[Michael Niedermayer]

Hi

On Wed, Jul 04, 2007 at 01:08:43PM -0000, Wolfram Gloger wrote:
[...]
> > > +            return AVERROR_NOMEM;
> > 
> > please use AVERROR() AVERROR_* is deprecated
> 
> Ok, but could someone please add a note to this effect to avcodec.h?
> I could find this nowhere, and AVERROR_* is used quite extensively.

patch welcome


[...]
> > also the uc and size (temporary) variables are unneeded
> 
> Disagree, that would make a leak-free open() very messy.

leak free in the sense that the OS will free things after the code segfaults
that is uinfo->url being freed and then accessed afterwards
(or if its not accessed then theres a leak, as its just freed on some error
paths)

also if you use the argument that checking for all error cases and properly
freeing stuff would be messy without the tmp vars then please first do 
check for all, your current code does not, half of the malloc() failures
will end in a segfault the other half is checked for properly


> 
> > and i think the seeking code should be simpler if instead of storing the
> > sizes of all urls the sum of sizes of the previous urls would be stored
> > (so that url_sizes[i] is the size of url 0 to i)
> > though if its not simpler then just disregard this comment
> 
> I don't think is becomes simpler because of the whence==SEEK_END
> case (added just for you :), so I didn't change this.

ill investigate this more completely after the other issues have been dealt
with


[...]

> +#ifndef AV_CAT_SEPARATOR
> +#define AV_CAT_SEPARATOR '|'
> +#endif

whats the #ifndef good for?


[...]
> +static int urlconcat_close(URLContext *h)
> +{
> +    size_t i;
> +    int res = 0;
> +    struct urlconcat_info *uinfo = h->priv_data;
> +
> +    if (uinfo != NULL) {

the != NULL is unneeded


> +        for (i=0; i<uinfo->urlnum; i++)
> +            res |= url_close(uinfo->url[i].uc);
> +        av_free(uinfo->url);
> +        av_freep(&h->priv_data);
> +    }
> +    return res;

i dont think the bitwise or of the returns is guranteed to be a proper
error code


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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070704/685ae626/attachment.pgp>