Opened 3 years ago

Closed 3 years ago

#71 closed defect (fixed)

Typos in VVC D2 v4

Reported by: vdrugeon Owned by: bbross
Priority: minor Milestone: VVC D2 v5
Component: spec Version: VVC D2 v4
Keywords: Cc: ksuehring, bbross, XiangLi, fbossen, jvet@…

Description

Here is a list of some more typos and small issues that I found:

In section 8.2.4.2.1, when generating the reference samples p, there is a typo in step 1 "the reference samples refUnfilt[ x ][ y ] with x = −1, y = −1 refH − 1 and x = 0..refW − 1, y = −1 as output" where the ".." are missing for y=-1..refH-1.

In section 8.2.4.2.2 "The refW + refH − 1 neighbouring samples refUnfilt[ x ][ y ] that are constructed samples prior to the in-loop filter process, with x = −1, y = −1..refH − 1 and x = 0..refW − 1, y = −1": there are refW + refH + 1 samples.

In section 8.2.4.2.3, the outputs are currently listed as one of the inputs. They should be listed separately.

In section 8.2.4.2.3 step 3 "the value of refUnfilt[ x ][ y ] is set equal to p the value of refUnfilt[ x − 1 ][ y ]", the "p" before the "the value of" should be removed.

In section 8.2.4.2.4 at the end when p[x][y] is derived, equation (8-23) sets pF[x][-1] instead of p[x][-1].

In section 8.2.2, cbWidth and cbHeight should be added as inputs to the process.

General question about the order in which inputs to a process are listed (it may not be important):
In some processes, an array is needed as input together with its size, as for example in process 8.2.4.2.3:
"
Inputs to this process are:
– reference samples refUnfilt[ x ][ y ] with x = −1, y = −1..refH − 1 and x = 0..refW − 1, y = −1 for intra sample prediction,
– a variable refW specifying the reference samples width,
– a variable refH specifying the reference samples height,
"
Would it make more sense to list the size of the array BEFORE listing the array? Otherwise the first input contains two variables (in the example above refH and refW) that are defined afterwards.

Change history (7)

comment:1 Changed 3 years ago by vdrugeon

In section 8.2.4.2.5, there is a mismatch between the size of the reference samples array p as declared in the input of the process "– the neighbouring samples p[ x ][ y ], with x = −1, y = −1..nTbH and x = 0..nTbW, y = −1" and when invoking the process in section 8.2.4.2.1 where p has size x = −1, y = −1..refH − 1 and x = 0..refW − 1, y = −1.
I wanted to suggest to fix this by adding refW and refH to the inputs and providing the correct size for the array p, but I realised that refW and refH are not actually needed for the intra planar prediction process. In addition, based on the definition of refH and refW, it is always true that refW > nTbW+1 and refH > nTbH+1. The mismatch in the text still exists though.
Do you think that this mismatch is serious? If yes, do you have any idea to solve it?

The issue applies to section 8.2.4.2.6 for DC prediction and section 8.2.4.2.8 for CCLM prediction as well.

comment:2 Changed 3 years ago by bbross

  • Milestone set to VVC D2 v5
  • Owner set to bbross
  • Status changed from new to assigned

These typos will be fixed in v5.

Regarding
In section 8.2.4.2.5, there is a mismatch between the size of the reference samples array p as declared in the input of the process "– the neighbouring samples p[ x ][ y ], with x = −1, y = −1..nTbH and x = 0..nTbW, y = −1" and when invoking the process in section 8.2.4.2.1 where p has size x = −1, y = −1..refH − 1 and x = 0..refW − 1, y = −1.

This is not a critical issues can can be dealt with later since DC, PLANAR and CCLM are only using a subset of the reference samples so reference sample array boundaries are not exceeded.
There a several ways to improve this, one way would be to define refW and refH additionally depending on the predMode and already derive just the subset of reference samples in case of these mode.

comment:3 Changed 3 years ago by vdrugeon

In section 8.2.4.1 step 2 after "– Otherwise, the following ordered steps apply" it is written "2. The scaling and transformation process as specified in clause 8.3.2 is invoked with the luma location ( xTbY, yTbY ) set equal to ( xTb0, yTb0 )". However, (xTb0, yTb0) is not necessarily a luma location in this process. But it seems that process 8.3.2 does require a luma location as input. In that correct?
If correct, then the input to process 8.3.2 needs to be modified and the luma location needs to be calculated as ( xTbY, yTbY ) = ( cIdx = = 0 ) ? ( xTb0, yTb0 ) : ( xTb0 << 1, yTb0 << 1 )

comment:4 Changed 3 years ago by vdrugeon

Two more potential issues below:

In section 7.4.5.7 describing the semantics of tu_cbf_cb and tu_cbf_cr, the array index trafoDepth is mentioned: "The array index trafoDepth specifies the current subdivision level of a coding block into blocks for the purpose of transform coding. trafoDepth is equal to 0 for blocks that correspond to coding blocks."
However tu_cbf_cb and tu_cbf_cr do not depend on this array index, but only on x0 and y0. Could this be a leftover from the HEVC text?

In the syntax of residual coding in section 7.3.4.8, the role of the variable inferSbDcSigCoeffFlag is unclear. Should it be that the value of the DC coefficient in the sub-block is inferred to be known, as in the HEVC standard? In this is correct, why are these lines not added after signalling sig_coeff_flag[xC][yC], as in HEVC: if( sig_coeff_flag[ xC ][ yC ] ) inferSbDcSigCoeffFlag = 0 ?
In the current syntax, it seems to me that, for a sub-block with coded_sub_block_flag equal to 1 that is neither the first sub-block nor the sub-block that contains the last significant coefficient, the value of sig_coeff_flag[0][0] will always be inferred to be equal to 1. Is that the intended behaviour?

comment:5 Changed 3 years ago by vdrugeon

Section 9.3.3.2 Rice parameter derivation process
The last line of the pseudo code is:
locSumAbsPass1 += AbsLevelPass1 [ xC ][ yC + 2 ] − sig_coeff_flag[ xC ][ yC + 2 ]
However, locSumAbsPass1 is never initialised, and also never used. Is this a typo? Should it be locSumAbs instead? If it is a typo, is AbsLevelPass1 also a typo? Should it be the following instead (which seems more logical to me):
locSumAbs += AbsLevel [ xC ][ yC + 2 ] − sig_coeff_flag[ xC ][ yC + 2 ]

comment:6 Changed 3 years ago by vdrugeon

At the end of the residual_coding syntax, there is a condition for the presence of mts_idx[x0][y0]: ( CuPredMode[ x0 ][ y0 ] = = MODE_INTRA && numSigCoeff > 2 )
But numSigCoeff seems not to be defined anywhere.

comment:7 Changed 3 years ago by bbross

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

fixed in D2 v5

Note: See TracTickets for help on using tickets.