xiph / rav1e

The fastest and safest AV1 encoder.
BSD 2-Clause "Simplified" License
3.72k stars 253 forks source link

Question about PlaneOffset in half-pixel motion estimation #1171

Open rom1v opened 5 years ago

rom1v commented 5 years ago

In estimate_motion_ss2(), the half-res block offset is half of the full-res block offset:

https://github.com/xiph/rav1e/blob/265889fae2d3250b53e824ee868863f7455823a3/src/me.rs#L400

and the half-res plane offset is half of the full-res plane offset:

https://github.com/xiph/rav1e/blob/265889fae2d3250b53e824ee868863f7455823a3/src/me.rs#L401-L404

But if one of the block offset components is odd, then the computed half-res plane offset is not the same as the plane offset of the half-ref block offset.

For example:

let bo_adj = BlockOffset { x: 3, y: 5 };

Then:

let bo_adj_h = BlockOffset { x: 1, y: 2 };

And:

let po = PlaneOffset { x: 6, y: 10 };

However, the plane offset associated to bo_adj_h is:

let po_adj_h = PlaneOffset { x: 4, y: 8 };

Which plane offset is correct? The current version, or the one associated with bo_adj_h?

diff --git a/src/me.rs b/src/me.rs
index 14c64375..c3b68f16 100644
--- a/src/me.rs
+++ b/src/me.rs
@@ -399,8 +399,8 @@ pub trait MotionEstimation {
       let bo_adj = adjust_bo(bo, fi, blk_w, blk_h);
       let bo_adj_h = BlockOffset{x: bo_adj.x >> 1, y: bo_adj.y >> 1};
       let po = PlaneOffset {
-        x: (bo_adj.x as isize) << BLOCK_TO_PLANE_SHIFT >> 1,
-        y: (bo_adj.y as isize) << BLOCK_TO_PLANE_SHIFT >> 1
+        x: (bo_adj_h.x as isize) << BLOCK_TO_PLANE_SHIFT,
+        y: (bo_adj_h.y as isize) << BLOCK_TO_PLANE_SHIFT,
       };
       let (mvx_min, mvx_max, mvy_min, mvy_max) = get_mv_range(fi.w_in_b, fi.h_in_b, bo_adj, blk_w, blk_h);
ycho commented 5 years ago

FYI, the output of adjust_bo( ) function, i.e. bo_adj, should be always even numbers, because the width and height is always multiple of 8, so the BlockOffset is always even numbers.

rom1v commented 5 years ago

Oh yes. Thank you :+1: That explains why there is absolutely no difference in practice.

rom1v commented 5 years ago

Hmmm, in fact, I'm not convinced yet. adjust_bo() may return the very same block passed as argument (if it's already within bounds).

ycho commented 5 years ago

Another tip with adjust_bo( ) is that, it only alters the input bo when it is close to right and bottom frame border. Precisely, when 8x8 block starting with bo is straddle on the right or bottom frame border.

rom1v commented 5 years ago

Yes, so if input bo has an odd component, the output bo may also have an odd component, am I correct?

ycho commented 5 years ago

Hmm, you are right and when the blk_size is 4, either side, offset is odd number. Also, fn adjust_bo(bo: BlockOffset, fi: &FrameInvariants, blk_w: usize, blk_h: usize), there is blk size, which plays as condition for clipping. So, my previous words of 8x8 was a LIE! :)