[FFmpeg-devel] [PATCH] Fix nonsense non-mod16 AMV flipping code.

[Reimar Döffinger]

On Sun, Apr 29, 2012 at 09:45:42PM +0200, Michael Niedermayer wrote:
> On Sat, Apr 28, 2012 at 11:37:08PM +0200, Reimar Döffinger wrote:
> > It is obviously nonsense since it produces wrong results
> > or even crashes (crashes should be encode-only though).
> > Fixes trac issue #1092.
> > 
> > Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> > ---
> >  libavcodec/mjpegdec.c      |    3 +--
> >  libavcodec/mjpegenc.c      |    2 +-
> >  libavcodec/mpegvideo_enc.c |    5 -----
> >  3 files changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > index a841384..fe54b45 100644
> > --- a/libavcodec/mjpegdec.c
> > +++ b/libavcodec/mjpegdec.c
> > @@ -972,8 +972,7 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
> >          s->coefs_finished[c] |= 1;
> >          if (s->flipped) {
> >              // picture should be flipped upside-down for this codec
> > -            int offset = (linesize[c] * (s->v_scount[i] *
> > -                         (8 * s->mb_height - ((s->height / s->v_max) & 7)) - 1));
> > +            int offset = linesize[c] * (s->v_scount[c] * s->height / s->v_max - 1);
> 
> this will draw above the image i think, is this ok for all MB sizes
> and interlacing ?

Well, at least not more than before, and the tests I made looked ok.
But I can't tell for sure. I believe the way this is supposed to work
is like this:
Since EMU_EDGE may not be set, we have 16 pixels extra all around the
picture. These we use to both flip and expand the image (since the codec
is intra-only they aren't really needed for anything else).
A special check for AMV will ensure we do not use the edge expansion
code that would require those extra pixels (necessary since we already
used them otherwise).
However I certainly won't guarantee that one CODEC_ID_AMV test catches
all cases that would ordinarily use extra edge pixels.
Plus, I believe it would make more sense for that check to test for
intra_only instead of AMV specifically, it's a waste of processing
power to fill in edge pixels that are guaranteed to never be used...