Wrong size for array of coeff delta idx

[pierrick.bouvier]

In, source/Lib/CommonLib/AlfParameters.h

size of arrays:

• lumaCoeff
• lumaClipp
• filterCoeffDeltaIdx
• alfLumaCoeffFlag

is wrongly defined to MAX_NUM_ALF_CLASSES (= 25).

In spec, 7.4.3.16, it is said:
The length of alf_luma_coeff_delta_idx[ filtIdx ] is
Ceil( Log2( alf_luma_num_filters_signalled_minus1 + 1 ) ) bits.

This results in this array having value possible between 0 and 32, instead of 0 and 25.

This results in out-of-bounds memory access, leading to wrong values (unitialized) used in reconstruction, and finally, random differences in YUV output.

[pierrick.bouvier]

Fixing issue: ​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/merge_requests/1209

[nanhu]

In spec, 7.4.3.16, it says "alf_luma_coeff_delta_idx[ filtIdx ] specifies the indices of the signalled adaptive loop filter luma coefficient deltas for the filter class indicated by filtIdx ranging from 0 to NumAlfFilters − 1.", where "The variable NumAlfFilters specifying the number of different adaptive loop filters is set equal to 25.". This is MAX_NUM_ALF_CLASSES (equal to 25) in VTM.

This means alf_luma_coeff_delta_idx[ filtIdx ] is in the range of [0, 24]. So there is no problem with VTM.

When alf_luma_num_filters_signalled_minus1 + 1 is less than 25, alf_luma_coeff_abs[ sfIdx ][ j ] with sfIdx > alf_luma_num_filters_signalled_minus1 are not in the bit stream and will be inferred to be 0. So when an alf_luma_coeff_delta_idx[ filtIdx ] is larger than alf_luma_num_filters_signalled_minus1, zero filters will be referred to.

However, to make this cleaner, in spec, it is better to change "alf_luma_coeff_delta_idx[ filtIdx ] specifies the indices of the signalled adaptive loop filter luma coefficient deltas for the filter class indicated by filtIdx ranging from 0 to NumAlfFilters − 1." into "alf_luma_coeff_delta_idx[ filtIdx ] specifies the indices of the signalled adaptive loop filter luma coefficient deltas for the filter class indicated by filtIdx ranging from 0 to alf_luma_num_filters_signalled_minus1." such that alf_luma_coeff_delta_idx[ filtIdx ] is in the range of [0, alf_luma_num_filters_signalled_minus1].

[pierrick.bouvier]

Nanhu, Thank you for your feedback.

I agree with you that filtIdx as a value between 0 and 25, as spec (exactly between [0, alf_luma_num_filters_signalled_minus1] as you said, even if, in VTM, is was implemented more simply with [0,25 always).

However, I still think alf_luma_coeff_delta_idx[filtIdx] is an array with values between 0 and 32.

Why?
Part you don't quote in the spec says:
The length of alf_luma_coeff_delta_idx[ filtIdx ] is
Ceil( Log2( alf_luma_num_filters_signalled_minus1 + 1 ) ) bits.

I am probably missing something, or spec is misleading, but to me, it says min value is 0, and max value is 32.
Again, for me there is a difference between filtIdx and value of coeff delta idx. To me, the name of variable in VTM as in 'filterIdx = alfParam.filterCoeffDeltaIdx[classIdx]' is not the filter idx, but the coeff delta idx.

Am I on a false track?

Thank you.

[pierrick.bouvier]

After talking with Nan,

it seems like this is a spec issue.

---

From Nan

I think what you said is correct based on the description in spec. The spec makes people confused about filtIdx and coeff delta idx. Before using fixed length code to signal alf_luma_coeff_delta_idx[ filtIdx ], truncated binary code is used with alf_luma_num_filters_signalled_minus1 as the max value. When changing truncated binary code into fixed length code, this constraint was not added back.

In the attached file, I change filtIdx into classIdx to match what is done in VTM and remove the misleading. In addition, I also add the max constraint.

Nevertheless, I agree with you that VTM should check alf_luma_coeff_delta_idx[ filtIdx ] to guarantee that is it in the range of [0, alf_luma_num_filters_signalled_minus1].

---

Here is its fix proposed for spec:
​https://vcgit.hhi.fraunhofer.de/pierrick.bouvier/VVCSoftware_VTM/raw/spec_changes_798/JVET-P2001-vE-798.docx

Here is an updated merge request to check compliance:
​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/merge_requests/1214

[bbross]

Thanks for reporting.

This has been fixed in JVET-Q2001-v8.