[FFmpeg-devel] [PATCH] fix Avidec.c to not use pkt->size when discard

[Thierry Foucu]

On Mon, May 3, 2010 at 10:07 AM, Michael Niedermayer <michaelni at gmx.at>wrote:

> On Fri, Apr 30, 2010 at 09:57:07AM -0700, Thierry Foucu wrote:
> > Here is the issue I found when debugging a AVI file:
> > The line number are based on SVN 22976
> >
> > in av_find_stream_info (ic=0xf358020) at libavformat/utils.c:2210
> >    We are calling av_read_frame_internal with pkt1 which is a variable on
> > the stack (not initialize)
> >
> > in av_read_frame_internal (s=0xf358020, pkt=0xffffcb64) at
> > libavformat/utils.c:1070
> >   When we enter av_read_frame_internal, we call  av_init_packet, which
> does
> > does set the pkt->data and pkt->size to zero
> >   Then we call av_read_packet with the same AVpacket.
> >
> > in av_read_packet (s=0xf358020, pkt=0xffffc654) at
> libavformat/utils.c:638
> >   we are calling avi_read_packet with the same AVPacket pointer, which
> still
> > does not have the data and size set to
> >
> > in avi_read_packet (s=0xf358020, pkt=0xffffc654) at
> libavformat/avidec.c:890
> >   Here we are adding to ast->frame_offset the pkt->size, which in this
> case,
> > it was not initialize. The pkt->data is NULL
> >   This will cause the ast->frame_offset for a audio packet to be wrong
> and
> > because we are using it for setting the DTS, we can get some first PTS
> value
> > to be 100% garbage.
> >
> > By applying the patch, the ast->frame_offset will not be incremented by
> some
> > garbage value.
> >
> > But not sure if this will be the right fix for this problem.
>
> probably the code should use size instead of pkt->size in avidec
> a patch changing this with is welcome
> a testcase is welcome as well thogh due to below not required
> (note a testcase will likely require to chnage AVStream.discard midstream
>  for this corruption to become user vissible)
>
>
Changing subject to reflect the patch:
============================
ok, what about this patch:

You get in this condition, when you call avi_read_packet with
avi->non_interleaved = false
and
avi->stream_index = -1

You then go into the for loop Line 820
at that point pkt->data and pkt->size were never initialize

so, with my change, the ast->frame_offset will be increase by size, not my
pkt->size.

Index: libavformat/avidec.c
===================================================================
--- libavformat/avidec.c (revision 23014)
+++ libavformat/avidec.c (working copy)
@@ -887,7 +887,7 @@
             if(   (st->discard >= AVDISCARD_DEFAULT && size==0)
                /*|| (st->discard >= AVDISCARD_NONKEY && !(pkt->flags &
AV_PKT_FLAG_KEY))*/ //FIXME needs a little reordering
                || st->discard >= AVDISCARD_ALL){
-                if(ast->sample_size) ast->frame_offset += pkt->size;
+                if(ast->sample_size) ast->frame_offset += size;
                 else                 ast->frame_offset++;
                 url_fskip(pb, size);
                 goto resync;
-------------- next part --------------
Index: libavformat/avidec.c
===================================================================
--- libavformat/avidec.c	(revision 23014)
+++ libavformat/avidec.c	(working copy)
@@ -887,7 +887,7 @@
             if(   (st->discard >= AVDISCARD_DEFAULT && size==0)
                /*|| (st->discard >= AVDISCARD_NONKEY && !(pkt->flags & AV_PKT_FLAG_KEY))*/ //FIXME needs a little reordering
                || st->discard >= AVDISCARD_ALL){
-                if(ast->sample_size) ast->frame_offset += pkt->size;
+                if(ast->sample_size) ast->frame_offset += size;
                 else                 ast->frame_offset++;
                 url_fskip(pb, size);
                 goto resync;