[FFmpeg-devel] [PATCH] fix H.264 crash (issue 706)

[Michael Niedermayer]

On Thu, Apr 09, 2009 at 07:46:19PM +0200, Reimar D?ffinger wrote:
> On Thu, Apr 09, 2009 at 07:24:19PM +0200, Michael Niedermayer wrote:
> > On Thu, Apr 09, 2009 at 12:17:43PM +0200, Reimar D?ffinger wrote:
> > > Hello,
> > > it is quite possible this just hides the real issue, but on the other hand it
> > > looks like a simple and not too ugly fix to me.
> > > Comments?
> > 
> > the fix is definitly not correct
> > a correct fix very likely would be in the error concealment code,
> > basically it should throw away motion vectors to non existing frames
> > or make sure they exist
> 
> Is attached patch what you had in mind? I am not sure if the B-frames
> case is so good, I guess it might be a bit asymmetric if only one of
> the reference frames is missing.
> There is one more case that sets mv_dir, in the lower part of guess_mv,
> but since there already is an
> > assert(s->last_picture_ptr && s->last_picture_ptr->data[0]);
> I assumed that one will not need any changes, I did not actually verify
> that though...

> Index: libavcodec/error_resilience.c
> ===================================================================
> --- libavcodec/error_resilience.c	(revision 18389)
> +++ libavcodec/error_resilience.c	(working copy)
> @@ -343,6 +343,7 @@
>  
>                  if(IS_INTRA(s->current_picture.mb_type[mb_xy]))  continue;
>                  if(!(s->error_status_table[mb_xy]&MV_ERROR)) continue;
> +                if(!s->last_picture.data[0]) continue;
>  
>                  s->mv_dir = MV_DIR_FORWARD;
>                  s->mb_intra=0;

this should use MV_DIR_BACKWARD i suspect instead of skiping it completely


> @@ -870,6 +871,7 @@
>              if(IS_INTRA(mb_type)) continue; //intra
>              if(error&MV_ERROR) continue;              //inter with damaged MV
>              if(!(error&AC_ERROR)) continue;           //undamaged inter
> +            if(!s->last_picture.data[0]) continue;
>  
>              s->mv_dir = MV_DIR_FORWARD;
>              s->mb_intra=0;

same


> @@ -908,8 +910,11 @@
>                  if(IS_INTRA(mb_type)) continue;
>                  if(!(error&MV_ERROR)) continue;           //inter with undamaged MV
>                  if(!(error&AC_ERROR)) continue;           //undamaged inter
> +                if(!s->last_picture.data[0] && !s->next_picture.data[0]) continue;

that should not happen, i mean mb_typeshould be intra if there are no reference
frames


>  
>                  s->mv_dir = MV_DIR_FORWARD|MV_DIR_BACKWARD;
> +                if(!s->last_picture.data[0]) s->mv_dir &= ~MV_DIR_FORWARD;
> +                if(!s->next_picture.data[0]) s->mv_dir &= ~MV_DIR_BACKWARD;

ok

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

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20090409/9be6e8cc/attachment.pgp>