Opened 4 years ago

Closed 4 years ago

#1184 closed defect (fixed)

Mismatch between spec and SW in MV rounding for affine AMVR with 1/16 precision

Reported by: chhuanb Owned by:
Priority: minor Milestone: VVC D10
Component: spec Version:
Keywords: Cc: ksuehring, bbross, XiangLi, fbossen, jvet@…

Description

The MV rounding process 8.5.2.14 is invoked during affine AMVP list construction (8.5.5.7 Derivation process for luma affine control point motion vector predictors) as following:

– The rounding process for motion vectors as specified in clause 8.5.2.14 is invoked with mvX set equal to cpMvpLX[ cpIdx ], rightShift set equal to AmvrShift, and leftShift set equal to AmvrShift as inputs and the rounded cpMvpLX[ cpIdx ] with cpIdx = 0 .. numCpMv − 1 as output.

In case the CPMV precision for affine is 1/16, AmvrShift is set to 0 as Table 16.
Then the input rightShift and leftShift to 8.5.2.14 are both 0.
According to 8.5.2.14, in such case if mv >=0, the output mv will become mv-1 :

offset = ( rightShift = = 0 ) ? 0 : ( 1 << ( rightShift − 1 ) ) (599)
mvX[ 0 ] = ( ( mvX[ 0 ] + offset − ( mvX[ 0 ] >= 0 ) ) >> rightShift ) << leftShift (600)
mvX[ 1 ] = ( ( mvX[ 1 ] + offset − ( mvX[ 1 ] >= 0 ) ) >> rightShift ) << leftShift (601)

However, the mv is unchanged in SW.

Suggest align spec to SW as following:

8.5.2.14 Rounding process for motion vectors
Inputs to this process are
– the motion vector mvX,
– the right shift parameter rightShift for rounding,
– the left shift parameter leftShift for resolution increase.
Output of this process is the rounded motion vector mvX.
For the rounding of mvX, the following applies:

If rightShift == 0:

mvX[ 0 ] = mvX[ 0 ] << leftShift
mvX[ 1 ] = mvX[ 1 ] << leftShift

else:

offset = ( 1 << ( rightShift − 1 ) ) (599)
mvX[ 0 ] = ( ( mvX[ 0 ] + offset − ( mvX[ 0 ] >= 0 ) ) >> rightShift ) << leftShift (600)
mvX[ 1 ] = ( ( mvX[ 1 ] + offset − ( mvX[ 1 ] >= 0 ) ) >> rightShift ) << leftShift (601)

Change history (3)

comment:1 follow-up: Changed 4 years ago by fbossen

The text should indeed be fixed.
Alternatively it could be expressed as:
offset = ( rightShift = = 0 ) ? 0 : ( 1 << ( rightShift − 1 ) ) − 1 (599)
mvX[ 0 ] = sign( mvX[ 0 ] ) * ( ( ( abs( mvX[ 0 ] ) + offset ) >> rightShift ) << leftShift ) (600)
mvX[ 1 ] = sign( mvX[ 1 ] ) * ( ( ( abs( mvX[ 1 ] ) + offset ) >> rightShift ) << leftShift ) (601)

comment:2 in reply to: ↑ 1 Changed 4 years ago by chhuanb

Replying to fbossen:

The text should indeed be fixed.
Alternatively it could be expressed as:
offset = ( rightShift = = 0 ) ? 0 : ( 1 << ( rightShift − 1 ) ) − 1 (599)
mvX[ 0 ] = sign( mvX[ 0 ] ) * ( ( ( abs( mvX[ 0 ] ) + offset ) >> rightShift ) << leftShift ) (600)
mvX[ 1 ] = sign( mvX[ 1 ] ) * ( ( ( abs( mvX[ 1 ] ) + offset ) >> rightShift ) << leftShift ) (601)

This expression is more concise, and sign and abs should be repalced by Sign and Abs according to clause 5.8 Mathematical functions.

comment:3 Changed 4 years ago by jlchen

  • Resolution set to fixed
  • Status changed from new to closed

Thank to Huanbang for reporting the issue and Frank to confirm and suggest the solution. The fix will be included in JVET-S2001-vD release.

Note: See TracTickets for help on using tickets.