[FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Tue Jul 14 18:34:50 EEST 2020


get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used
to parse exp-golomb codes of length <= 9, i.e. those codes with at most
four leading bits that have five effective bits; this implies a range of
0..30 and not 31. In particular, this function must not be used to parse
e.g. the H.264 SPS id.

This commit renames the function to get_ue_golomb_30() and uses it
whereever possible. In places where it is not possible, it has been
replaced by get_ue_golomb2(). Given that the returned value of said
function can be a negative error code, it is no longer included in
av_log() statements (the parsed value can actually also be wrong when
using get_ue_golomb30()).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
I have only used get_ue_golomb_30() where I am certain that its range is
sufficient; this unfortunately excludes certain cases in h264_cavlc,
because for field based content, the number of refs can be in the range
of 0..31.

 libavcodec/golomb.h      |  6 +++---
 libavcodec/h264_cavlc.c  | 14 +++++++-------
 libavcodec/h264_parser.c |  8 ++++----
 libavcodec/h264_ps.c     | 18 +++++++++---------
 libavcodec/h264_refs.c   |  6 +++---
 libavcodec/h264_sei.c    |  8 ++++----
 libavcodec/h264_slice.c  |  6 +++---
 libavformat/mxfenc.c     |  2 +-
 8 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 63069f63e5..172d9934e0 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -134,10 +134,10 @@ static inline unsigned get_ue_golomb_long(GetBitContext *gb)
 }
 
 /**
- * read unsigned exp golomb code, constraint to a max of 31.
- * the return value is undefined if the stored value exceeds 31.
+ * Read unsigned exp golomb code, constraint to a max of 30.
+ * the return value is undefined if the stored value exceeds 30.
  */
-static inline int get_ue_golomb_31(GetBitContext *gb)
+static inline int get_ue_golomb_30(GetBitContext *gb)
 {
     unsigned int buf;
 
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 6481992e58..51091abbe1 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -838,7 +838,7 @@ decode_intra_mb:
         }
         if(decode_chroma){
             pred_mode= ff_h264_check_intra_pred_mode(h->avctx, sl->top_samples_available,
-                                                     sl->left_samples_available, get_ue_golomb_31(&sl->gb), 1);
+                                                     sl->left_samples_available, get_ue_golomb_30(&sl->gb), 1);
             if(pred_mode < 0)
                 return -1;
             sl->chroma_pred_mode = pred_mode;
