From patchwork Tue Jul 14 15:34:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21005 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 0E12544AA50 for ; Tue, 14 Jul 2020 18:35:17 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DEA8368B12F; Tue, 14 Jul 2020 18:35:16 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E949E6898F7 for ; Tue, 14 Jul 2020 18:35:09 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id f7so22549385wrw.1 for ; Tue, 14 Jul 2020 08:35:09 -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=1/JxBmUAdSMyS6AfFAadRuKoZzQ8QX8AdyCqZLYGPso=; b=lgGxoSJRGHRgKlz1BEa6evFZW1SEggH15WhlNmZ7XjqWcWPJ9HnmcKv+Jq/lIcOPaW wyRv+yYzQ+f1fRqDzziVuSFFCbHZhRATZ8in93Lkb3UVoEGO1PSFu1xIE+8ravS475c1 JAfkyTlPewFTISMarMzPGtuPynq0g7yG5JtG74qBN3oNJLdu/Cw01vb82tfqoaytMj6D 0efMCTWwJpejvmltfUUt4ZCvYhrTVN6V5usoMib+rN161YicYN9/5SDpBOYcy6wP7AFv DNtSv51544X7zC3zzJvU0pJ10zM2aPlaTJjYMCWpHHpEzrFXS/KWRdEb+lzmpi1HLgjN cXxw== 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=1/JxBmUAdSMyS6AfFAadRuKoZzQ8QX8AdyCqZLYGPso=; b=hecadx6FfAVvRE3Ul9n/l5RgOn28llJxNygWxt2pTZg2Sw7LLa+RY1REnn+PSbymKn 5RMkcJ29oqr54AmYsA5+SB/ULoOvkqtBRNeaacih6CMlEKHCb+9msUeUqNfOgV56nwjA vJTfZaAtvotr5jAcr78RqF3RVt596gTIvyPQkHJZbNM6JXou6Lf/QbzkWaQyMe5okukg K+SPc9kfEx/WD0ZwnQjKy6MlQaZjjgBWPd8oSL8+XRi+jF4gSZJzReY/LhyOr0wrriGa DhJhyJ5ocPwr/aQyWPm53ejc1N422huVzekhR0M4ZKoUOSIaUJhUVQmb/f+7rcAlM41Z G1HQ== X-Gm-Message-State: AOAM530WTWIXchrK4TNLaJtyLShl5/dTl3LYEhU5BeYuSn9Z3x7m3QC3 G2mZVk7JOun4W4d08+Junm4sm/WL X-Google-Smtp-Source: ABdhPJxuTFfA9TYkyjht+uvSJi7u+TlusZzxmpZUwVphAmPbVIFMnuMatx2OKgAcMJfOt5zMlhDelA== X-Received: by 2002:adf:eecf:: with SMTP id a15mr6511349wrp.83.1594740908675; Tue, 14 Jul 2020 08:35:08 -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.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2020 08:35:08 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 14 Jul 2020 17:34:50 +0200 Message-Id: <20200714153454.9354-2-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 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31() 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() uses a LUT of 512 entries; therefore it can be used to parse exp-golomb codes of length <= 9, i.e. those codes with at most four leading bits that have five effective bits; this implies a range of 0..30 and not 31. In particular, this function must not be used to parse e.g. the H.264 SPS id. This commit renames the function to get_ue_golomb_30() and uses it whereever possible. In places where it is not possible, it has been replaced by get_ue_golomb2(). Given that the returned value of said function can be a negative error code, it is no longer included in av_log() statements (the parsed value can actually also be wrong when using get_ue_golomb30()). Signed-off-by: Andreas Rheinhardt --- I have only used get_ue_golomb_30() where I am certain that its range is sufficient; this unfortunately excludes certain cases in h264_cavlc, because for field based content, the number of refs can be in the range of 0..31. libavcodec/golomb.h | 6 +++--- libavcodec/h264_cavlc.c | 14 +++++++------- libavcodec/h264_parser.c | 8 ++++---- libavcodec/h264_ps.c | 18 +++++++++--------- libavcodec/h264_refs.c | 6 +++--- libavcodec/h264_sei.c | 8 ++++---- libavcodec/h264_slice.c | 6 +++--- libavformat/mxfenc.c | 2 +- 8 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h index 63069f63e5..172d9934e0 100644 --- a/libavcodec/golomb.h +++ b/libavcodec/golomb.h @@ -134,10 +134,10 @@ static inline unsigned get_ue_golomb_long(GetBitContext *gb) } /** - * read unsigned exp golomb code, constraint to a max of 31. - * the return value is undefined if the stored value exceeds 31. + * Read unsigned exp golomb code, constraint to a max of 30. + * the return value is undefined if the stored value exceeds 30. */ -static inline int get_ue_golomb_31(GetBitContext *gb) +static inline int get_ue_golomb_30(GetBitContext *gb) { unsigned int buf; diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c index 6481992e58..51091abbe1 100644 --- a/libavcodec/h264_cavlc.c +++ b/libavcodec/h264_cavlc.c @@ -838,7 +838,7 @@ decode_intra_mb: } if(decode_chroma){ pred_mode= ff_h264_check_intra_pred_mode(h->avctx, sl->top_samples_available, - sl->left_samples_available, get_ue_golomb_31(&sl->gb), 1); + sl->left_samples_available, get_ue_golomb_30(&sl->gb), 1); if(pred_mode < 0) return -1; sl->chroma_pred_mode = pred_mode; @@ -850,7 +850,7 @@ decode_intra_mb: if (sl->slice_type_nos == AV_PICTURE_TYPE_B) { for(i=0; i<4; i++){ - sl->sub_mb_type[i]= get_ue_golomb_31(&sl->gb); + sl->sub_mb_type[i] = get_ue_golomb_30(&sl->gb); if(sl->sub_mb_type[i] >=13){ av_log(h->avctx, AV_LOG_ERROR, "B sub_mb_type %u out of range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y); return -1; @@ -868,7 +868,7 @@ decode_intra_mb: }else{ av_assert2(sl->slice_type_nos == AV_PICTURE_TYPE_P); //FIXME SP correct ? for(i=0; i<4; i++){ - sl->sub_mb_type[i]= get_ue_golomb_31(&sl->gb); + sl->sub_mb_type[i] = get_ue_golomb_30(&sl->gb); if(sl->sub_mb_type[i] >=4){ av_log(h->avctx, AV_LOG_ERROR, "P sub_mb_type %u out of range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y); return -1; @@ -889,7 +889,7 @@ decode_intra_mb: }else if(ref_count == 2){ tmp= get_bits1(&sl->gb)^1; }else{ - tmp= get_ue_golomb_31(&sl->gb); + tmp = get_ue_golomb2(&sl->gb); if(tmp>=ref_count){ av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", tmp); return -1; @@ -965,7 +965,7 @@ decode_intra_mb: } else if (rc == 2) { val= get_bits1(&sl->gb)^1; }else{ - val= get_ue_golomb_31(&sl->gb); + val = get_ue_golomb2(&sl->gb); if (val >= rc) { av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; @@ -996,7 +996,7 @@ decode_intra_mb: } else if (rc == 2) { val= get_bits1(&sl->gb)^1; }else{ - val= get_ue_golomb_31(&sl->gb); + val = get_ue_golomb2(&sl->gb); if (val >= rc) { av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; @@ -1034,7 +1034,7 @@ decode_intra_mb: } else if (rc == 2) { val= get_bits1(&sl->gb)^1; }else{ - val= get_ue_golomb_31(&sl->gb); + val = get_ue_golomb2(&sl->gb); if (val >= rc) { av_log(h->avctx, AV_LOG_ERROR, "ref %u overflow\n", val); return -1; diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index aacd44cf3b..8af094075f 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -180,7 +180,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb, if (get_bits1(gb)) { int index; for (index = 0; ; index++) { - unsigned int reordering_of_pic_nums_idc = get_ue_golomb_31(gb); + unsigned reordering_of_pic_nums_idc = get_ue_golomb_30(gb); if (reordering_of_pic_nums_idc < 3) get_ue_golomb_long(gb); @@ -210,7 +210,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb, if (get_bits1(gb)) { // adaptive_ref_pic_marking_mode_flag int i; for (i = 0; i < MAX_MMCO_COUNT; i++) { - MMCOOpcode opcode = get_ue_golomb_31(gb); + MMCOOpcode opcode = get_ue_golomb_30(gb); if (opcode > (unsigned) MMCO_LONG) { av_log(logctx, AV_LOG_ERROR, "illegal memory management control operation %d\n", @@ -226,7 +226,7 @@ static int scan_mmco_reset(AVCodecParserContext *s, GetBitContext *gb, get_ue_golomb_long(gb); // difference_of_pic_nums_minus1 if (opcode == MMCO_SHORT2LONG || opcode == MMCO_LONG2UNUSED || opcode == MMCO_LONG || opcode == MMCO_SET_MAX_LONG) - get_ue_golomb_31(gb); + get_ue_golomb2(gb); } } @@ -342,7 +342,7 @@ static inline int parse_nal_units(AVCodecParserContext *s, /* fall through */ case H264_NAL_SLICE: get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice - slice_type = get_ue_golomb_31(&nal.gb); + slice_type = get_ue_golomb_30(&nal.gb); s->pict_type = ff_h264_golomb_to_pict_type[slice_type % 5]; if (p->sei.recovery_point.recovery_frame_cnt >= 0) { /* key frame, since recovery_frame_cnt is set */ diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c index e774929e21..13823b8dc0 100644 --- a/libavcodec/h264_ps.c +++ b/libavcodec/h264_ps.c @@ -108,10 +108,10 @@ static inline int decode_hrd_parameters(GetBitContext *gb, void *logctx, SPS *sps) { int cpb_count, i; - cpb_count = get_ue_golomb_31(gb) + 1; + cpb_count = get_ue_golomb2(gb); - if (cpb_count > 32U) { - av_log(logctx, AV_LOG_ERROR, "cpb_count %d invalid\n", cpb_count); + if (cpb_count++ > 31U) { + av_log(logctx, AV_LOG_ERROR, "cpb_count invalid\n"); return AVERROR_INVALIDDATA; } @@ -361,10 +361,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, constraint_set_flags |= get_bits1(gb) << 5; // constraint_set5_flag skip_bits(gb, 2); // reserved_zero_2bits level_idc = get_bits(gb, 8); - sps_id = get_ue_golomb_31(gb); + sps_id = get_ue_golomb2(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 out of range\n"); goto fail; } @@ -391,7 +391,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, sps->profile_idc == 128 || // Multiview High profile (MVC) sps->profile_idc == 138 || // Multiview Depth High profile (MVCD) sps->profile_idc == 144) { // old High444 profile - sps->chroma_format_idc = get_ue_golomb_31(gb); + sps->chroma_format_idc = get_ue_golomb_30(gb); if (sps->chroma_format_idc > 3U) { avpriv_request_sample(avctx, "chroma_format_idc %u", sps->chroma_format_idc); @@ -438,7 +438,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, } sps->log2_max_frame_num = log2_max_frame_num_minus4 + 4; - sps->poc_type = get_ue_golomb_31(gb); + sps->poc_type = get_ue_golomb_30(gb); if (sps->poc_type == 0) { // FIXME #define unsigned t = get_ue_golomb(gb); @@ -482,7 +482,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, goto fail; } - sps->ref_frame_count = get_ue_golomb_31(gb); + sps->ref_frame_count = get_ue_golomb_30(gb); if (avctx->codec_tag == MKTAG('S', 'M', 'V', '2')) sps->ref_frame_count = FFMAX(2, sps->ref_frame_count); if (sps->ref_frame_count > MAX_DELAYED_PIC_COUNT) { @@ -781,7 +781,7 @@ 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); + 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); diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c index dae8bd278a..4ceaaa3863 100644 --- a/libavcodec/h264_refs.c +++ b/libavcodec/h264_refs.c @@ -432,7 +432,7 @@ int ff_h264_decode_ref_pic_list_reordering(H264SliceContext *sl, void *logctx) continue; for (index = 0; ; index++) { - unsigned int op = get_ue_golomb_31(&sl->gb); + unsigned int op = get_ue_golomb_30(&sl->gb); if (op == 3) break; @@ -850,7 +850,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl, GetBitContext *gb, sl->explicit_ref_marking = get_bits1(gb); if (sl->explicit_ref_marking) { for (i = 0; i < MAX_MMCO_COUNT; i++) { - MMCOOpcode opcode = get_ue_golomb_31(gb); + MMCOOpcode opcode = get_ue_golomb_30(gb); mmco[i].opcode = opcode; if (opcode == MMCO_SHORT2UNUSED || opcode == MMCO_SHORT2LONG) { @@ -860,7 +860,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl, GetBitContext *gb, } if (opcode == MMCO_SHORT2LONG || opcode == MMCO_LONG2UNUSED || opcode == MMCO_LONG || opcode == MMCO_SET_MAX_LONG) { - unsigned int long_arg = get_ue_golomb_31(gb); + unsigned int long_arg = get_ue_golomb2(gb); if (long_arg >= 32 || (long_arg >= 16 && !(opcode == MMCO_SET_MAX_LONG && long_arg == 16) && diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index 7b8e6bd7ba..63d43b2bc5 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -320,11 +320,11 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb, int sched_sel_idx; const SPS *sps; - sps_id = get_ue_golomb_31(gb); - if (sps_id > 31 || !ps->sps_list[sps_id]) { + sps_id = get_ue_golomb2(gb); + if (sps_id > 31U || !ps->sps_list[sps_id]) { av_log(logctx, AV_LOG_ERROR, - "non-existing SPS %d referenced in buffering period\n", sps_id); - return sps_id > 31 ? AVERROR_INVALIDDATA : AVERROR_PS_NOT_FOUND; + "non-existing SPS referenced in buffering period\n"); + return sps_id > 31U ? 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..8f4b7ef1ec 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -1739,7 +1739,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl, sl->first_mb_addr = get_ue_golomb_long(&sl->gb); - slice_type = get_ue_golomb_31(&sl->gb); + slice_type = get_ue_golomb_30(&sl->gb); if (slice_type > 9) { av_log(h->avctx, AV_LOG_ERROR, "slice type %d too large at %d\n", @@ -1874,7 +1874,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl, } if (sl->slice_type_nos != AV_PICTURE_TYPE_I && pps->cabac) { - tmp = get_ue_golomb_31(&sl->gb); + tmp = get_ue_golomb_30(&sl->gb); if (tmp > 2) { av_log(h->avctx, AV_LOG_ERROR, "cabac_init_idc %u overflow\n", tmp); return AVERROR_INVALIDDATA; @@ -1902,7 +1902,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl, sl->slice_alpha_c0_offset = 0; sl->slice_beta_offset = 0; if (pps->deblocking_filter_parameters_present) { - tmp = get_ue_golomb_31(&sl->gb); + tmp = get_ue_golomb_30(&sl->gb); if (tmp > 2) { av_log(h->avctx, AV_LOG_ERROR, "deblocking_filter_idc %u out of range\n", tmp); diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 5a3a609bf6..c7ceb40b55 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2223,7 +2223,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, case H264_NAL_SLICE: init_get_bits8(&gb, buf, buf_end - buf); get_ue_golomb_long(&gb); // skip first_mb_in_slice - slice_type = get_ue_golomb_31(&gb); + slice_type = get_ue_golomb_30(&gb); switch (slice_type % 5) { case 0: e->flags |= 0x20; // P Picture