Motion vector clipping in vp9_find_mv_refs(), vp9_append_sub8x8_mvs_for_idx() and vp9_dec_build_inter_predictors_sb() is broken.

[GoogleCodeExporter]

So, I'm going to give a handful of examples of stuff that is broken, and then 
I'll conclude with a generic suggestion for fixing the more general concept of 
bugs pointed out here. You should feel free to fix it in another way if you 
feel that's practically doable, as long as the whole class of bugs disappears.

One example is clipping interplay between the two aforementioned functions. 
find_mv_refs_idx() looks for two unique motion vector references, then clips 
them, and returns these to the caller. If the caller is sub8x8, it will _again_ 
do a uniqueness check and if the two MVs are identical, it will return _zero_, 
as long as we're not the topleft sub8x8 block. This leads to really obscure 
situations. If we are the bottomleft/topright sub8x8 block, find_ref_mvs() had 
multiple MVs available as potential references, but the first two happened to 
be identical after clipping (and identical to the topleft sub8x8 one), then we 
indeed return zero as a MV, rather than another candidate or the first 
candidate.

Another example: if we had no MV references available (for whatever reason) and 
we're in a block that's partially off-screen, the clipping will cause us 
find_ref_mvs_idx() to return a _non_-zero MV (i.e. the clip changes the zero MV 
to a non-zero MV). This is exposed in e.g. https://trac.ffmpeg.org/ticket/3849

The clipping is particularly obscure when you compare sub8x8 with regular 
blocks. The idiotic thing is that the clipping in sub8x8 blocks is applied to 
the 8x8 block dimensions and position, not the sub8x8 (e.g. 4x4) 
block/position. Although then, honestly, we're not talking about 
mvref_common.[ch] anymore, but rather vp9_dec_build_inter_predictors_sb() in 
vp9_decodeframe.c, which does its own clipping (which is also broken).

Speaking of that function, when we look at scalable references, the MV clamping 
is done to pre-scaled motion vectors, which means that the prediction actually 
can change as a result of the clip (which should not ever happen).

My conclusion is that the clipping of MVs in the aforementioned functions is 
broken. I think it should all be deleted in vp10. That is, the codebase should 
not have any functions called clamp_mv(), at least not in a v1 release. I'm 
fine with them _down the road_ as optimizations, but they should be verified to 
not affect any portion of the bitstream, and the pre-clamp result should be the 
golden. Right now I'm just chasing the clippings that libvpx does in random 
places to get the same result.

Original issue reported on code.google.com by rsbul...@gmail.com on 4 Jun 2015 at 11:38

[GoogleCodeExporter]

Original comment by fgalli...@google.com on 18 Jun 2015 at 10:08

• Changed state: Available

[GoogleCodeExporter]

To be fixed in VP10.

Original comment by ya...@google.com on 18 Jun 2015 at 11:45

• Added labels: Type-Enhancement
• Removed labels: Type-Bug