[FFmpeg-devel] [PATCH] libavcodec/zmbvenc.c: don't allow motion estimation out of range.

Michael Niedermayer michael at niedermayer.cc
Sun Dec 9 18:27:42 EET 2018


On Sat, Dec 08, 2018 at 02:53:44PM +0100, Tomas Härdin wrote:
> lör 2018-12-08 klockan 00:29 +0000 skrev Matthew Fearnley:
> > Hi Tomas, thanks for looking through my patch.
> > 
> > > > Practically, this patch fixes graphical glitches e.g. when reencoding
> > 
> > the
> > > > Commander Keen sample video with me_range 65 or higher:
> > > > 
> > > >     ffmpeg -i keen4e_000.avi -c:v zmbv -me_range 65 keen4e_me65.avi
> > > I'd expect this problem to pop up with -me_range 64 too, no?
> > 
> > I initially thought this would be the case, but taking the tx loop and
> > removing the edge constraints:
> > 
> >     for(tx = x - c->range; tx < x + c->range; tx++){
> >         ...
> >         dx = tx - x;
> > 
> > The effective range is (-c->range) <= dx < (c->range), meaning when
> > c->range = me_range = 64, the dx value ranges from -64 to 63, which happens
> > to be exactly in bounds.
> > So I could have just capped me_range to 64, and that would have fixed the
> > bug...
> > 
> > But more generally, I've concluded the '<' sign is a mistake, not just
> > because of the general asymmetry, but because of the way it prevents tx,ty
> > reaching the bottom/right edges.
> > In practice it means, for example, that if the screen pans left to right,
> > the bottom edge will have to match against blocks elsewhere in the image.
> > 
> > > I went over the patch, and it looks fine. But what's up with the xored
> > > logic? It seems like it would compute xored always from the bottom-
> > > right-most MV. The loop in zmbv_me() should probably have a temporary
> > > xored and only output to *xored in if(tv < bv)..
> > 
> > Hmm, you're right.  In most cases, the code actually works anyway - because
> > when *xored==0, the block entropy returned by block_cmp() is supposed to be
> > 0 anyway, so it still finishes then.
> > But... I've realised there are some exceptions to this:
> > - the entropy calculations in block_cmp() use a lookup table
> > (score_tab[256]), which assumes each block has 16*16 pixels.  This will
> > only be valid when the dimensions are a multiple of 16, otherwise the
> > bottom/right edge blocks may be smaller.
> > - I've just realised the lookup table only goes up to 255.  If all 16*16
> > xored pixels are the same value, it will hit the 256th entry!  (I'm
> > surprised valgrind hasn't found this..?)
> 
> Static analysis would've caught this, which is something I've harped on
> previously on this list (http://ffmpeg.org/pipermail/ffmpeg-devel/2018-
> June/231598.html)
> 
> > All that said, *xored==0 is actually the most desirable outcome because it
> > means the block doesn't need to be output.
> > So if *xored is 0, the best thing to do is probably to finish immediately
> > (making sure *mx,*my are set), without processing any more MVs.
> > And then it doesn't matter (from a correctness point of view, at least) if
> > block_cmp() gives bad entropy results, as long as *xored is set correctly.
> 
> Oh yeah, that's right. It might also be a good idea to try the MV of
> the previous block first for the next block, since consecutive blocks
> are likely to have the same MV. Even fancier would be to gather
> statistics from the previous frame, and try MVs in order of decreasing
> popularity. A threshold might also be useful, like "accept any MV that
> results in no more than N bits entropy"

looking at existing fancy motion estimation methods, for example variants of
predictive zonal searches. Might safe some time from redesigning a ME method
from scratch
also maybe something from libavcodec/motion_est.c can be reused


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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181209/8d5ba792/attachment.sig>


More information about the ffmpeg-devel mailing list