From patchwork Tue Jul 14 20:19:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21020 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 1575344A518 for ; Tue, 14 Jul 2020 23:20:41 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F067068AF9D; Tue, 14 Jul 2020 23:20:40 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id EDF6068AEEB for ; Tue, 14 Jul 2020 23:20:33 +0300 (EEST) Received: by mail-wm1-f43.google.com with SMTP id q15so198086wmj.2 for ; Tue, 14 Jul 2020 13:20:33 -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=UkkjiaidesvIEy6meLLbvGVMh8hc1GH42LofMS8WDic=; b=sNqFhsE2eiySDNH7zNUcVP6RdvWNMPRCvL2txhzQb4OTHBJ1JNPlL4MCo84xZ3P3La tI8Cb2CpPXPyxyKvi1iafOrQ9YIOx98qhUpoctn3c4FPYHme2FIyFhSTR9420k3pctiX BGzGdu7EhaaRJvt2GofQKb85l/Uf+OlHh/1J4LB/AEFhTz1Q2ali3NI1FOc9L9QCwFTq fiukifDg8egPVvugr1lqqNMrfOZnuIGoiO7oUAaqs3phCdOJKu6nnfgtdovvXbmxHgYq IQ+y84NUMWuotfEzKu5HQ50AGP0HyYK4nA5YLv44K6CIT0P4R1jtaIiXu7GA7QytWaLy YBaw== 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=UkkjiaidesvIEy6meLLbvGVMh8hc1GH42LofMS8WDic=; b=JU5AD8/rtaSqmyB9nfSiwBiJLtfnXuVnshL0iRCdF7z6O1NRseJqYXz7FNu/sBtYBl JiQi+ZZmF9gz0pohMSpt7V1l6NmrkopbgzDYREzsiPQVjNdl9Z5z+Lb6cNHptuRSkBmb Yu9Psq4fVn2qgQvqbw74J5Csiwq1GCQ9LPnjUSj43050yOswcuvrQle8Jk4j3jzu21+w vtj9yFKd/vWS8rFVWUNrLzWfYaZzgDBXBKQYcU9VSW75IHhBQwWst7wDKRrauMdUEBgk q2jMjWdd/L4kWmS4MarS81TwdBXR2ZDHfqNOAWEEgo7sqpxBjop9uA2oih70XstKUFyo mSDg== X-Gm-Message-State: AOAM531HrksV4v9HUe1PvgpO/l+ZEbI/BP98hyrZBTTBi1oDcue9PlgC rSYRNLy/b8R6+7borDeS55ClEvbm X-Google-Smtp-Source: ABdhPJxBG7hya5Zp/NSE/xlVIZHYZocYzlbtPXpLd34RmAhnz5vuft8eb+vidYz/yg2gZ3AOHhpK3Q== X-Received: by 2002:a7b:cbc1:: with SMTP id n1mr5529148wmi.18.1594758032712; Tue, 14 Jul 2020 13:20:32 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id j15sm29462355wrx.69.2020.07.14.13.20.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2020 13:20:31 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 14 Jul 2020 22:19:54 +0200 Message-Id: <20200714201954.30327-6-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200714201954.30327-1-andreas.rheinhardt@gmail.com> References: <20200714201954.30327-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 6/6] 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 --- 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 --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]) {