Editorial modifications from JVET-P0626 create reference to undefined value

[fbossen]

JVET-P0626 modifies the range of values to which equations (8-134) and (8-144) are applied. With this modification, equations (8-138) and (8-148) can reference undefined values.

For example when nTbW = 8, nTbH = 16, refIdx = 0 and intraPredAngle = 16, equation (8-138) may refer to ref[18]. This happens when x = 7, y = 15:
From equation (8-135) iIdx = 16 * 16 >> 5 = 8
In equation (8-138) x + iIdx + i = 7 + 8 + 3 = 18 which is used to index ref[]

However, equation (8-134) defines ref[i] only up to i=17.
The largest index for which ref[] is defined is refW + refIdx + x = 17, where refW = 2 * 8 = 16, refIdx = 0, and x = 1 (was x = 2 before JVET-P0626 modifications)

It is suggested to revert the JVET-P0626 modifications to avoid referencing undefined values (even though they end up being multiplied by 0).

The application of the JVET-P0626 modifications in SW have led to bug report #635. The modifications are thus also proposed to be reverted in SW.

[afilippov]

I guess two different things are mixed up here, namely (1) removal of redundant padding operations and (2) access to uninitialized memory.

1. JVET-P0626 as indicated in its title "Non-CE3: Cleanup of reference sample padding for intra prediction" addresses the problem of redundant padding operation that existed in the VVC spec draft before incorporating JVET-P0626. Actually, there are no reasons to force implementers to set sample ref[ refW + refIdx + ( Max( 1, nTbW / nTbH ) * refIdx + 2 ] to any specific value (e.g., as previously defined by formula (8-134)). Even if this reference sample is used (however, it is implementation-dependent as shown below), it may take an arbitrary value since it is further multiplied by 0. Thus, JVET-P0626 gives more implementation freedom with minimal spec changes.

2. In my opinion, the problem of accessing to uninitialized memory is not about reference sample values. It is about how to handle interpolation filter coefficients equal to 0. The bug reported in ticket #635 can be fixed by using different ways. Instead of performing one more (actually, redundant) padding operation, at least, 2 other options can be considered:

• don't access reference sample if the last filter coefficient is 0 (as done in the fix for ticket #108 and as used in VTM-3/4/5);
• allocate and initialize reference sample buffers before encoding/decoding starts as done in merge request ​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/merge_requests/1096.

In my opinion, the solution in merge request ​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/merge_requests/1096 is the simplest and most straightforward one as

• no extra operations are carried out;
• the reference sample buffers refAbove[] and refLeft[] are preallocated when encoder / decoder starts up. Hence, they shouldn't be reallocated per each directionally predicted block as currently done in VTM SW.

Thus, I don't think JVET-P0626 itself introduces any issues with memory access. Ticket #635 is just a software integration bug that could be easily fixed (valgrind check passed) by initializing left and above reference sample buffers before encoding/decoding. Thus, it is suggested to

1. keep JVET-P0626 modifications in the VVC spec draft;
2. accept merge request !1096 that fixes the bug reported in ticket #635.

[fbossen]

This report is intended to be about the spec.

The non-normative change suggested in JVET-P0626 introduces a reference to an undefined value. This undefined value is then multiplied by 0.

The main question here is whether we are happy with the spec having such quirks.

[afilippov]

Since multiplication by 0 is well defined, to refer to this reference sample (ref[18] in your example) or not is just an implementation question that can be handled in different ways. Thus, to me, the reported issue is irrelevant to the spec as a result of equation (8-138) is always defined and doesn't depend on a value of this reference sample (ref[18] in your example) that may take an arbitrary value which is called "undefined" in your report.

JVET-P0626 is aimed at removing the redundant operations that forced implementers to perform an extra padding operation. If JVET-P0626 is not applied, it's unclear why the reference sample (ref[18] in your example) should be set to p[ −1 + refW ][ −1 − refIdx ], although any value can be used instead. Thus, the idea to define a variable that can take an arbitrary value without any effect on a result is not quite clear to me.

I don't see any spec related issues. The SW bug mentioned at the end of your report is fixed in the merge request ​​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/merge_requests/1096 with JVET-P0626 inside.

[fbossen]

We're talking about an editorial issue here. It has no impact on what implementers can and cannot do.

[afilippov]

We're talking about an editorial issue here.

According to my understanding, there is no editorial issue here:

1. In equation (8-138), coefficient value precedes the reference sample value: ∑ fT[ i ]*ref[ x+iIdx+i ]
2. Expressions are evaluated from left to right. Hence, fT[i] is evaluated first.

3. In equation (8-138), when i=3, f[ 3 ] is zero, since the subsample offset p = (intraPredAngle*(15+1)) % 32 = (256 % 32) = 0.

4. When fT[i] is 0, there is no need to refer to ref[ x+iIdx+i ], taking into account a well-known property of multiplication: 0 * x =0. Multiplication operation is defined in "5.2. Arithmetic operators".

5. Hence, it is absolutely unnecessary to refer to ref[x + iIdx + i] = ref[7 + 8 + 3] = ref[ 18 ].

If the order of a multiplier and a multiplicand in (8-138) had been opposite, it wouldn't be that crystal clear. But the current spec with JVET-P0626 is absolutely unambiguous.

It has no impact on what implementers can and cannot do.

I am not very sure about that. The VVC spec. draft before incorporating JVET-P0626 prescribed to set ref[ refW + refIdx + Max( 1, nTbW / nTbH ) * refIdx + 2 ] to specific value p[ −1 + refW ][ −1 − refIdx ]. But in fact, it could take any value in contrast to all other reference samples. However, there were no any clarifiction notes in the spec about that. JVET-P0626 removed this unreasonable restriction that could mislead implementers.

[fbossen]

Some operators such as && and || guarantee that the second operand is not evaluated if the first one takes a given value. This is not the case for operators such as &, |, * and /.

What you are describing is fT[ i ] == 0 ? 0 : ft[ i ] * ref[ x + iIdx + i ]

[afilippov]

I think spec- and implementation-related questions are mixed up again.

Although there is an informative note in Section 5.1 that "the mathematical operators used in this Specification are similar to those used in the C programming language", I guess implementation-related aspects of the C programming language are irrelevant to the VVC spec draft. Since the VVC spec draft clearly states that "only the externally observable output behaviour is required to conform to the specifications of this Recommendation | International Standard", it is enough for the spec to guarantee that output is always specified. Outputs of all the operations and the entire process described in Section 8.4.5.2.12 are well defined and unambiguous since the C programming language standard clearly defines that "the result of the binary * operator is the product of the operands". The output value of 0 * x is 0 for any x. Thus, output behaviour of fT[ i ] * ref[ x + iIdx + i ] is well defined for any value of ref[ x + iIdx + i ] if fT[ i ] == 0. Since the expression fT[ i ] * ref[ x + iIdx + i ] is evaluated from left to right, it is absolutely unnecessary to refer to ref[ 18 ] in your example if the output value of 0 * ref[ x + iIdx + i ] is known without ref[ x + iIdx + i ]. At least, I couldn't find any relevant constraints / requirement in the VVC spec draft that could force a VVC spec reader to evaluate x in the expression 0 * x. Thus, I don't see any issues in the VVC spec draft with JVET-P0626 incorporated. By the way, using the expression (fT[ i ] == 0) ? 0 : fT[ i ] * ref[ x + iIdx + i ] in formulae (8-138) and (8-148) could exclude any misunderstandings if you see any.

On the other hand, the version of the VVC spec draft before incorporating JVET-P0626 restricted implementation freedom as the value of the reference sample ref[ refW + refIdx + ( Max( 1, nTbW / nTbH ) * refIdx + 2 ] was set to p[ −1 + refW ][ −1 − refIdx ] although ref[ refW + refIdx + ( Max( 1, nTbW / nTbH ) * refIdx + 2 ] can actually take an arbitrary value. Undoubtedly, the result of multiplying 0 by ref[ refW + refIdx + ( Max( 1, nTbW / nTbH ) * refIdx + 2 ] is much more obvious for VVC spec readers than the fact that assigning p[ −1 + refW ][ −1 − refIdx ] to the reference sample ref[ refW + refIdx + ( Max( 1, nTbW / nTbH ) * refIdx + 2 ] is actually optional although this operation is prescribed by the spec.