[FFmpeg-devel] [RFC] missing av_free_packet on error?

[Michael Niedermayer]

On Fri, Jul 11, 2008 at 06:40:18PM +0200, Reimar D?ffinger wrote:
> On Thu, Jul 10, 2008 at 10:31:32PM +0200, Michael Niedermayer wrote:
> > On Thu, Jul 10, 2008 at 09:53:15PM +0200, Reimar D?ffinger wrote:
> > > I have been trying to fix
> > > http://bugzilla.mplayerhq.hu/show_bug.cgi?id=1136
> > > and thus noticed the following code in libavformat/eacdata.c:
> > > ================================
> > > if (av_get_packet(s->pb, pkt, packet_size) != packet_size)
> > >    return AVERROR(EIO);
> > > ================================
> > > 
> > > Whereas av_find_stream_info() assumes that if there is an error, there
> > > is no packet to free.
> > > My question is: where is the error, where should it be fixed?
> > > In the demuxer, in av_find_stream_info, av_read_... or maybe this check
> > > should be moved to av_get_packet, to make it always fail when not all
> > > requested data could be read?
> > > IMO in the demuxer, in which case I suggest this change:
> > 
> > Your change is buggy.
> > You either need to limit the freeing for >=0 return or
> > make av_new_packet() set data=NULL in case of malloc failure
> 
> Hm, I first wanted to change av_init_packet, until I read its doxy...
> That really is a badly chosen name. Changing it would change the public
> API though, and may not be acceptable 

Id change av_init_packet() and the doxy.


> (and if someone has time, could
> you check its use in oggenc.c? It looks somewhat suspicious to me,
> though I do not know the exact semantic of the interleave_packet
> functions)...

hmm iam not sure what you mean, av_interleave_packet_per_dts() looks
similar ;)

besides i do not think oggenc generates valid files anyway, the timestamps
and keyframe flags should be off by 1 if it was implemented based on the same
assumtions about granulepos as oggdec.c

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080712/a58e4232/attachment.pgp>