[FFmpeg-devel] [PATCH 3/4] avcodec/cbs_h265: move the payload_extension_present check into its own function

[Andreas Rheinhardt]

James Almer:
> Will be reused in the following patch.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/cbs_h2645.c                | 9 +++++++++
>  libavcodec/cbs_h265_syntax_template.c | 8 +++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index d42073cc5a..dffff862e2 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -233,6 +233,15 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
>      return 0;
>  }
>  
> +static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t payload_size,
> +                                              int cur_pos)
> +{
> +    int bits_left;
> +    bits_left = payload_size * 8 - cur_pos;
> +    return (bits_left > 0 &&
> +            (bits_left > 7 || ff_ctz(show_bits(gbc, bits_left)) < bits_left - 1));
> +}
> +
>  #define HEADER(name) do { \
>          ff_cbs_trace_header(ctx, name); \
>      } while (0)
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index fe5ffac80f..f978e16549 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1568,7 +1568,7 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw,
>      int err, i, length;
>  
>  #ifdef READ
> -    int start_pos, end_pos, bits_left;
> +    int start_pos;
>      start_pos = get_bits_count(rw);
>  #endif
>  
> @@ -1649,10 +1649,8 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw,
>  #ifdef READ
>      // payload_extension_present() - true if we are before the last 1-bit
>      // in the payload structure, which must be in the last byte.
> -    end_pos = get_bits_count(rw);
> -    bits_left = *payload_size * 8 - (end_pos - start_pos);
> -    if (bits_left > 0 &&
> -        (bits_left > 7 || ff_ctz(show_bits(rw, bits_left)) < bits_left - 1))
> +    if (cbs_h265_payload_extension_present(rw, *payload_size,
> +                                           get_bits_count(rw) - start_pos))
>          flag(use_alt_cpb_params_flag);
>      else
>          infer(use_alt_cpb_params_flag, 0);
> 
The value of ff_ctz is undefined if the argument is zero. It can be zero
for invalid input (namely if all of the bits_left are zero). You should
instead use a check like show_bits(gbc, bits_left) &
MAX_UINT_BITS(bits_left - 1).

(In this situation where you are only reading one bit it doesn't really
matter - invalid input will always be detected as such if ff_ctz returns
something >= 0. But if we use this later in a scenario where it is about
more than just one bit, this can make invalid input slip through.)

This is similar to d4035ca849bdb90e95c87e2737a99ea657be0716.

- Andreas