Re: [FFmpeg-devel] [PATCH 1/7] proresdec2: port and fix for cached reader

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Christophe Gisquet:
> Summary of changes
> - move back to regular, non-macro, get_bits API
> - reduce the lookup to switch the coding method
> - shorter reads wherever possible, in particular for the end of bitstream
>   (16 bits instead of 32, as per the above)
> 
> There are cases that really need longer lengths (larger EG codes) of up
> to 27 bits.
> 
> Win64: 6.10 -> 4.87 (~20% speedup)
> 
> Reference for an hypothetical 32bits version of the cached reader:
> Win32: 11.4 -> 9.8  (14%, because iDCT is not SIMDed)
> ---

The commit message claims to fix something; what does it fix?
Also, changing to the non-macro API should be done in a separate commit
to the one changing the type of bitstream reader.

Furthermore, you should also provide information about the code size
impact when switching the type of reader.

>  libavcodec/proresdec2.c | 53 ++++++++++++++++++-----------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)
> 
> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> index 9297860946..6e243cfc17 100644
> --- a/libavcodec/proresdec2.c
> +++ b/libavcodec/proresdec2.c
> @@ -24,9 +24,7 @@
>   * Known FOURCCs: 'apch' (HQ), 'apcn' (SD), 'apcs' (LT), 'apco' (Proxy), 'ap4h' (4444), 'ap4x' (4444 XQ)
>   */
>  
> -//#define DEBUG
> -
> -#define LONG_BITSTREAM_READER
> +#define CACHED_BITSTREAM_READER 1
>  
>  #include "config_components.h"
>  
> @@ -422,35 +420,37 @@ static int decode_picture_header(AVCodecContext *avctx, const uint8_t *buf, cons
>      return pic_data_size;
>  }
>  
> -#define DECODE_CODEWORD(val, codebook, SKIP)                            \
> +/* bitstream_read may fail on 32bits ARCHS for >24 bits, so use long version there */
> +#if 0 //BITSTREAM_BITS == 32
> +# define READ_BITS get_bits_long
> +#else
> +# define READ_BITS get_bits
> +#endif
> +
> +#define DECODE_CODEWORD(val, codebook)                                  \
>      do {                                                                \
>          unsigned int rice_order, exp_order, switch_bits;                \
>          unsigned int q, buf, bits;                                      \
>                                                                          \
> -        UPDATE_CACHE(re, gb);                                           \
> -        buf = GET_CACHE(re, gb);                                        \
> +        buf = show_bits(gb, 14);                                        \
>                                                                          \
>          /* number of bits to switch between rice and exp golomb */      \
>          switch_bits =  codebook & 3;                                    \
>          rice_order  =  codebook >> 5;                                   \
>          exp_order   = (codebook >> 2) & 7;                              \
>                                                                          \
> -        q = 31 - av_log2(buf);                                          \
> +        q = 13 - av_log2(buf);                                          \
>                                                                          \
>          if (q > switch_bits) { /* exp golomb */                         \
>              bits = exp_order - switch_bits + (q<<1);                    \
> -            if (bits > FFMIN(MIN_CACHE_BITS, 31))                       \
> -                return AVERROR_INVALIDDATA;                             \
> -            val = SHOW_UBITS(re, gb, bits) - (1 << exp_order) +         \
> +            val = READ_BITS(gb, bits) - (1 << exp_order) +              \
>                  ((switch_bits + 1) << rice_order);                      \
> -            SKIP(re, gb, bits);                                         \
>          } else if (rice_order) {                                        \
> -            SKIP_BITS(re, gb, q+1);                                     \
> -            val = (q << rice_order) + SHOW_UBITS(re, gb, rice_order);   \
> -            SKIP(re, gb, rice_order);                                   \
> +            skip_remaining(gb, q+1);                                    \
> +            val = (q << rice_order) + get_bits(gb, rice_order);         \
>          } else {                                                        \
>              val = q;                                                    \
> -            SKIP(re, gb, q+1);                                          \
> +            skip_remaining(gb, q+1);                                    \
>          }                                                               \
>      } while (0)
>  
> @@ -466,9 +466,7 @@ static av_always_inline int decode_dc_coeffs(GetBitContext *gb, int16_t *out,
>      int16_t prev_dc;
>      int code, i, sign;
>  
> -    OPEN_READER(re, gb);
> -
> -    DECODE_CODEWORD(code, FIRST_DC_CB, LAST_SKIP_BITS);
> +    DECODE_CODEWORD(code, FIRST_DC_CB);
>      prev_dc = TOSIGNED(code);
>      out[0] = prev_dc;
>  
> @@ -477,13 +475,12 @@ static av_always_inline int decode_dc_coeffs(GetBitContext *gb, int16_t *out,
>      code = 5;
>      sign = 0;
>      for (i = 1; i < blocks_per_slice; i++, out += 64) {
> -        DECODE_CODEWORD(code, dc_codebook[FFMIN(code, 6U)], LAST_SKIP_BITS);
> +        DECODE_CODEWORD(code, dc_codebook[FFMIN(code, 6U)]);
>          if(code) sign ^= -(code & 1);
>          else     sign  = 0;
>          prev_dc += (((code + 1) >> 1) ^ sign) - sign;
>          out[0] = prev_dc;
>      }
> -    CLOSE_READER(re, gb);
>      return 0;
>  }
>  
> @@ -497,11 +494,9 @@ static av_always_inline int decode_ac_coeffs(AVCodecContext *avctx, GetBitContex
>      const ProresContext *ctx = avctx->priv_data;
>      int block_mask, sign;
>      unsigned pos, run, level;
> -    int max_coeffs, i, bits_left;
> +    int max_coeffs, i, bits_rem;
>      int log2_block_count = av_log2(blocks_per_slice);
>  
> -    OPEN_READER(re, gb);
> -    UPDATE_CACHE(re, gb);                                           \
>      run   = 4;
>      level = 2;
>  
> @@ -509,28 +504,26 @@ static av_always_inline int decode_ac_coeffs(AVCodecContext *avctx, GetBitContex
>      block_mask = blocks_per_slice - 1;
>  
>      for (pos = block_mask;;) {
> -        bits_left = gb->size_in_bits - re_index;
> -        if (!bits_left || (bits_left < 32 && !SHOW_UBITS(re, gb, bits_left)))
> +        bits_rem = get_bits_left(gb);
> +        if (!bits_rem || (bits_rem < 16 && !show_bits(gb, bits_rem)))
>              break;
>  
> -        DECODE_CODEWORD(run, run_to_cb[FFMIN(run,  15)], LAST_SKIP_BITS);
> +        DECODE_CODEWORD(run, run_to_cb[FFMIN(run,  15)]);
>          pos += run + 1;
>          if (pos >= max_coeffs) {
>              av_log(avctx, AV_LOG_ERROR, "ac tex damaged %d, %d\n", pos, max_coeffs);
>              return AVERROR_INVALIDDATA;
>          }
>  
> -        DECODE_CODEWORD(level, lev_to_cb[FFMIN(level, 9)], SKIP_BITS);
> +        DECODE_CODEWORD(level, lev_to_cb[FFMIN(level, 9)]);
>          level += 1;
>  
>          i = pos >> log2_block_count;
>  
> -        sign = SHOW_SBITS(re, gb, 1);
> -        SKIP_BITS(re, gb, 1);
> +        sign = -get_bits1(gb);
>          out[((pos & block_mask) << 6) + ctx->scan[i]] = ((level ^ sign) - sign);
>      }
>  
> -    CLOSE_READER(re, gb);
>      return 0;
>  }
>  

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".