[FFmpeg-devel] [PATCH 5/5] avcodec/cbs_av1: use the new signed value read/write functions

[Mark Thompson]

On 15/04/2019 22:17, James Almer wrote:
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> The sample https://0x0.st/sljR.webm appears to be parsed the exact same way
> after this patch.
> 
>  libavcodec/cbs_av1.c | 68 ++++++--------------------------------------
>  1 file changed, 8 insertions(+), 60 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 41dd4c97ac..eb2d03ef43 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -207,62 +207,6 @@ static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc,
>      return 0;
>  }
>  
> -static int cbs_av1_read_su(CodedBitstreamContext *ctx, GetBitContext *gbc,
> -                           int width, const char *name,
> -                           const int *subscripts, int32_t *write_to)
> -{
> -    int position;
> -    int32_t value;
> -
> -    if (ctx->trace_enable)
> -        position = get_bits_count(gbc);
> -
> -    if (get_bits_left(gbc) < width) {
> -        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at "
> -               "%s: bitstream ended.\n", name);
> -        return AVERROR_INVALIDDATA;
> -    }
> -
> -    value = get_sbits(gbc, width);
> -
> -    if (ctx->trace_enable) {
> -        char bits[33];
> -        int i;
> -        for (i = 0; i < width; i++)
> -            bits[i] = value & (1 << (width - i - 1)) ? '1' : '0';

Ohright, it was bad here too.  (But only ever used on small values in AV1, so it couldn't actually be hit.)

> -        bits[i] = 0;
> -
> -        ff_cbs_trace_syntax_element(ctx, position,
> -                                    name, subscripts, bits, value);
> -    }
> -
> -    *write_to = value;
> -    return 0;
> -}
> -
> -static int cbs_av1_write_su(CodedBitstreamContext *ctx, PutBitContext *pbc,
> -                            int width, const char *name,
> -                            const int *subscripts, int32_t value)
> -{
> -    if (put_bits_left(pbc) < width)
> -        return AVERROR(ENOSPC);
> -
> -    if (ctx->trace_enable) {
> -        char bits[33];
> -        int i;
> -        for (i = 0; i < width; i++)
> -            bits[i] = value & (1 << (width - i - 1)) ? '1' : '0';
> -        bits[i] = 0;
> -
> -        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
> -                                    name, subscripts, bits, value);
> -    }
> -
> -    put_sbits(pbc, width, value);
> -
> -    return 0;
> -}
> -
>  static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc,
>                             uint32_t n, const char *name,
>                             const int *subscripts, uint32_t *write_to)
> @@ -639,8 +583,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  
>  #define xsu(width, name, var, subs, ...) do { \
>          int32_t value = 0; \
> -        CHECK(cbs_av1_read_su(ctx, rw, width, #name, \
> -                              SUBSCRIPTS(subs, __VA_ARGS__), &value)); \
> +        CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \
> +                                 SUBSCRIPTS(subs, __VA_ARGS__), &value, \
> +                                 MIN_INT_BITS(width), \
> +                                 MAX_INT_BITS(width))); \
>          var = value; \
>      } while (0)
>  
> @@ -723,8 +669,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>      } while (0)
>  
>  #define xsu(width, name, var, subs, ...) do { \
> -        CHECK(cbs_av1_write_su(ctx, rw, width, #name, \
> -                               SUBSCRIPTS(subs, __VA_ARGS__), var)); \
> +        CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \
> +                                  SUBSCRIPTS(subs, __VA_ARGS__), var, \
> +                                  MIN_INT_BITS(width), \
> +                                  MAX_INT_BITS(width))); \
>      } while (0)
>  
>  #define uvlc(name, range_min, range_max) do { \
> 

LGTM.

Thanks,

- Mark