Opened 10 months ago

Closed 5 weeks ago

#271 closed defect (fixed)

Motion vector rounding: tiny contradiction between N0335 integration and the meeting decision

Reported by: tim.solovyev Owned by:
Priority: minor Milestone:
Component: spec Version: VVC D7 vE
Keywords: Motion vector rounding, N0335, round toward zero Cc: ksuehring, bbross, XiangLi, fbossen, jvet@…

Description

In Geneva it was decided to use rounding towards zero for motion vectors everywhere:

“…
N0085/N0335: unified MV rounding by setting the rounding offset to (1 << (shift – 1)) − 1 for rounding applied to an absolute value, to round toward zero.

Notes of Track B review:

  • At the Macao meeting, based on JVET-L0377 we made the software match the text by rounding toward zero rather than toward minus infinity (at least for AMVR).
  • Then at the Marrakech meeting, based on JVET-M0265, we tried to consistently apply rounding away from zero.
    • Offset = 1 << (F-1);
    • M= S >= 0 ? (S + Offset) >> F: -((-S + Offset) >> F).
  • It was said that for deriving chroma vectors from luma vectors, it is necessary to round toward negative infinity for IBC to avoid a range problem. This is an exception.
  • For temporal scaling in HEVC, it was said that we rounded toward zero. Some (tiny) gain was shown for doing that.
  • Decision (consistency): Round MVs toward zero for MV rounding (incl. temporal scaling and spatial neighbour MV scaling and other cases if applicable). Use the software variation from M0335.

…“

In my understanding, according to the meeting notes, rounding should be performed as following:

  • Offset = (1 << (F-1)) - 1;
  • M= S >= 0 ? (S + Offset) >> F: -((-S + Offset) >> F).

It also corresponds to HEVC rounding in TMVP (that was mentioned in meeting notes), e.g.:

"mvLXCol = Clip3( −32768, 32767, Sign( distScaleFactor * mvCol ) * ( ( Abs( distScaleFactor * mvCol ) + 127 ) >> 8 ) ) (8-209)"


After N0335 integration Draft text have following (section 8.5.5.9 Derivation process for motion vector arrays from affine control point motion vectors):

  • mvAvgLX[ 0 ] = ( mvAvgLX[ 0 ] + 1 − ( mvAvgLX[ 0 ] >= 0 ) ) >> 1 (8-713)
  • mvAvgLX[ 1 ] = ( mvAvgLX[ 1 ] + 1 − ( mvAvgLX[ 1 ] >= 0 ) ) >> 1 (8-714)

It is another rounding method that was proposed in N0335, but not the method that was finally adopted, according to the meeting notes.

In my understanding, these formulas should look like:

  • mvAvgLX[ 0 ] = mvAvgLX[ 0 ] >= 0 ? mvAvgLX[ 0 ] >> 1 : −( ( −mvAvgLX[ 0 ] ) >> 1 ) (8-713)
  • mvAvgLX[ 1 ] = mvAvgLX[ 1 ] >= 0 ? mvAvgLX[ 1 ] >> 1 : −( ( −mvAvgLX[ 1 ] ) >> 1 ) (8-714)

Section 8.5.5.9 is not the only place with this problem. It is just one example.

This problem first occurs in VVC D5 v3.

Change history (14)

comment:1 Changed 10 months ago by tim.solovyev

Of course, the results of two formulas could be bitexact (that's why I call the ticket "tiny contradiction"). The question here is which form is better for spec: with shift of negative value or without it?

E.g. for software, shift for negative value is not a good thing (due to behavior could be undefined or implementation-defined). For the spec, I'm not very sure, whether formula with shift of negative value is absolutely clear or not.

comment:2 Changed 9 months ago by bbross

  • Version changed from VVC D5 v5 to VVC D5 v6

comment:3 Changed 9 months ago by bbross

  • Version changed from VVC D5 v6 to VVC D5 v7

comment:4 Changed 9 months ago by bbross

  • Version changed from VVC D5 v7 to VVC D5 v8

comment:5 Changed 8 months ago by bbross

  • Version changed from VVC D5 v8 to VVC D5 v9

comment:6 Changed 8 months ago by bbross

  • Version changed from VVC D5 v9 to VVC D5 v10

comment:7 Changed 7 months ago by bbross

  • Version changed from VVC D5 v10 to VVC D6 vD

comment:8 Changed 7 months ago by bbross

  • Version changed from VVC D6 vD to VVC D6 vE

comment:9 Changed 4 months ago by bbross

  • Version changed from VVC D6 vE to VVC D7 vC

comment:10 Changed 4 months ago by bbross

  • Version changed from VVC D7 vC to VVC D7 vD

comment:11 Changed 4 months ago by bbross

  • Version changed from VVC D7 vD to VVC D7 vE

comment:12 Changed 5 weeks ago by jlchen

In spec, there is explicit definition of "right shift" as follows:

x >> y Arithmetic right shift of a two's complement integer representation of x by y binary digits. This function is defined only for non-negative integer values of y. Bits shifted into the most significant bits (MSBs) as a result of the right shift have a value equal to the MSB of x prior to the shift operation.

Is the spec absolutely clear?

comment:13 Changed 5 weeks ago by zhangkai

To my understanding, the results of two kinds of formulas are exactly the same.

But I agree with Tim, the following formula is easier to be understood and matches with the meeting notes better.
Offset = (1 << (F-1)) - 1;
M= S >= 0 ? (S + Offset) >> F: -((-S + Offset) >> F).

In the spec, we only need to make some editorial changes to section 8.5.2.14

comment:14 Changed 5 weeks ago by jlchen

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.