@@ -850,7 +850,7 @@ decode_intra_mb:
 
         if (sl->slice_type_nos == AV_PICTURE_TYPE_B) {
             for(i=0; i<4; i++){
-                sl->sub_mb_type[i]= get_ue_golomb_31(&sl->gb);
+                sl->sub_mb_type[i] = get_ue_golomb_30(&sl->gb);
                 if(sl->sub_mb_type[i] >=13){
                     av_log(h->avctx, AV_LOG_ERROR, "B sub_mb_type %u out of range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
                     return -1;
@@ -868,7 +868,7 @@ decode_intra_mb:
         }else{
             av_assert2(sl->slice_type_nos == AV_PICTURE_TYPE_P); //FIXME SP correct ?
             for(i=0; i<4; i++){
-                sl->sub_mb_type[i]= get_ue_golomb_31(&sl->gb);
+                sl->sub_mb_type[i] = get_ue_golomb_30(&sl->gb);
                 if(sl->sub_mb_type[i] >=4){
                     av_log(h->avctx, AV_LOG_ERROR, "P sub_mb_type %u out of range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
                     return -1;
@@ -889,7 +889,7 @@ decode_intra_mb:
                     }else if(ref_count == 2){
                         tmp= get_bits1(&sl->gb)^1;
                     }else{
-                        tmp= get_ue_golomb_31(&sl->gb);
+                        tmp = get_ue_golomb2(&sl->gb);
                         if(tmp>=ref_count){
                             av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", tmp);
                             return -1;
@@ -965,7 +965,7 @@ decode_intra_mb:
                         } else if (rc == 2) {
                             val= get_bits1(&sl->gb)^1;
                         }else{
-                            val= get_ue_golomb_31(&sl->gb);
+                            val = get_ue_golomb2(&sl->gb);
                             if (val >= rc) {
                                 av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
                                 return -1;
@@ -996,7 +996,7 @@ decode_intra_mb:
                             } else if (rc == 2) {
                                 val= get_bits1(&sl->gb)^1;
                             }else{
-                                val= get_ue_golomb_31(&sl->gb);
+                                val = get_ue_golomb2(&sl->gb);
                                 if (val >= rc) {
                                     av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
                                     return -1;
@@ -1034,7 +1034,7 @@ decode_intra_mb:
                             } else if (rc == 2) {
                                 val= get_bits1(&sl->gb)^1;
                             }else{
-                                val= get_ue_golomb_31(&sl->gb);
+                                val = get_ue_golomb2(&sl->gb);
                                 if (val >= rc) {
                                     av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
                                     return -1;
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index aacd44cf3b..8af094075f 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -180,7 +180,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb,
             if (get_bits1(gb)) {
                 int index;
                 for (index = 0; ; index++) {
-                    unsigned int reordering_of_pic_nums_idc = get_ue_golomb_31(gb);
+                    unsigned reordering_of_pic_nums_idc = get_ue_golomb_30(gb);
 
                     if (reordering_of_pic_nums_idc < 3)
                         get_ue_golomb_long(gb);
@@ -210,7 +210,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb,
     if (get_bits1(gb)) { // adaptive_ref_pic_marking_mode_flag
         int i;
         for (i = 0; i < MAX_MMCO_COUNT; i++) {
-            MMCOOpcode opcode = get_ue_golomb_31(gb);
+            MMCOOpcode opcode = get_ue_golomb_30(gb);
             if (opcode > (unsigned) MMCO_LONG) {
                 av_log(logctx, AV_LOG_ERROR,
                        "illegal memory management control operation %d\n",
@@ -226,7 +226,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb,
                 get_ue_golomb_long(gb); // difference_of_pic_nums_minus1
             if (opcode == MMCO_SHORT2LONG || opcode == MMCO_LONG2UNUSED ||
                 opcode == MMCO_LONG || opcode == MMCO_SET_MAX_LONG)
-                get_ue_golomb_31(gb);
+                get_ue_golomb2(gb);
         }
     }
 
@@ -342,7 +342,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
         /* fall through */
         case H264_NAL_SLICE:
             get_ue_golomb_long(&nal.gb);  // skip first_mb_in_slice
-            slice_type   = get_ue_golomb_31(&nal.gb);
+            slice_type   = get_ue_golomb_30(&nal.gb);
             s->pict_type = ff_h264_golomb_to_pict_type[slice_type % 5];
             if (p->sei.recovery_point.recovery_frame_cnt >= 0) {
                 /* key frame, since recovery_frame_cnt is set */
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index e774929e21..13823b8dc0 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -108,10 +108,10 @@ static inline int decode_hrd_parameters(GetBitContext *gb, void *logctx,
                                         SPS *sps)
 {
     int cpb_count, i;
-    cpb_count = get_ue_golomb_31(gb) + 1;
+    cpb_count = get_ue_golomb2(gb);
 
-    if (cpb_count > 32U) {
-        av_log(logctx, AV_LOG_ERROR, "cpb_count %d invalid\n", cpb_count);
+    if (cpb_count++ > 31U) {
+        av_log(logctx, AV_LOG_ERROR, "cpb_count invalid\n");
         return AVERROR_INVALIDDATA;
     }
 
@@ -361,10 +361,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     constraint_set_flags |= get_bits1(gb) << 5;   // constraint_set5_flag
     skip_bits(gb, 2);                             // reserved_zero_2bits
     level_idc = get_bits(gb, 8);
-    sps_id    = get_ue_golomb_31(gb);
+    sps_id    = get_ue_golomb2(gb);
 
     if (sps_id >= MAX_SPS_COUNT) {
-        av_log(avctx, AV_LOG_ERROR, "sps_id %u out of range\n", sps_id);
+        av_log(avctx, AV_LOG_ERROR, "sps_id out of range\n");
         goto fail;
     }
 
@@ -391,7 +391,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
         sps->profile_idc == 128 ||  // Multiview High profile (MVC)
         sps->profile_idc == 138 ||  // Multiview Depth High profile (MVCD)
         sps->profile_idc == 144) {  // old High444 profile
-        sps->chroma_format_idc = get_ue_golomb_31(gb);
+        sps->chroma_format_idc = get_ue_golomb_30(gb);
         if (sps->chroma_format_idc > 3U) {
             avpriv_request_sample(avctx, "chroma_format_idc %u",
                                   sps->chroma_format_idc);
@@ -438,7 +438,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     }
     sps->log2_max_frame_num = log2_max_frame_num_minus4 + 4;
 
-    sps->poc_type = get_ue_golomb_31(gb);
+    sps->poc_type = get_ue_golomb_30(gb);
 
     if (sps->poc_type == 0) { // FIXME #define
         unsigned t = get_ue_golomb(gb);
@@ -482,7 +482,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
         goto fail;
     }
 
-    sps->ref_frame_count = get_ue_golomb_31(gb);
+    sps->ref_frame_count = get_ue_golomb_30(gb);
     if (avctx->codec_tag == MKTAG('S', 'M', 'V', '2'))
         sps->ref_frame_count = FFMAX(2, sps->ref_frame_count);
     if (sps->ref_frame_count > MAX_DELAYED_PIC_COUNT) {
@@ -781,7 +781,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
     }
     memcpy(pps->data, gb->buffer, pps->data_size);
 
-    pps->sps_id = get_ue_golomb_31(gb);
+    pps->sps_id = get_ue_golomb2(gb);
     if ((unsigned)pps->sps_id >= MAX_SPS_COUNT ||
         !ps->sps_list[pps->sps_id]) {
         av_log(avctx, AV_LOG_ERROR, "sps_id %u out of range\n", pps->sps_id);
diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index dae8bd278a..4ceaaa3863 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -432,7 +432,7 @@ int ff_h264_decode_ref_pic_list_reordering(H264SliceContext *sl, void *logctx)
             continue;
 
         for (index = 0; ; index++) {
-            unsigned int op = get_ue_golomb_31(&sl->gb);
+            unsigned int op = get_ue_golomb_30(&sl->gb);
 
             if (op == 3)
                 break;
@@ -850,7 +850,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl, GetBitContext *gb,
         sl->explicit_ref_marking = get_bits1(gb);
         if (sl->explicit_ref_marking) {
             for (i = 0; i < MAX_MMCO_COUNT; i++) {
-                MMCOOpcode opcode = get_ue_golomb_31(gb);
+                MMCOOpcode opcode = get_ue_golomb_30(gb);
 
                 mmco[i].opcode = opcode;
                 if (opcode == MMCO_SHORT2UNUSED || opcode == MMCO_SHORT2LONG) {
@@ -860,7 +860,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl, GetBitContext *gb,
                 }
                 if (opcode == MMCO_SHORT2LONG || opcode == MMCO_LONG2UNUSED ||
                     opcode == MMCO_LONG || opcode == MMCO_SET_MAX_LONG) {
-                    unsigned int long_arg = get_ue_golomb_31(gb);
+                    unsigned int long_arg = get_ue_golomb2(gb);
                     if (long_arg >= 32 ||
                         (long_arg >= 16 && !(opcode == MMCO_SET_MAX_LONG &&
                                              long_arg == 16) &&
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 7b8e6bd7ba..63d43b2bc5 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -320,11 +320,11 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb,
     int sched_sel_idx;
     const SPS *sps;
 
-    sps_id = get_ue_golomb_31(gb);
-    if (sps_id > 31 || !ps->sps_list[sps_id]) {
+    sps_id = get_ue_golomb2(gb);
+    if (sps_id > 31U || !ps->sps_list[sps_id]) {
         av_log(logctx, AV_LOG_ERROR,
-               "non-existing SPS %d referenced in buffering period\n", sps_id);
-        return sps_id > 31 ? AVERROR_INVALIDDATA : AVERROR_PS_NOT_FOUND;
+               "non-existing SPS referenced in buffering period\n");
+        return sps_id > 31U ? AVERROR_INVALIDDATA : AVERROR_PS_NOT_FOUND;
     }
     sps = (const SPS*)ps->sps_list[sps_id]->data;
 
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c7b2764270..8f4b7ef1ec 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1739,7 +1739,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
 
     sl->first_mb_addr = get_ue_golomb_long(&sl->gb);
 
-    slice_type = get_ue_golomb_31(&sl->gb);
+    slice_type = get_ue_golomb_30(&sl->gb);
     if (slice_type > 9) {
         av_log(h->avctx, AV_LOG_ERROR,
                "slice type %d too large at %d\n",
@@ -1874,7 +1874,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
     }
 
     if (sl->slice_type_nos != AV_PICTURE_TYPE_I && pps->cabac) {
-        tmp = get_ue_golomb_31(&sl->gb);
+        tmp = get_ue_golomb_30(&sl->gb);
         if (tmp > 2) {
             av_log(h->avctx, AV_LOG_ERROR, "cabac_init_idc %u overflow\n", tmp);
             return AVERROR_INVALIDDATA;
@@ -1902,7 +1902,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
     sl->slice_alpha_c0_offset = 0;
     sl->slice_beta_offset     = 0;
     if (pps->deblocking_filter_parameters_present) {
-        tmp = get_ue_golomb_31(&sl->gb);
+        tmp = get_ue_golomb_30(&sl->gb);
         if (tmp > 2) {
             av_log(h->avctx, AV_LOG_ERROR,
                    "deblocking_filter_idc %u out of range\n", tmp);
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 5a3a609bf6..c7ceb40b55 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2223,7 +2223,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st,
         case H264_NAL_SLICE:
             init_get_bits8(&gb, buf, buf_end - buf);
             get_ue_golomb_long(&gb); // skip first_mb_in_slice
-            slice_type = get_ue_golomb_31(&gb);
+            slice_type = get_ue_golomb_30(&gb);
             switch (slice_type % 5) {
             case 0:
                 e->flags |= 0x20; // P Picture
-- 
2.20.1



More information about the ffmpeg-devel mailing list