Mismatch between SW and Spec on PDPC to angular intra modes

[huayg]

A spec error is found in Clause 8.4.5.2.14 on PDPC to angular intra modes. SW is correct.

Eq (8-258) should be changed from

wT[ y ] = 32  >>  ( ( y  <<  1 )  >>  nScale )

to

wT[ y ] = ( y < ( 3 << nScale ) )  ?  32  >>  ( ( y  <<  1 )  >>  nScale ) : 0

Eq (8-265) should be changed from

wL[ x ] = 32  >>  ( ( x  <<  1 )  >>  nScale )

to

wL[ x ] = ( x < ( 3 << nScale ) )  ?  32  >>  ( ( x  <<  1 )  >>  nScale ) : 0

(Note that a related typo error in Eq (8-262) has been reported in Ticket #479.)

[fbossen]

I don't think there is a mismatch here.

Note that 32 >> ( ( y << 1 ) >> nScale ) is equal to 0 for values of y >= 3 << nScale

Indeed 32 >> ( ( 3 << nScale ) << 1 ) >> nScale )
is equal to 32 >> ( ( 3 << ( nScale + 1 ) ) >> nScale )
is equal to 32 >> ( 3 << ( nScale + 1 - nScale) )
is equal to 32 >> ( 3 << 1 )
is equal to 32 >> 6 which is 0

The issue is that in software a >> b is undefined if a is a n-bit value and b is equal to or larger than n (from the C++ spec: "The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand"). Since (y << 1) >> nScale may be equal to or larger than 32, something needs to be done in SW to avoid this issue, which is why you see a condition such as y < (3 << nScale). This condition is present in SW to address a limitation of C/C++ and also to avoid making unnecessary computations, but is not intended to be normative.

It is not crystal clear how the >> operator is supposed to behave in the VVC specification. There is a note in section 5.1 that says "The mathematical operators used in this Specification are similar to those used in the C programming language". Section 5.10 doesn't really say anything about bit width of variables. My assumption has been that variables in the VVC specification have no bit width limitation unless otherwise specified. I therefore think that the undefined behaviour cases defined by C/C++ for the '>>' operator do not apply to the VVC specification.

Therefore, there shouldn't be a need to modify the equations as suggested.

[huayg]

I see. Thank you very much for the detailed explanation.

The next question then would be:

since when y >= ( 3 << nScale ), 32 >> ( ( y << 1 ) >> nScale ), i.e. wT[x], is equal to 0, there seems no need in Eq (8-257) on refT[x][y] to explicitly include the condition check of ( y < ( 3 << nScale ) ), as mainRef[dX[ x ][ y ]]*wT[x] would be equal to 0 anyway.

So, should we remove the ( y < ( 3 << nScale ) ? and ( x < ( 3 << nScale ) ? condition check from Eq (8-257) and Eq (8-262), respectively?

[fbossen]

Not sure about changing equations (8-257) and (8-262). If you remove the y < (3 << nScale) condition, refT[x][y] could be undefined because dX[x][y] may be larger than refW - 1 if y >= (3 << nScale) and mainRef[z] is not defined for values z >= refW - 1. An undefined value of refT[x][y] times wT[y] may be considered to be 0 if wT[y] is known to be 0 in such case, but I believe it is cleaner to not have undefined intermediate variables. I would thus not remove the condition in these equations.

[huayg]

Given the related PDPC restriction on predModeIntra to be "less than INTRA_ANGULAR18" and (x,y) being just a position inside the current block, dX[x][y] should never be larger than refW - 1, and should always yield a valid value. This should already be guaranteed by the refW definition in Clause 8.4.5.2.5 and the related angular intra mode definition in Clause 8.4.5.2.12. (And as far as I can see, it is the case with the current Spec.)

In case this is not true, meaning that some other bug exists in the Spec and/or SW, then it is the newly identified bug that should be fixed, but not to put some unnecessary condition check explicitly and permanently in the Spec to prevent such potential bugs, if any.

[fbossen]

You say that "dX[x][y] should never be larger than refW - 1". I think that needs to be qualified: dX[x][y] should never be larger than refW - 1 when mainRef[dX[x][y]] is evaluated. Note that the ternary operator x ? y : z used in equation (8-257) is defined as follows in section 5.3: If x is TRUE or not equal to 0, evaluates to the value of y; otherwise, evaluates to the value of z.

Also note that in older drafts of the spec (from before the last meeting), we had refT[ x ][ y ] = ( dX[ x ][ y ] < refW − 1 ) ? mainRef[ dX[ x ][ y ] + ( dXFrac[ y ] >> 5 ) ] : 0. That seems to indicate that dX[x][y] never being larger than refW - 1 isn't a design goal.

I really don't see any technical issue here that calls for a fix (other than what was previously reported in #479 and #489).

I am closing this ticket since I believe we agree that the mismatch between SW and spec initially reported doesn't exist. Please open a new ticket if you believe there is a bug in either text or SW.