[Ffmpeg-devel] [PATCH] CRYO APC demuxer

[Michael Niedermayer]

Hi

On Sat, Apr 07, 2007 at 02:37:56PM +0200, Reimar D?ffinger wrote:
[...]
> > +static int apc_read_header(AVFormatContext *s,
> > +                           AVFormatParameters *ap)
> > +{
> > +    ByteIOContext *pb = &s->pb;
> > +    AVStream *st;
> > +
> > +    if (get_le32(pb) != MKTAG('C', 'R', 'Y', 'O'))
> > +        return AVERROR_INVALIDDATA;
> > +
> > +    if (get_le32(pb) != MKTAG('_', 'A', 'P', 'C'))
> > +        return AVERROR_INVALIDDATA;
> 
> And this check should be consistent with the above. But actually I'd
> prefer if you just wouldn't do the check here at all, since that allows
> users to force the demuxer in case they have a file where this signature
> is (intentionally?) broken.

indeed i agree, this also should be fixed in the existing demuxers, iam
sure several do such checks ...


> 
> > +    st->nb_frames = get_le32(pb); /* total number of samples */
> 
> Hm. Is nb_frames really supposed to be used for the number of samples?

a quick check indicates that AVStream.nb_frames is unused except by a
few (de)muxers but the generic code doesnt seem to touch it so id
say this should be removed


> 
> > +    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 


> But freeing st after it was registered really seems wrong to me.

yes to me too

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20070407/e426de3c/attachment.pgp>