[FFmpeg-devel] [PATCH v3 5/5] avcodec/h264*: Omit potentially wrong values from log messages

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Mon Jul 27 12:08:10 EEST 2020


get_ue_golomb_31() and get_ue_golomb() can only parse values within
a certain range correctly; if the parsed value is not within said range,
the latter function returns AVERROR_INVALIDDATA, while the former returns
something different from the encountered value. So the return values are
good enough to determine whether an exp golomb code in the desired range
has been encountered, but they are not necessarily correct. Therefore
they should not be used in error messages stating that a certain value
(the return value of these functions) is out-of-range; instead just state
the correct range and that the parsed value is not in said range.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavcodec/cavsdec.c     |  2 +-
 libavcodec/h264_cavlc.c  | 14 +++++++-------
 libavcodec/h264_parse.c  |  4 ++--
 libavcodec/h264_parser.c |  2 +-
 libavcodec/h264_ps.c     | 28 +++++++++++++++-------------
 libavcodec/h264_sei.c    |  2 +-
 libavcodec/h264_slice.c  |  2 +-
 7 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 9c3825df38..6b5ad587ca 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -615,7 +615,7 @@ static inline int decode_residual_inter(AVSContext *h)
     /* get coded block pattern */
     int cbp = get_ue_golomb(&h->gb);
     if (cbp > 63U) {
-        av_log(h->avctx, AV_LOG_ERROR, "illegal inter cbp %d\n", cbp);
+        av_log(h->avctx, AV_LOG_ERROR, "Inter cbp not in 0-63 range\n");
         return AVERROR_INVALIDDATA;
     }
     h->cbp = cbp_tab[cbp][1];
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 6481992e58..137329479c 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -762,7 +762,7 @@ int ff_h264_decode_mb_cavlc(const H264Context *h, H264SliceContext *sl)
             mb_type--;
 decode_intra_mb:
         if(mb_type > 25){
-            av_log(h->avctx, AV_LOG_ERROR, "mb_type %d in %c slice too large at %d %d\n", mb_type, av_get_picture_type_char(sl->slice_type), sl->mb_x, sl->mb_y);
+            av_log(h->avctx, AV_LOG_ERROR, "mb_type in %c slice too large at %d %d\n", av_get_picture_type_char(sl->slice_type), sl->mb_x, sl->mb_y);
             return -1;
         }
         partition_count=0;
@@ -891,7 +891,7 @@ decode_intra_mb:
                     }else{
                         tmp= get_ue_golomb_31(&sl->gb);
                         if(tmp>=ref_count){
-                            av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", tmp);
+                            av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
                             return -1;
                         }
                     }
@@ -967,7 +967,7 @@ decode_intra_mb:
                         }else{
                             val= get_ue_golomb_31(&sl->gb);
                             if (val >= rc) {
-                                av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
+                                av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
                                 return -1;
                             }
                         }
@@ -998,7 +998,7 @@ decode_intra_mb:
                             }else{
                                 val= get_ue_golomb_31(&sl->gb);
                                 if (val >= rc) {
-                                    av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
+                                    av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
                                     return -1;
                                 }
                             }
@@ -1036,7 +1036,7 @@ decode_intra_mb:
                             }else{
                                 val= get_ue_golomb_31(&sl->gb);
                                 if (val >= rc) {
-                                    av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val);
+                                    av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
                                     return -1;
                                 }
                             }
@@ -1071,7 +1071,7 @@ decode_intra_mb:
 
         if(decode_chroma){
             if(cbp > 47){
-                av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d %d\n", cbp, sl->mb_x, sl->mb_y);
+                av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 47, sl->mb_x, sl->mb_y);
                 return -1;
             }
             if (IS_INTRA4x4(mb_type))
@@ -1080,7 +1080,7 @@ decode_intra_mb:
                 cbp = ff_h264_golomb_to_inter_cbp[cbp];
         }else{
             if(cbp > 15){
-                av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d %d\n", cbp, sl->mb_x, sl->mb_y);
+                av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 15, sl->mb_x, sl->mb_y);
                 return -1;
             }
             if(IS_INTRA4x4(mb_type)) cbp= golomb_to_intra4x4_cbp_gray[cbp];
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index 1c1d1c04b0..660a922621 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -37,7 +37,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS *sps,
 
     pwt->luma_log2_weight_denom = get_ue_golomb_31(gb);
     if (pwt->luma_log2_weight_denom > 7U) {
-        av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of range\n", pwt->luma_log2_weight_denom);
+        av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom not in 0-7 range\n");
         pwt->luma_log2_weight_denom = 0;
     }
     luma_def = 1 << pwt->luma_log2_weight_denom;
