Derivation of ArCoeffsYPlus128 etc. doesn't make sense

[haasn]

You specify:

ArCoeffsYPlus128[ i ] = ar_coeffs_y[ i ] - (1<<(BitsArY - 1))

This suggests a signed interpretation of ArCoeffsYPlus128, with value ranges spanning from [-16,15] to [-127,128] depending on the value of BitsArY. When you then go on to subtract 128 from this value as part of the film grain application process, you get nonsensical value ranges like [-144,-113] or [-255,0].

Contrast this with AV1 film grain spec, in which ar_coeffs_y_plus_128[ i ] is parsed as an unsigned f(8), which is then shifted by subtracting 128 (as the name implies), giving a signed integer in the range [-128,127], as part of the film grain application process.

I strongly suspect the intent here was either one of:

• ArCoeffsYPlus128[ i ] = ar_coeffs_y[ i ] << (8 - BitsArY): in which case we simply shift these values into corresponding 8-bit range
• ArCoeffsYPlus128[ i ] = (ar_coeffs_y[i] - (1<<(BitsArY - 1)) + 128: in which case we take the lower precision numbers as-is (the +128 here just cancelling out the -128 implied by the application semantics).

But I don't have enough information (let alone a sample file) to confirm which interpretation is correct.

[haasn]

I suspect that this error was introduced accidentally as part of https://github.com/AOMediaCodec/afgs1-spec/pull/65/commits/635ef34c4f6891b68804fe970bb179ec9225782e, which replaced the previous formula ar_coeffs_y[ i ] = ar_coeffs_y[ i ] - (1<<(BitsArY - 1)) by changing the name of the field.

That would imply the second interpretation is correct.

[andrey-norkin]

The second interpretation is correct. The PR #116 should solve the issue.