diff mbox series

[FFmpeg-devel,v2,6/6] avcodec/h264*: Omit potentially wrong values from log messages

Message ID 20200714201954.30327-6-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/6] avcodec/golomb: Don't emit error message in get_ue_golomb | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 14, 2020, 8:19 p.m. UTC
get_ue_golomb_30() and get_ue_golomb() (as well as
get_ue_golomb2()) can only parse values within a certain range
correctly; if the parsed value is not within said range, the latter two
functions return AVERROR_INVALIDDATA, while the former returns something
outside of said range (currently 32 for a 0-30 range). 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@gmail.com>
---
Is it actually intentional that the SPS.offset_for_ref_frame array has
256 entries, although the specs only allow 255 elements in an SPS?

 libavcodec/cavsdec.c     |  2 +-
 libavcodec/h264_cavlc.c  | 14 +++++++-------
 libavcodec/h264_parse.c  |  4 ++--
 libavcodec/h264_parser.c |  2 +-
 libavcodec/h264_ps.c     | 26 ++++++++++++++------------
 libavcodec/h264_slice.c  |  2 +-
 6 files changed, 26 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 3d4b9cdeff..2857646438 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 48f0f0689d..af1a3b1370 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(&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(&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(&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(&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 53e644da66..fd9b824756 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_30(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_30(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 d0269497d5..3dfd1127a1 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 57ee74ec32..b1285255aa 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -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_30(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(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_slice.c b/libavcodec/h264_slice.c
index 8f4b7ef1ec..29b57bc305 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]) {