From patchwork Mon Jul 27 09:08:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21280 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 0FE7F44983A for ; Mon, 27 Jul 2020 12:08:32 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F043868B792; Mon, 27 Jul 2020 12:08:31 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7ED3368B7AB for ; Mon, 27 Jul 2020 12:08:23 +0300 (EEST) Received: by mail-ej1-f46.google.com with SMTP id dk23so4403593ejb.11 for ; Mon, 27 Jul 2020 02:08:23 -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=Zp3OJXlg4oKMAmFqeVcA44tNshHX3A9+YYi5xaxhgGA=; b=VYAg1dogX8u/S4hIaaNFFwHz4SeV4oJY9nf4xn/GpVgTGJKLVdAuwXYy3oc+Xr5s2L XmKXau9LxE+oifmKkQzs5nXBueY1xmA2g5i/jPnnscIpv3CwLI21Id2/hVTEuqIxduUR 5qmhm+Qi9TtnR/QT3+dIiwW8muOXIWgcjdkvKKhS1ExNYegxzc5lORoqFF7zuCD8oJYS T4qlEZ0iZuIBC7yudu2szXzJ3waMm9xCVS9X+tTYrqcEnjgmaCJ7DgNROAxAq05GkKFR Cd98HBd0Bs5AHJwY04YPIRNRdWbtVU2apE/UBiIMCEOtD7uihNsn+wH9F0DdB5a9x0AQ 61LA== 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=Zp3OJXlg4oKMAmFqeVcA44tNshHX3A9+YYi5xaxhgGA=; b=rSq563pG+RTlU8N1PNDgBiOjz07bFFBTaE/t3pAeWCggzSJDTw6etiGWdmXWckwCoD llLf24cTBdgnOn1MiyYAYWtUSlS5lDRO+Qs0ptZLvzlyiSeJzNWuoJlnsJjR9S9P601m BIGPlNFDPU/QNt4ETqybNP7MUSj7sF6lRLkLDRtDa9Fxh5WCOoggCn2RxYwLfCx7XmuQ 8ZbeQgWGbb3mP+6GHUzEihL6LUBoIr+zWGhTpChmhGKyclj5oh0casBFENQYNc88P+qW e+YspuJpvU+4UveDLm/UlCDnVZz76Ok91q7RJJQvYyppbgrSMwJ3aWlMNAlhwHLKfCeI M9EA== X-Gm-Message-State: AOAM533gf0wGICrE/jzasqhkkLLotG+7m8SKuVO1pqUcNAufXlW9jMHD jjD5UnNE7KACKhXMwztAghA9gIdr X-Google-Smtp-Source: ABdhPJw+KPTBo+Aewr5fftg/kspgXhj78mq0IGwlScwacVNLv7oeqIMhLBN8gerszt9G7INVOX7Edg== X-Received: by 2002:a17:906:6852:: with SMTP id a18mr19683900ejs.348.1595840902364; Mon, 27 Jul 2020 02:08:22 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id ck6sm7080961edb.18.2020.07.27.02.08.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jul 2020 02:08:21 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 27 Jul 2020 11:08:10 +0200 Message-Id: <20200727090810.23794-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200727090810.23794-1-andreas.rheinhardt@gmail.com> References: <20200727090810.23794-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v3 5/5] 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_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 --- 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]) {