Opened 2 years ago
Closed 22 months ago
#1564 closed defect (fixed)
Intra prediction ref pixel array bounds too small for wide angle
Reported by: | swarrington | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | spec | Version: | VVC D10 vH |
Keywords: | Cc: | ksuehring, bbross, XiangLi, fbossen, jvet@… |
Description
It appears that the range of the ref[] array specified in eq.332 contains insufficient samples for generating the prediction plane in the case of some rectangular regions. It seems it may be an equation error in the max bounds specified in eq.332.
Consider:
nCbW = 64
nCbH = 4
nTbW = 32
nTbH = 4
cIdx = 0
IntraSubPartitionsSplitType = ISP_NO_SPLIT
refIdx = 0
Unmapped predModeIntra = 13
- eq.304 refW = nTbW*2 = 64
- 8.4.5.2.7: predModeIntra modified to 78 (whRatio= 5-2=3, predModeIntra < 8+2*whRatio, so predModeIntra += 65 ==> 78)
- eq.332: max index for ref[] is refW+refIdx+x
- max x index is (Max(1,nTbW/nTbH)*refIdx+1)=Max(1,32/4)*0+1=1
- ref is populated up to refW+refIdx+x = 64+0+1 = 65
- Table 24: intraPredAngle = 256 for predModeIntra=13
- eq.333: predSamples[x][y] runs, reaching max y of 3
- eq.333: iIdx = (((y+1+refIdx)*intraPredAngle)>>5)+refIdx = (((3+1+0)*256)>>5)+0 = 32
- eq.336: predSamples uses ref[x+iIdx+i]
- iIdx=32 and maximums of x=31 and i=3
- max index of ref used is x+iIdx+i=31+32+3=66
However, per eq.332, ref is only populated up to max index of 65. So it seems this process is accessing an unitialized value of ref[66].
Examining the VTM code for equivalent bounds handling to eq.332 in xPredIntraAng():
const int log2Ratio = floorLog2(width) - floorLog2(height);
const int s = std::max<int>(0, bIsModeVer ? log2Ratio : -log2Ratio);
const int maxIndex = (multiRefIdx << s) + 2;
const Pel val = refMain[refLength + multiRefIdx];
for (int z = 1; z <= maxIndex; z++)
{
refMain[refLength + multiRefIdx + z] = val;
}
Per the spec eq.332, max index when refIdx==0 is refW+refIdx+x=64+0+(...*0) + 1 == 65, while for VTM maxIndex it is refLength+multiRefIdx+z = 64+0+(0<<...) + 2 == 66.
Change history (5)
comment:1 Changed 2 years ago by yk
comment:2 Changed 23 months ago by bbross
Thanks, good catch and the following fixes the issues:
– The additional samples ref[ refW + refIdx +x ] with x = 1..( Max( 1, nTbW / nTbH ) * refIdx + 21 ) are derived as follows:
ref[ refW + refIdx + x ] = p[ −1 + refW ][ −1 − refIdx ] (335)
and:
– The additional samples ref[ refH + refIdx + x ] with x = 1..( Max( 1, nTbH / nTbW ) * refIdx + 21 ) are derived as follows:
ref[ refH + refIdx +x ] = p[ −1 − refIdx ][ −1 + refH ] (345)
comment:3 Changed 23 months ago by iole_moccagatta
This is a low level fix. So it is worth to spell out that the fix is to modify the written spec to match the VTM, correct ?
comment:4 Changed 22 months ago by bbross
As far as I can tell, this issue is editorial and can be seen as aligning spec with software although the result is identical in all cases.
See discussion under #636:
The currently undefined but referenced value is later multiplied by 0 so there is no impact on the results.
However, this ticket shows that it would be clearer to not reference an undefined value.
comment:5 Changed 22 months ago by yk
- Resolution set to fixed
- Status changed from new to closed
Thanks! Fixed in JVET-AC0346, confirmed by JVET. To be integrated in JVET-AC2005-v1.
Can someone familiar with the intra prediction process help check this? Thanks!