Derivation of ArCoeffsYPlus128 etc. doesn't make sense #115
Assignees
Comments
|
I suspect that this error was introduced accidentally as part of 635ef34, which replaced the previous formula That would imply the second interpretation is correct. |
haasn
added a commit
to haasn/FFmpeg
that referenced
this issue
Feb 26, 2024
Based on the AOMedia Film Grain Synthesis 1 (AFGS1) spec: https://aomediacodec.github.io/afgs1-spec/ The parsing has been changed substantially relative to the AV1 film grain OBU. In particular: 1. There is the possibility of maintaining multiple independent film grain parameter sets, and decoders are recommended to pick the one most appropriate for the intended display resolution. This is to support scalable / multi-level codecs, although this could also be used to e.g. switch between different grain profiles without having to re-signal the appropriate coefficients. 2. Supporting this, it's possible to *predict* the grain coefficients from previously signalled parameter sets, transmitting only the residual. 3. When not predicting, the parameter sets are now stored as a series of increments, rather than being directly transmitted. I placed this parser in its own file, rather than h2645_sei.c, since nothing in the generic AFGS1 film grain payload is specific to T.35. Note: Due to an ambiguity/mistake in the specification, the parsing of AR coefficients is possibly incorrect. See for details: AOMediaCodec/afgs1-spec#115
haasn
added a commit
to haasn/FFmpeg
that referenced
this issue
Feb 29, 2024
Based on the AOMedia Film Grain Synthesis 1 (AFGS1) spec: https://aomediacodec.github.io/afgs1-spec/ The parsing has been changed substantially relative to the AV1 film grain OBU. In particular: 1. There is the possibility of maintaining multiple independent film grain parameter sets, and decoders are recommended to pick the one most appropriate for the intended display resolution. This is to support scalable / multi-level codecs, although this could also be used to e.g. switch between different grain profiles without having to re-signal the appropriate coefficients. 2. Supporting this, it's possible to *predict* the grain coefficients from previously signalled parameter sets, transmitting only the residual. 3. When not predicting, the parameter sets are now stored as a series of increments, rather than being directly transmitted. I placed this parser in its own file, rather than h2645_sei.c, since nothing in the generic AFGS1 film grain payload is specific to T.35. Note: Due to an ambiguity/mistake in the specification, the parsing of AR coefficients is possibly incorrect. See for details: AOMediaCodec/afgs1-spec#115
|
The second interpretation is correct. The PR #116 should solve the issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
You specify:
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 unsignedf(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 rangeArCoeffsYPlus128[ 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.
The text was updated successfully, but these errors were encountered: