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

[Reimar Döffinger]

On Thu, Apr 09, 2009 at 10:00:00PM +0200, Michael Niedermayer wrote:
> On Thu, Apr 09, 2009 at 09:51:56PM +0200, Reimar D?ffinger wrote:
> > On Thu, Apr 09, 2009 at 09:03:31PM +0200, Michael Niedermayer wrote:
> > > On Thu, Apr 09, 2009 at 07:46:19PM +0200, Reimar D?ffinger wrote:
> > > > 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
> > 
> > Do I need to flip the sign of the mvs or is that correct as-is (well, in
> > one of those cases those mvs are 0 anyway).
> 
> you should use the other MV, MBs have 2 (its likely a [0] that should be [1])

You mean e.g.
s->current_picture.motion_val[0][ mb_x*2 + mb_y*2*s->b8_stride ][0];
should be
s->current_picture.motion_val[dir][ mb_x*2 + mb_y*2*s->b8_stride ][0];
With dir = !s->last_picture.data[0]; ?

> > @@ -850,6 +850,12 @@
> >      /* set unknown mb-type to most likely */
> >      for(i=0; i<s->mb_num; i++){
> >          const int mb_xy= s->mb_index2xy[i];
> > +        // change inter with no references to intra
> > +        if (!IS_INTRA(s->current_picture.mb_type[mb_xy]) &&
> > +            !s->last_picture.data[0] && !s->next_picture.data[0]) {
> > +            s->current_picture.mb_type[mb_xy]= MB_TYPE_INTRA4x4;
> > +            continue;
> > +        }
> >          error= s->error_status_table[mb_xy];
> >          if(!((error&DC_ERROR) && (error&MV_ERROR)))
> >              continue;
> 
> is_intra_more_likely() should return 1
> and that should cause all to be set to intra

Well, but that code will only run if there is either either a DC or a MV
error (see above), whereas the problematic code will only run if there
is no MV error. So if there is only an AC error I think the code I added
above would be necessary I think...