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

[Reimar Döffinger]

On Sun, Apr 29, 2012 at 11:09:12PM +0200, Michael Niedermayer wrote:
> On Sun, Apr 29, 2012 at 10:09:52PM +0200, Reimar Döffinger wrote:
> > 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).
> 
> the picture size is rounded up first then the edge is added
> thus there should be more space at the bottom then top.

Sure. I don't see what that has to do with anything though.
We do not need the edge pixels, so those 16 edge pixels on top
should be enough to compensate for the missing alignment.
With still the caveat that I might miss something. For the encoder
there might be an issue if someone sets interlaced dct, which
will lead to a broken encode anyway though I think.
I wouldn't object to anyone adding code to just make a copy
of the frame to be extra safe though (and get rid of the
annoying !EMU_EDGE requirement).