From patchwork Tue Jul 14 15:34:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21008 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id E0D0044AA50 for ; Tue, 14 Jul 2020 18:35:21 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C0FE868B29C; Tue, 14 Jul 2020 18:35:21 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E3FDE6898F7 for ; Tue, 14 Jul 2020 18:35:13 +0300 (EEST) Received: by mail-wr1-f54.google.com with SMTP id a6so22513964wrm.4 for ; Tue, 14 Jul 2020 08:35:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=QVR+Va4Wip8Xfig3oZgqpwY2epXXLnTy/XXNRkirBNc=; b=rpVU301CILxhhXRw4e8cvam68b/4AC/Hn5S7aNkybgijb5+3QCEqRfzwYJ4U7n+Dlq UQ/1si7qSo8lKNwoqy7Zz9l5bUjBEZ0Z94XSPV3NfLSUUzFCWv22AZij+rrGTZ2e1sw3 U5moznEmYkR/25dXEfeHkatD1Q9PmlBDrG9ilTFF/CIPNFaobEAF6A1VQoF420cNFKqn FM/Ey3LJd9FinWkxjNJ4l+F1N9NFNbR6vFj12XXblPou+gLsSSOij4kqiFG3hbeU3xI/ BpXF6C8l5G0V9Rd4kca3TY2S8GryP3X2XJFAbJkPcD3JIBRfqVo7EBLGdz9ldJ6XZqLA 1uoQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=QVR+Va4Wip8Xfig3oZgqpwY2epXXLnTy/XXNRkirBNc=; b=m4sZ7zuKkXGrGK+kVJA9QHZobx9+GQx/idFrYSwHcvnLzfcS673gCCk9bMlT+LxgD6 6pD3O0hoI7QoSV7aWFvGs1Q+0Dmzc8jVsZYu6DPa1rq4ZtUkPce8ClX5odm0CisdqPqy 2Xrp5L6mkdw5RTPicUuh3T6PIymKvLmtWm6HfJrc6HI3CPFDXxw24pm3X/o8fl30EXMi N0db1b+8pVtaxzWHvhjfrVY/AUK3AZTzyZbUNpG6+7aDufUZNU0oewxa/HMCSMjOvX66 VjWNZDPte7pZhPXHDSI7lU+YzOakvPnty8mrX5izbWXicliuwwUFVdXXRTlfyftZeFZb 2K1w== X-Gm-Message-State: AOAM532VX1LCeujf1gXDPnBhpzpU4s8yEWtvpot/JU545QjLqMzThp3t h75r439TYyhSdaADq8GHYT4FYwSB X-Google-Smtp-Source: ABdhPJxqjNiJbfBX8MSEa+Z0m1hTcwD6kMSY/IioOfiqsGF1vX90VZTmNre8aC/+riWzL9ppyAs1jA== X-Received: by 2002:adf:fd8e:: with SMTP id d14mr6520975wrr.202.1594740912627; Tue, 14 Jul 2020 08:35:12 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id w14sm29545452wrt.55.2020.07.14.08.35.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2020 08:35:11 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 14 Jul 2020 17:34:53 +0200 Message-Id: <20200714153454.9354-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200710134803.4347-1-andreas.rheinhardt@gmail.com> References: <20200710134803.4347-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 6/7] avcodec/h264*: Omit potentially wrong values from log messages X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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 --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c index 1c8585f5af..0aea9757b7 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 51091abbe1..c4b14d2578 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_golomb2(&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_golomb2(&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_golomb2(&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_golomb2(&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 8af094075f..3b62567245 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 5ad311e52e..18e5d9578f 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_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); + 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]) {