[FFmpeg-devel] [PATCH] cbs_h265: Detect more reference combinations which would overflow the DPB

[Mark Thompson]

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