[FFmpeg-devel] [PATCH 2/3 v3] 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>
> ---
> Moved the comment for the function to its new place, but otherwise, no
> difference compared to v2.
> 
>  libavcodec/cbs_h2645.c                | 10 ++++++++++
>  libavcodec/cbs_h265_syntax_template.c |  9 +++------
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index d42073cc5a..095e449ddc 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -233,6 +233,16 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
>      return 0;
>  }
>  
> +// payload_extension_present() - true if we are before the last 1-bit
> +// in the payload structure, which must be in the last byte.
> +static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t payload_size,
> +                                              int cur_pos)
> +{
> +    int bits_left = payload_size * 8 - cur_pos;
> +    return (bits_left > 0 &&
> +            (bits_left > 7 || show_bits(gbc, bits_left) & MAX_UINT_BITS(bits_left - 1)));
Consider a buffering period SEI message whose bit position is 7 mod 8
before the payload extension present check and where there is only one
bit, namely a zero bit left in the SEI message. According to the
definition of payload_extension_present() in the standard, it will
Consider a buffering period SEI message whose bit position is 7 mod 8
before the payload extension present check and where there is only one
bit, namely a zero bit left in the SEI message. According to the
definition of payload_extension_present() in the standard, it will
return true and the last bit is use_alt_cpb_params_flag (with a value
zero); yet nevertheless the more_data_in_payload() check after the
buffering_period() returns false. This is a valid SEI message according
to the current version of the standard (in fact, for all versions except
for the very first version).

Yet the check above will return false in this scenario and the SEI
message unit will be considered invalid. I don't object to this, but you
should mention this discrepancy in a comment.

- Andreas

> +}
> +
>  #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 a51b12cfc6..ed08b06e9c 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1572,7 +1572,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, end_pos;
>      start_pos = get_bits_count(rw);
>  #endif
>  
> @@ -1651,12 +1651,9 @@ 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,
> +                                           end_pos - start_pos))
>          flag(use_alt_cpb_params_flag);
>      else
>          infer(use_alt_cpb_params_flag, 0);
>