[FFmpeg-devel] ffprobe: Do not decode zero-sized packets in ffprobe -show_frame

[Petter Ericson]

On Wed, Feb 15, 2012 at 04:30:21AM +0100, Michael Niedermayer wrote:
> On Wed, Feb 15, 2012 at 12:10:49AM +0100, Stefano Sabatini wrote:
> > On date Tuesday 2012-02-14 17:26:13 +0100, Petter Ericson encoded:
> > > Greetings
> > > 
> > > Ticket #997 details a segfault in ffprobe that was exposed by the file
> > > http://titan.codemill.se/~peteri/120210144737.ts
> > > 
> > > This patch fixes the segfault. However, the desyncing issue that I
> > > mentioned in the ticket still remains (I was sloppy when looking at the
> > > output - It is still present in git master). If anyone could advice on what
> > > it is that is causing transcoding to result in desynced output, I would be
> > > most grateful. 
> > > 
> > > The patched ffmpeg passes make fate.
> > > 
> > > Best regards
> > > 
> > > Petter Ericson
> > > 
> > 
> > > commit e40952af2f5b6eccf24b34926bf09747117affdc
> > > Author: Petter Ericson <petter.ericson at codemill.se>
> > > Date:   Tue Feb 14 16:59:56 2012 +0100
> > > 
> > >     ffprobe: Do not avcodec_decode_video2 video packets with size == 0
> > > 
> > > diff --git a/ffprobe.c b/ffprobe.c
> > > index 79f7494..e23ef98 100644
> > > --- a/ffprobe.c
> > > +++ b/ffprobe.c
> > > @@ -1349,6 +1349,8 @@ static av_always_inline int get_decoded_frame(AVFormatContext *fmt_ctx,
> > >      *got_frame = 0;
> > >      switch (dec_ctx->codec_type) {
> > >      case AVMEDIA_TYPE_VIDEO:
> > > +        if(pkt->size == 0)
> > > +            return ret;
> > >          ret = avcodec_decode_video2(dec_ctx, frame, got_frame, pkt);
> > >          break;
> > 
> > Should be fine, alternatively I propose the attached patch which I
> > like more.
> > -- 
> > FFmpeg = Fundamental Frenzy Meaningful Pacific Extravagant Gadget
> 
> >  utils.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 22035da337e788216178af03b134d1ef38cd98b6  0001-lavc-make-avcodec_decode_video2-immediately-return-0.patch
> > From 173f70d8fca9698067e95f5fc31d487fae77d168 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Wed, 15 Feb 2012 00:06:59 +0100
> > Subject: [PATCH] lavc: make avcodec_decode_video2() immediately return 0 for
> >  pkt with size == 0
> > 
> > Avoid check in application code, simplify handling generic code.
> > 
> > In particular, fix crash in ffprobe.c when decoding a packet with size 0,
> > trac ticket #997.
> 
> why does a size=0 packet cause a crash ?

I am not sure if it does at all times, to be honest. What I think happens
for this specific file is that (in lavc/h264.c:decode_frame), the first
if-clause hits, s->current_picture_ptr is unset, and then for some reason
is not set again before another call is made to decode_frame, which then
goes down the call stack to decode_slice_header, where the null dereference
is made.

However, neither ffmpeg nor ffplay will call avcodec_decode_video2 if
avpkt->size==0 See ffmpeg.c:2246 (the while loop in output_packet) for the
conditional in ffmpeg.

The audio desync is still a mystery to me, though. 

Best

/P