Opened 4 years ago

Closed 3 years ago

#1435 closed defect (invalid)

Mismatch on ScalingMatrixRec derivation in eq.(105) between Spec and SW

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

Description

In the spec (JVET-S2001vH), ScalingMatrixRec derivation is defined as in eq. (105):

ScalingMatrixRec[ id ][ x ][ y ] = ( ScalingMatrixPred[ x ][ y ] + ScalingList[ id ][ k ] ) & 255 (105)

with k = 0..( matrixSize * matrixSize − 1 ),

x = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 0 ], and
y = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 1 ]

However, in VTM-10.2, ScalingMatrixRec derivation is implemented as below:

VLCReader.cpp void HLSSyntaxReader::decodeScalingList(...)
{
... 
  for(i = 0; i < coefNum; i++)
  {
    ...
    READ_SVLC( data, "scaling_list_delta_coef");
    nextCoef += data;
    predCoef = (isPredictor) ? srcPred[scan[i].idx] : 0;
    dst[scan[i].idx] = (nextCoef + predCoef + 256) & 255;
  }
...
}

, where the parameter nextCoef, predCoef and dst[scan[i].idx] in code are corresponding to ScalingList[id][k], ScalingMatrixPred[x][y], and ScalingMatrixRec[id][x][y] in the spec.

As can be seen, there is "+256" in SW but not in Spec.

Looking at the adopt text JVET-P1034, "+256" is found in the syntax table.

...
scaling_list_delta_coef se(v)
nextCoef = ( nextCoef + scaling_list_delta_coef + 256 ) % 256
ScalingList[ scalingListId ][ i ] = nextCoef
...

Or looking at JVET-P2001vC, "+256" is found in ScalingMatrixRec derivation as below.

ScalingMatrixRec[ id ][ x ][ y ] = ( ScalingMatrixPred[ x ][ y ] + ScalingList[ id ][ k ] + 256 ) % & 256 ) (7 72)

with k = 0..( matrixSize * matrixSize − 1 ),

x = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 0 ], and
y = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 1 ]

But text clean-up after JVET-P2001vD, "+256" was deleted.

Suggested fix is to align Spec to SW.

ScalingMatrixRec[ id ][ x ][ y ] = ( ScalingMatrixPred[ x ][ y ] + ScalingList[ id ][ k ] + 256) & 255 (105)

with k = 0..( matrixSize * matrixSize − 1 ),

x = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 0 ], and
y = DiagScanOrder[ Log2( matrixSize ) ][ Log2( matrixSize ) ][ k ][ 1 ]

Change history (3)

comment:1 Changed 3 years ago by fbossen

A & 255 is the same as (A + 256) & 255
A % 256 is not the same as (A + 256) % 256

When using % 256 one needs to be careful to work with positive numbers. However, there is no such issue with & 255.

I don't see an issue here.

comment:2 Changed 3 years ago by tsukuba.takeshi

I agree that there is no issue.

comment:3 Changed 3 years ago by yk

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