[FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Tue Jul 14 18:34:50 EEST 2020
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 <andreas.rheinhardt at gmail.com>
---
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
--
2.20.1
More information about the ffmpeg-devel
mailing list