@@ -45,7 +45,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS *sps,
     if (sps->chroma_format_idc) {
         pwt->chroma_log2_weight_denom = get_ue_golomb_31(gb);
         if (pwt->chroma_log2_weight_denom > 7U) {
-            av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of range\n", pwt->chroma_log2_weight_denom);
+            av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom not in 0-7 range\n");
             pwt->chroma_log2_weight_denom = 0;
         }
         chroma_def = 1 << pwt->chroma_log2_weight_denom;
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index aacd44cf3b..5be7d55081 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -351,7 +351,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
             pps_id = get_ue_golomb(&nal.gb);
             if (pps_id >= MAX_PPS_COUNT) {
                 av_log(avctx, AV_LOG_ERROR,
-                       "pps_id %u out of range\n", pps_id);
+                       "pps_id not in 0-255 range\n");
                 goto fail;
             }
             if (!p->ps.pps_list[pps_id]) {
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index e21c2b56ac..0a500feb82 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -364,7 +364,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     sps_id    = get_ue_golomb_31(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 not in 0-31 range\n");
         goto fail;
     }
 
@@ -412,8 +412,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
         }
         if (sps->bit_depth_luma   < 8 || sps->bit_depth_luma   > 14 ||
             sps->bit_depth_chroma < 8 || sps->bit_depth_chroma > 14) {
-            av_log(avctx, AV_LOG_ERROR, "illegal bit depth value (%d, %d)\n",
-                   sps->bit_depth_luma, sps->bit_depth_chroma);
+            av_log(avctx, AV_LOG_ERROR, "bit depth value outside of 8-14 range\n");
             goto fail;
         }
         sps->transform_bypass = get_bits1(gb);
@@ -443,7 +442,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     if (sps->poc_type == 0) { // FIXME #define
         unsigned t = get_ue_golomb_31(gb);
         if (t>12) {
-            av_log(avctx, AV_LOG_ERROR, "log2_max_poc_lsb (%d) is out of range\n", t);
+            av_log(avctx, AV_LOG_ERROR, "log2_max_poc_lsb not in 0-12 range\n");
             goto fail;
         }
         sps->log2_max_poc_lsb = t + 4;
@@ -465,7 +464,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
         if ((unsigned)sps->poc_cycle_length >=
             FF_ARRAY_ELEMS(sps->offset_for_ref_frame)) {
             av_log(avctx, AV_LOG_ERROR,
-                   "poc_cycle_length overflow %d\n", sps->poc_cycle_length);
+                   "num_ref_frames_in_pic_order_cnt_cycle not in 0-255 range\n");
             goto fail;
         }
 
@@ -478,7 +477,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
             }
         }
     } else if (sps->poc_type != 2) {
-        av_log(avctx, AV_LOG_ERROR, "illegal POC type %d\n", sps->poc_type);
+        av_log(avctx, AV_LOG_ERROR, "illegal POC type\n");
         goto fail;
     }
 
@@ -487,7 +486,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
         sps->ref_frame_count = FFMAX(2, sps->ref_frame_count);
     if (sps->ref_frame_count > MAX_DELAYED_PIC_COUNT) {
         av_log(avctx, AV_LOG_ERROR,
-               "too many reference frames %d\n", sps->ref_frame_count);
+               "too many reference frames\n");
         goto fail;
     }
     sps->gaps_in_frame_num_allowed_flag = get_bits1(gb);
@@ -758,7 +757,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
     int ret;
 
     if (pps_id >= MAX_PPS_COUNT) {
-        av_log(avctx, AV_LOG_ERROR, "pps_id %u out of range\n", pps_id);
+        av_log(avctx, AV_LOG_ERROR, "pps_id not in 0-255 range\n");
         return AVERROR_INVALIDDATA;
     }
 
@@ -782,9 +781,13 @@ 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);
-    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);
+    if ((unsigned)pps->sps_id >= MAX_SPS_COUNT) {
+        av_log(avctx, AV_LOG_ERROR, "sps_id not in 0-31 range\n");
+        ret = AVERROR_INVALIDDATA;
+        goto fail;
+    }
+    if (!ps->sps_list[pps->sps_id]) {
+        av_log(avctx, AV_LOG_ERROR, "unavailable SPS (id %d) referenced\n", pps->sps_id);
         ret = AVERROR_INVALIDDATA;
         goto fail;
     }
@@ -798,8 +801,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
 
     if (sps->bit_depth_luma > 14) {
         av_log(avctx, AV_LOG_ERROR,
-               "Invalid luma bit depth=%d\n",
-               sps->bit_depth_luma);
+               "Invalid luma bit depth > 14\n");
         ret = AVERROR_INVALIDDATA;
         goto fail;
     } else if (sps->bit_depth_luma == 11 || sps->bit_depth_luma == 13) {
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 7b8e6bd7ba..3aad7cf9a7 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -323,7 +323,7 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb,
     sps_id = get_ue_golomb_31(gb);
     if (sps_id > 31 || !ps->sps_list[sps_id]) {
         av_log(logctx, AV_LOG_ERROR,
-               "non-existing SPS %d referenced in buffering period\n", sps_id);
+               "non-existing SPS referenced in buffering period\n");
         return sps_id > 31 ? 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..515a796af0 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1764,7 +1764,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
 
     sl->pps_id = get_ue_golomb(&sl->gb);
     if (sl->pps_id >= MAX_PPS_COUNT) {
-        av_log(h->avctx, AV_LOG_ERROR, "pps_id %u out of range\n", sl->pps_id);
+        av_log(h->avctx, AV_LOG_ERROR, "pps_id not in 0-255 range\n");
         return AVERROR_INVALIDDATA;
     }
     if (!h->ps.pps_list[sl->pps_id]) {
-- 
2.20.1



More information about the ffmpeg-devel mailing list