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

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(-)

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