[FFmpeg-devel] [PATCH] avcodec/h264_sei: fix H.274 film grain parsing

[Niklas Haas]

From: Niklas Haas <git at haasn.dev>

The current code has a number of issues:
1. The fg_model_id is specified in H.274 as u(2), not u(8)
2. This SEI has no ue(v) "repetition period", but a u(1) persistence flag

Additionally, we can get away with using the non-long version of
get_se_golomb() because the SEI spec as written limits these values to
2^(fgBitDepth)-1 where fgBitDepth maxes out at 2^3-1 + 8 = 15 bits.

Fixes: corrupt bistream values due to wrong number of bits read
Fixes: 4ff73add5dbe6c319d693355be44df2e17a0b8bf

Signed-off-by: Niklas Haas <git at haasn.dev>
---
 libavcodec/h264_sei.c   | 6 +++---
 libavcodec/h264_sei.h   | 2 +-
 libavcodec/h264_slice.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 7797f24650..8de1ce4b90 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -424,7 +424,7 @@ static int decode_film_grain_characteristics(H264SEIFilmGrainCharacteristics *h,
 
     if (h->present) {
         memset(h, 0, sizeof(*h));
-        h->model_id = get_bits(gb, 8);
+        h->model_id = get_bits(gb, 2);
         h->separate_colour_description_present_flag = get_bits1(gb);
         if (h->separate_colour_description_present_flag) {
             h->bit_depth_luma = get_bits(gb, 3) + 8;
@@ -448,11 +448,11 @@ static int decode_film_grain_characteristics(H264SEIFilmGrainCharacteristics *h,
                     h->intensity_interval_lower_bound[c][i] = get_bits(gb, 8);
                     h->intensity_interval_upper_bound[c][i] = get_bits(gb, 8);
                     for (int j = 0; j < h->num_model_values[c]; j++)
-                        h->comp_model_value[c][i][j] = get_se_golomb_long(gb);
+                        h->comp_model_value[c][i][j] = get_se_golomb(gb);
                 }
             }
         }
-        h->repetition_period = get_ue_golomb_long(gb);
+        h->persistent = get_bits1(gb);
 
         h->present = 1;
     }
diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
index f9166b45df..dbceb15627 100644
--- a/libavcodec/h264_sei.h
+++ b/libavcodec/h264_sei.h
@@ -183,7 +183,7 @@ typedef struct H264SEIFilmGrainCharacteristics {
     uint8_t intensity_interval_lower_bound[3][256];
     uint8_t intensity_interval_upper_bound[3][256];
     int16_t comp_model_value[3][256][6];
-    int repetition_period;
+    int persistent;
 } H264SEIFilmGrainCharacteristics;
 
 typedef struct H264SEIContext {
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 41338fbcb6..03606bf504 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1381,7 +1381,7 @@ static int h264_export_frame_props(H264Context *h)
         memcpy(&fgp->codec.h274.comp_model_value, &fgc->comp_model_value,
                sizeof(fgp->codec.h274.comp_model_value));
 
-        fgc->present = !!fgc->repetition_period;
+        fgc->present = fgc->persistent;
     }
 
     if (h->sei.picture_timing.timecode_cnt > 0) {
-- 
2.32.0