Message ID | 2d7cf9d5-2939-64ee-61b4-cd8fc68369f3@jkqxz.net |
---|---|
State | Accepted |
Headers | show |
Series | [FFmpeg-devel] cbs_h265: Detect more reference combinations which would overflow the DPB | expand |
Context | Check | Description |
---|---|---|
andriy/configure | warning | Failed to apply patch |
On 03/02/2021 21:34, Mark Thompson wrote: > In total, the number of short term references (from the selected short > term ref pic set), the number of long term references (combining both the > used candidates from the SPS and those defined in the slice header) and > the number of instances of the current picture (usually one, but can be > two if current picture reference is enabled) must never exceed the size > of the DPB. This is a generalisation of the condition associated with > num_long_term_pics in 7.4.7.1. > > We use this to apply tighter bounds to the number of long term pictures > referred to in the slice header, and also to detect the invalid case where > the second reference to the current picture would not fit in the DPB (this > case can't be detected earlier because an STRPS with 15 pictures can still > be valid in the same stream when used with a different PPS which does not > require two DPB slots for the current picture). > --- > Michael: does this fix your fuzz case for num_long_term_sps? > > Thanks, > > - Mark > > > libavcodec/cbs_h265_syntax_template.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c > index d09934cfeb..5d216aad36 100644 > --- a/libavcodec/cbs_h265_syntax_template.c > +++ b/libavcodec/cbs_h265_syntax_template.c > @@ -1369,6 +1369,7 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, > if (current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_W_RADL && > current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_N_LP) { > const H265RawSTRefPicSet *rps; > + int dpb_slots_remaining; > > ub(sps->log2_max_pic_order_cnt_lsb_minus4 + 4, slice_pic_order_cnt_lsb); > > @@ -1387,6 +1388,22 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, > rps = &sps->st_ref_pic_set[0]; > } > > + dpb_slots_remaining = HEVC_MAX_DPB_SIZE - 1 - > + rps->num_negative_pics - rps->num_positive_pics; > + if (pps->pps_curr_pic_ref_enabled_flag && > + (sps->sample_adaptive_offset_enabled_flag || > + !pps->pps_deblocking_filter_disabled_flag || > + pps->deblocking_filter_override_enabled_flag)) { > + // This picture will occupy two DPB slots. > + if (dpb_slots_remaining == 0) { > + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: " > + "short-term ref pic set contains too many pictures " > + "to use with current picture reference enabled.\n"); > + return AVERROR_INVALIDDATA; > + } > + --dpb_slots_remaining; > + } > + > num_pic_total_curr = 0; > for (i = 0; i < rps->num_negative_pics; i++) > if (rps->used_by_curr_pic_s0_flag[i]) > @@ -1399,13 +1416,15 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, > unsigned int idx_size; > > if (sps->num_long_term_ref_pics_sps > 0) { > - ue(num_long_term_sps, 0, sps->num_long_term_ref_pics_sps); > + ue(num_long_term_sps, 0, FFMIN(sps->num_long_term_ref_pics_sps, > + dpb_slots_remaining)); > idx_size = av_log2(sps->num_long_term_ref_pics_sps - 1) + 1; > + dpb_slots_remaining -= current->num_long_term_sps; > } else { > infer(num_long_term_sps, 0); > idx_size = 0; > } > - ue(num_long_term_pics, 0, HEVC_MAX_REFS - current->num_long_term_sps); > + ue(num_long_term_pics, 0, dpb_slots_remaining); > > for (i = 0; i < current->num_long_term_sps + > current->num_long_term_pics; i++) { Ping. - Mark
On Wed, Feb 03, 2021 at 09:34:07PM +0000, Mark Thompson wrote: > In total, the number of short term references (from the selected short > term ref pic set), the number of long term references (combining both the > used candidates from the SPS and those defined in the slice header) and > the number of instances of the current picture (usually one, but can be > two if current picture reference is enabled) must never exceed the size > of the DPB. This is a generalisation of the condition associated with > num_long_term_pics in 7.4.7.1. > > We use this to apply tighter bounds to the number of long term pictures > referred to in the slice header, and also to detect the invalid case where > the second reference to the current picture would not fit in the DPB (this > case can't be detected earlier because an STRPS with 15 pictures can still > be valid in the same stream when used with a different PPS which does not > require two DPB slots for the current picture). > --- > Michael: does this fix your fuzz case for num_long_term_sps? yes thanks [...]
On 21/02/2021 20:54, Michael Niedermayer wrote: > On Wed, Feb 03, 2021 at 09:34:07PM +0000, Mark Thompson wrote: >> In total, the number of short term references (from the selected short >> term ref pic set), the number of long term references (combining both the >> used candidates from the SPS and those defined in the slice header) and >> the number of instances of the current picture (usually one, but can be >> two if current picture reference is enabled) must never exceed the size >> of the DPB. This is a generalisation of the condition associated with >> num_long_term_pics in 7.4.7.1. >> >> We use this to apply tighter bounds to the number of long term pictures >> referred to in the slice header, and also to detect the invalid case where >> the second reference to the current picture would not fit in the DPB (this >> case can't be detected earlier because an STRPS with 15 pictures can still >> be valid in the same stream when used with a different PPS which does not >> require two DPB slots for the current picture). >> --- > >> Michael: does this fix your fuzz case for num_long_term_sps? > > yes Added appropriate annotation and applied. Thanks, - Mark
diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c index d09934cfeb..5d216aad36 100644 --- a/libavcodec/cbs_h265_syntax_template.c +++ b/libavcodec/cbs_h265_syntax_template.c @@ -1369,6 +1369,7 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, if (current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_W_RADL && current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_N_LP) { const H265RawSTRefPicSet *rps; + int dpb_slots_remaining; ub(sps->log2_max_pic_order_cnt_lsb_minus4 + 4, slice_pic_order_cnt_lsb); @@ -1387,6 +1388,22 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, rps = &sps->st_ref_pic_set[0]; } + dpb_slots_remaining = HEVC_MAX_DPB_SIZE - 1 - + rps->num_negative_pics - rps->num_positive_pics; + if (pps->pps_curr_pic_ref_enabled_flag && + (sps->sample_adaptive_offset_enabled_flag || + !pps->pps_deblocking_filter_disabled_flag || + pps->deblocking_filter_override_enabled_flag)) { + // This picture will occupy two DPB slots. + if (dpb_slots_remaining == 0) { + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: " + "short-term ref pic set contains too many pictures " + "to use with current picture reference enabled.\n"); + return AVERROR_INVALIDDATA; + } + --dpb_slots_remaining; + } + num_pic_total_curr = 0; for (i = 0; i < rps->num_negative_pics; i++) if (rps->used_by_curr_pic_s0_flag[i]) @@ -1399,13 +1416,15 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, unsigned int idx_size; if (sps->num_long_term_ref_pics_sps > 0) { - ue(num_long_term_sps, 0, sps->num_long_term_ref_pics_sps); + ue(num_long_term_sps, 0, FFMIN(sps->num_long_term_ref_pics_sps, + dpb_slots_remaining)); idx_size = av_log2(sps->num_long_term_ref_pics_sps - 1) + 1; + dpb_slots_remaining -= current->num_long_term_sps; } else { infer(num_long_term_sps, 0); idx_size = 0; } - ue(num_long_term_pics, 0, HEVC_MAX_REFS - current->num_long_term_sps); + ue(num_long_term_pics, 0, dpb_slots_remaining); for (i = 0; i < current->num_long_term_sps + current->num_long_term_pics; i++) {