Opened 4 years ago
Closed 4 years ago
#1182 closed defect (fixed)
Bug in spec for affine AMVP list construction
Reported by: | chhuanb | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | VVC D10 |
Component: | spec | Version: | |
Keywords: | Cc: | ksuehring, bbross, XiangLi, fbossen, jvet@… |
Description
In clause "8.5.5.7 Derivation process for luma affine control point motion vector predictors", step 6, affine constructed candidate is inserted into candidate list when availableConsFlagLX is equal to 1, and numCpMvpCandLX is equal to 0.
The second condition is not present in the SW and the SW matches the original design for affine AMVP list construction.
Suggest spec fixes:
- When numCpMvpCandLX is less than 2, the following applies
– The derivation process for constructed affine control point motion vector prediction candidate as specified in clause 8.5.5.8 is invoked with the luma coding block location ( xCb, yCb ), the luma coding block width cbWidth, the luma coding block height cbHeight, and the reference index of the current coding unit refIdxLX as inputs, and the availability flag availableConsFlagLX, the availability flags availableFlagLX[ cpIdx ] and cpMvpLX[ cpIdx ] with cpIdx = 0..numCpMv − 1 as outputs.
– When availableConsFlagLX is equal to 1, and numCpMvpCandLX is equal to 0, the following assignments are made:
Change history (7)
comment:1 Changed 4 years ago by fbossen
comment:2 Changed 4 years ago by chhuanb
Rename cpMvpLX to cpConstMvpLX would make the spec more clear.
And it seems that the output cpIdx range should be 0..2 regardless of numCpMv.
Because in step 7, cpMvpLX is reused for the range 2..0:
- The following applies for cpIdx = 2..0:
– When numCpMvpCandLX is less than 2 and availableFlagLX[ cpIdx ] is equal to 1, the following assignments are made:
cpMvpListLX[ numCpMvpCandLX ][ 0 ] = cpMvpLX[ cpIdx ] (846)
cpMvpListLX[ numCpMvpCandLX ][ 1 ] = cpMvpLX[ cpIdx ] (847)
cpMvpListLX[ numCpMvpCandLX ][ 2 ] = cpMvpLX[ cpIdx ] (848)
numCpMvpCandLX = numCpMvpCandLX + 1 (849)
comment:3 follow-up: ↓ 4 Changed 4 years ago by fbossen
You are right. cpMvpLx[ 2 ] should be defined. It currently isn't for the case where numCpMv is 2.
There is a similar issue in (825) and (833) I think.
Also the output is defined as:
Output of this process are the luma affine control point motion vector predictors cpMvpLX[ cpIdx ] with X being 0 or 1, and cpIdx = 0 .. numCpMv − 1.
So probably cpMvpLX[ 2 ] shouldn't be assigned a value when numCpMv is 2, unless you want to change the range here as well (but that would mean changing in more places in the spec).
comment:4 in reply to: ↑ 3 Changed 4 years ago by chhuanb
Replying to fbossen:
You are right. cpMvpLx[ 2 ] should be defined. It currently isn't for the case where numCpMv is 2.
There is a similar issue in (825) and (833) I think.
Also the output is defined as:
Output of this process are the luma affine control point motion vector predictors cpMvpLX[ cpIdx ] with X being 0 or 1, and cpIdx = 0 .. numCpMv − 1.
So probably cpMvpLX[ 2 ] shouldn't be assigned a value when numCpMv is 2, unless you want to change the range here as well (but that would mean changing in more places in the spec).
I think (825) and (833) are different to the issue in step 6 and 7. When numCpMV is 2, the assigned cpMvpListLX[numCpMvpCandLX][2] will not be used in anymore. Assgin an undefined variable here doesn't make the spec broken.
comment:5 Changed 4 years ago by hanhuang
Note that in "8.5.5.8 Derivation process for constructed affine control point motion vector prediction candidates", The third (bottom-left) control point motion vector cpMvLX[ 2 ] is derived regardless the numCpMv. So cpMvpLx[ 2 ] is defined. The output described in 8.5.5.8 is: "the constructed affine control point motion vector prediction candidiates cpMvLX[ cpIdx ] with cpIdx = 0..2 and X being 0 or 1." The issue is in step 6 of 8.5.5.7, it's described as "and cpMvpLX[ cpIdx ] with cpIdx = 0..numCpMv − 1 as outputs" I think it should be typo here.
Huanban is correct that it doesn't make spec broken even cpMvpLX[2] is undefined value.
But if we would make spec clear, avoid undefined cpMvpLX[2], we can change the relative text in 8.5.5.7 to "and cpMvpLX[ cpIdx ] with cpIdx = 0..2 as outputs"
comment:6 Changed 4 years ago by jlchen
Thanks for reporting the issue, the condition "numCpMvpCandLX is equal to 0" will be removed in the JVET-S2001-vD release.
Regarding the issue of assignments using undefined cpMvpLX[ 2 ] raised by Frank, quite a few places in section 8.5.5.8 need to be modified. My preference is to solve this issue in the next release of JVET-S2001 even though many changes are needed. I will send to refined documents to the related experts for verification.
comment:7 Changed 4 years ago by jlchen
- Resolution set to fixed
- Status changed from new to closed
The suggested fix looks ok to me, but there seem to be more issues in this section.
Use of cpMvpLX seems to be overloaded. It is the output of the process but also some intermediate variable.
Assignments such as cpMvpListLX[ numCpMvpCandLX ][ 2 ] = cpMvpLX[ 2 ] should probably not occur when numCpMv is 2 since cpMvpLX[ 2 ] would be undefined.