From patchwork Tue Jul 14 20:19:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21017 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 DBAB144A518 for ; Tue, 14 Jul 2020 23:20:35 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C928468A370; Tue, 14 Jul 2020 23:20:35 +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 9699168AD86 for ; Tue, 14 Jul 2020 23:20:29 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id j4so24531566wrp.10 for ; Tue, 14 Jul 2020 13:20:29 -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=9BRaJoc7aYZ0fAvZa8kBaUyUfnWUm87HtjkJmq9EEGQ=; b=LTCgXckKv/lMnEovBQUmZFybTnIocuAJccHpCCJYTp/gd59p5MEd/Lhr271B7w/f2u jRZja+R5dhxJ01wQSWeUhNaPHQY31KrLTB3bwTRFMVWFTLUtxiEOslT5ebXcrJpc0Q6y /7U3gYIw3pCdA4vHKFfQyDZKGZnGa2isRWadx2m8rDYwmq8gfncGxslfyzxkQrvuy1jF F4j/VGNsAL8Rclapn3mpKAe1SeAEC2mFwWdCqJ8kV+iL5jhyT16ydZl3jrrzwyvUJso0 sI0JGfeoFII42965uyixC5gs0BGe5aB57RBTwcGnoLCHXelYCqfN8kWhiPXyOTXZ3s6P +haQ== 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=9BRaJoc7aYZ0fAvZa8kBaUyUfnWUm87HtjkJmq9EEGQ=; b=eipRwFRrhNWPqC82oy9PTwtWD+TsdyQbHa91mlRGQmcWX5IcxZFGp/R0VJwA6mu2AG 5FBCBXgy05KPomf0wnbNlL3r05sGvUFO1cGbrVAMg0IA/OwmlACG0eeJPIU85Ee8gB/w f6Tx0Hbd/2oSuPy1V5KfLu8EdfgtIYXKBm/GVyAXBu2F7qo1wwX24IG/62K+P/HMcNPx qQv1Dl3cC89bJvuJlZk+965PpESv1Thu9Ro11igZsNYPxf/JTL/aakaHGkOrbWHET1zA EqMuSLf38ymqefvo/wtSgx209CSRytfWSLb7L4BlcbE7YYFvyQJ27qqTXrKh3Z67PVAx 9krA== X-Gm-Message-State: AOAM5306/odnQxB3cjfePH1L1n3eESZ6i9rTwD8Nn807iY1vu1bjkgg3 6tETE2PoF8xM88lNRO2pE2G4DcJ+ X-Google-Smtp-Source: ABdhPJyAoMzuH1uiVYEd1+ctaQjfopDsl7IgA7+U5ClYpyw/UyhxQmheQni4DcHaYvJEesn48to75A== X-Received: by 2002:adf:e850:: with SMTP id d16mr7946773wrn.426.1594758028445; Tue, 14 Jul 2020 13:20:28 -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.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2020 13:20:27 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 14 Jul 2020 22:19:51 +0200 Message-Id: <20200714201954.30327-3-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 3/6] 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 --- 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 | 4 ++-- libavcodec/h264_slice.c | 6 +++--- libavformat/mxfenc.c | 2 +- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h index a53486d7cf..775ac9878d 100644 --- a/libavcodec/golomb.h +++ b/libavcodec/golomb.h @@ -114,10 +114,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..48f0f0689d 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_golomb(&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_golomb(&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_golomb(&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_golomb(&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..d0269497d5 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_golomb(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..c1a79bde53 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_golomb(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_golomb(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_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); diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c index dae8bd278a..3cba2646e7 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_golomb(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..70d8f284ee 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -320,10 +320,10 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb, int sched_sel_idx; const SPS *sps; - sps_id = get_ue_golomb_31(gb); + sps_id = get_ue_golomb(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..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