PR #25664 opened by Kilian (lusoris) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25664 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25664.patch
This patch makes the AFGS1 parser derive predicted scaling points when a set predicts them from its reference but codes no residual (bits_per_y_scaling_res or bits_per_cb/cr_scaling_res equal to 0). The AFGS1 film grain parameter set syntax (https://aomediacodec.github.io/afgs1-spec/) derives numYPointsInRef points from the reference set whenever predict_y_scaling_flag is 1; the residual term is zero when no residual is coded. The same holds for predict_cb_scaling_flag and predict_cr_scaling_flag. What goes wrong today: - ff_aom_parse_film_grain_sets() sets num_y_points and the predicted points only inside `if (bits_res)`, so with a zero residual width the set ends up with no luma points. The chroma loop has the same shape. - AR coefficient presence is keyed on num_y_points and num_uv_points, so the AR coefficients of such a set are skipped and every later field of the set is read from the wrong bits. What the patch does: - Copies the point count from the reference and derives the points for any residual width; the residuals and their granularity are read only when the width is non-zero. - Uses the spec conditions for AR coefficient presence: luma `num_y_points || predict_y_scaling_flag`, chroma `chroma_scaling_from_luma || num_points || predict_scaling_flag`, with the chroma coefficient count following the luma condition. - Initialises predict_uv_scaling[] for the luma-only and chroma-from-luma paths before it is read. Base: FFmpeg master a88838af48. Depends on #25534: based on master a88838af48 and meant to be merged after #25534, which removes the -Wstringop-overflow warning that gcc 16.2 reports twice in the predicted luma loop without it. On top of #25534 it needs a small textual conflict resolution in the same loop. Testing: - A parser test (not part of this patch) writes AFGS1 sets with a bit writer checked against the spec syntax and compares every parsed field. Cases: luma predicted with zero residual, the same with a 2-bit residual as control, Cb predicted with zero residual, and no prediction as control. On master a88838af48 (gcc 16.2) the two zero-residual cases differ in 13 fields; with this patch all 52 fields match, with gcc 15.2 and gcc 16.2. - With #25534 applied on master a88838af48 (tested tree 8d5823b955), libavcodec/aom_film_grain.c builds without -Wstringop-overflow warnings on gcc 15.2 and gcc 16.2 at -O3 -march=x86-64-v3, and the full build (ubuntu:26.04, gcc 15.2.0-16ubuntu1, x86-64-v3, `--fatal-warnings --enable-version3 --disable-indevs`) shows the same 91 warnings as without this patch. FATE fate-vvc, fate-lossless-wmall, fate-film_grain_params, fate-checkasm and fate-cbs-av1 for both av1-1-b8/b10-23-film_grain-50 samples, 143 tests, pass. - Without #25534, gcc 16.2 at -O3 -march=x86-64-v3 reports the -Wstringop-overflow warning of the predicted luma loop twice at the y_points store instead of once. #25534 states the loop bound that removes it. >From c2754247bf2d2e6de4211975c64a4f8e87b50a0b Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Sat, 31 Oct 2026 23:07:32 +0100 Subject: [PATCH] avcodec/aom_film_grain: Derive predicted points without residual AFGS1 derives predicted scaling points even when the residual bit width is zero. In that case the residual contribution is zero, but the point count and predicted values still come from the reference set. Keeping the point count at zero also suppresses the AR coefficients and shifts every field that follows them. Derive luma and chroma points unconditionally and use the prediction flags in the AR conditions. Signed-off-by: lusoris <[email protected]> --- libavcodec/aom_film_grain.c | 46 +++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/libavcodec/aom_film_grain.c b/libavcodec/aom_film_grain.c index bb60c557741..7ba7f143730 100644 --- a/libavcodec/aom_film_grain.c +++ b/libavcodec/aom_film_grain.c @@ -144,7 +144,7 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s, num_sets = get_bits(gb, 3) + 1; for (n = 0; n < num_sets; n++) { int payload_4byte, payload_size, set_idx, apply_units_log2, vsc_flag; - int predict_scaling, predict_y_scaling, predict_uv_scaling[2]; + int predict_scaling, predict_y_scaling, predict_uv_scaling[2] = { 0 }; int payload_bits, start_position; size_t fgp_size; @@ -218,23 +218,23 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s, predict_y_scaling = predict_scaling ? get_bits1(gb) : 0; if (predict_y_scaling) { - int y_scale, y_offset, bits_res; + int y_scale, y_offset, bits_res, res[14], pred, granularity; y_scale = get_bits(gb, 9) - 256; y_offset = get_bits(gb, 9) - 256; bits_res = get_bits(gb, 3); + aom->num_y_points = ref->codec.aom.num_y_points; if (bits_res) { - int res[14], pred, granularity; - aom->num_y_points = ref->codec.aom.num_y_points; for (i = 0; i < aom->num_y_points; i++) res[i] = get_bits(gb, bits_res); granularity = get_bits(gb, 3); - for (i = 0; i < aom->num_y_points; i++) { - pred = ref->codec.aom.y_points[i][1]; - pred = ((pred * y_scale + 8) >> 4) + y_offset; + } + for (i = 0; i < aom->num_y_points; i++) { + pred = ref->codec.aom.y_points[i][1]; + pred = ((pred * y_scale + 8) >> 4) + y_offset; + if (bits_res) pred += (res[i] - (1 << (bits_res - 1))) * granularity; - aom->y_points[i][0] = ref->codec.aom.y_points[i][0]; - aom->y_points[i][1] = av_clip_uint8(pred); - } + aom->y_points[i][0] = ref->codec.aom.y_points[i][0]; + aom->y_points[i][1] = av_clip_uint8(pred); } } else { aom->num_y_points = get_bits(gb, 4); @@ -266,26 +266,26 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s, for (uv = 0; uv < 2; uv++) { predict_uv_scaling[uv] = predict_scaling ? get_bits1(gb) : 0; if (predict_uv_scaling[uv]) { - int uv_scale, uv_offset, bits_res; + int uv_scale, uv_offset, bits_res, res[10], pred, granularity; uv_scale = get_bits(gb, 9) - 256; uv_offset = get_bits(gb, 9) - 256; bits_res = get_bits(gb, 3); aom->uv_mult[uv] = ref->codec.aom.uv_mult[uv]; aom->uv_mult_luma[uv] = ref->codec.aom.uv_mult_luma[uv]; aom->uv_offset[uv] = ref->codec.aom.uv_offset[uv]; + aom->num_uv_points[uv] = ref->codec.aom.num_uv_points[uv]; if (bits_res) { - int res[10], pred, granularity; - aom->num_uv_points[uv] = ref->codec.aom.num_uv_points[uv]; for (i = 0; i < aom->num_uv_points[uv]; i++) res[i] = get_bits(gb, bits_res); granularity = get_bits(gb, 3); - for (i = 0; i < aom->num_uv_points[uv]; i++) { - pred = ref->codec.aom.uv_points[uv][i][1]; - pred = ((pred * uv_scale + 8) >> 4) + uv_offset; + } + for (i = 0; i < aom->num_uv_points[uv]; i++) { + pred = ref->codec.aom.uv_points[uv][i][1]; + pred = ((pred * uv_scale + 8) >> 4) + uv_offset; + if (bits_res) pred += (res[i] - (1 << (bits_res - 1))) * granularity; - aom->uv_points[uv][i][0] = ref->codec.aom.uv_points[uv][i][0]; - aom->uv_points[uv][i][1] = av_clip_uint8(pred); - } + aom->uv_points[uv][i][0] = ref->codec.aom.uv_points[uv][i][0]; + aom->uv_points[uv][i][1] = av_clip_uint8(pred); } } else { int bits_inc, bits_scaling, uv_offset; @@ -311,15 +311,17 @@ int ff_aom_parse_film_grain_sets(AVFilmGrainAFGS1Params *s, aom->scaling_shift = get_bits(gb, 2) + 8; aom->ar_coeff_lag = get_bits(gb, 2); num_y_coeffs = 2 * aom->ar_coeff_lag * (aom->ar_coeff_lag + 1); - if (aom->num_y_points) { + if (aom->num_y_points || predict_y_scaling) { int ar_bits = get_bits(gb, 2) + 5; for (i = 0; i < num_y_coeffs; i++) aom->ar_coeffs_y[i] = get_bits(gb, ar_bits) - (1 << (ar_bits - 1)); } for (uv = 0; uv < 2; uv++) { - if (aom->chroma_scaling_from_luma || aom->num_uv_points[uv]) { + if (aom->chroma_scaling_from_luma || aom->num_uv_points[uv] || + predict_uv_scaling[uv]) { int ar_bits = get_bits(gb, 2) + 5; - for (i = 0; i < num_y_coeffs + !!aom->num_y_points; i++) + for (i = 0; i < num_y_coeffs + + (aom->num_y_points || predict_y_scaling); i++) aom->ar_coeffs_uv[uv][i] = get_bits(gb, ar_bits) - (1 << (ar_bits - 1)); } } -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
