[Ffmpeg-devel] [PATCH] CRYO APC demuxer

[Anssi Hannula]

Reimar D?ffinger wrote:
> Hello,
> On Sat, Apr 07, 2007 at 04:41:36PM +0300, Anssi Hannula wrote:
>> Michael Niedermayer wrote:
>>> On Sat, Apr 07, 2007 at 02:37:56PM +0200, Reimar D?ffinger wrote:
>>> [...]
>>>>> +    st->codec->sample_rate = get_le32(pb);
>>>>> +
>>>>> +    st->codec->extradata_size = 2 * sizeof(int);
>>>>> +    st->codec->extradata = av_malloc(st->codec->extradata_size +
>>>>> +                                     FF_INPUT_BUFFER_PADDING_SIZE);
>>>>> +    if (!st->codec->extradata) {
>>>>> +        if (st->codec)
>>>>> +             av_free(st->codec);
>>>>> +        av_free(st);
>>>> Hmm... none of the other demuxers free st->codec. Actually, they just
>>>> ignore when they can't set extradata. No idea what is the right
>>>> behaviour.
>>> they likely segfault after mem alloc failure 
> 
> Don't think so. At least mov.c checks for extradata != NULL. It just
> does nothing if it is == NULL. Not sure that is such a good thing
> though.
> 
>>>> But freeing st after it was registered really seems wrong to me.
>>> yes to me too
>> So should I ignore extradata as well?
> 
> IMO no, just don't do the frees on failure. And/or check with valgrind that it's
> right. If there isn't some other code that frees all streams and codec
> entries on error return that is a bug IMO, it would be stupid to have
> all demuxers duplicate that code.

No need for valgrind here AFAICS..
read_header() is called from av_open_input_stream(), and on failure it
just frees demuxer's priv_data and AVFormatContext, leaving the streams
there.

-- 
Anssi Hannula