[FFmpeg-devel] [PATCH 19/21] avcodec/smacker: Avoid allocations for decoding Smacker

Paul B Mahol onemda at gmail.com
Sat Aug 1 17:00:24 EEST 2020


On 8/1/20, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
> by using buffers on the stack instead. The fact that the effective
> lifetime of most of the allocated buffers doesn't overlap enables one to
> limit the stack space used to a fairly modest size (about 1.5 KiB).
>
> That all the buffers used in HuffContexts have always the same number of
> elements (namely 256) makes it possible to include the buffers directly
> in the HuffContext. Doing so also makes the length field redundant; it has
> therefore been removed.
>
> This is beneficial for performance: For GCC 9 the time for one call to
> smka_decode_frame() for the sample in ticket #2425 went down from
> 1794494 to 1709043 decicyles; for Clang 9 it decreased from 1449420 to
> 1355273 decicycles.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavcodec/smacker.c | 69 +++++++++++++++-----------------------------
>  1 file changed, 23 insertions(+), 46 deletions(-)
>
> diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
> index 15c8856e69..e588b03820 100644
> --- a/libavcodec/smacker.c
> +++ b/libavcodec/smacker.c
> @@ -66,11 +66,10 @@ typedef struct SmackVContext {
>   * Context used for code reconstructing
>   */
>  typedef struct HuffContext {
> -    int length;
>      int current;
> -    uint32_t *bits;
> -    uint8_t *lengths;
> -    uint8_t *values;
> +    uint32_t bits[256];
> +    uint8_t lengths[256];
> +    uint8_t values[256];
>  } HuffContext;
>
>  /* common parameters used for decode_bigtree */
> @@ -114,7 +113,7 @@ static int smacker_decode_tree(GetBitContext *gb,
> HuffContext *hc, uint32_t pref
>      }
>
>      if(!get_bits1(gb)){ //Leaf
> -        if(hc->current >= hc->length){
> +        if (hc->current >= 256) {
>              av_log(NULL, AV_LOG_ERROR, "Tree size exceeded!\n");
>              return AVERROR_INVALIDDATA;
>          }
> @@ -198,7 +197,6 @@ static int smacker_decode_bigtree(GetBitContext *gb,
> DBCtx *ctx, int length)
>   */
>  static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext
> *gb, int **recodes, int *last, int size)
>  {
> -    HuffContext h[2] = { 0 };
>      VLC vlc[2] = { { 0 } };
>      int escapes[3];
>      DBCtx ctx;
> @@ -210,37 +208,30 @@ static int smacker_decode_header_tree(SmackVContext
> *smk, GetBitContext *gb, int
>      }
>
>      for (int i = 0; i < 2; i++) {
> -        h[i].length  = 256;
> -        h[i].current = 0;
> -        h[i].bits    = av_malloc(256 * sizeof(h[i].bits[0]));
> -        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths[0]));
> -        h[i].values  = av_malloc(256 * sizeof(h[i].values[0]));
> -        if (!h[i].bits || !h[i].lengths || !h[i].values) {
> -            err = AVERROR(ENOMEM);
> -            goto error;
> -        }
> +        HuffContext h;
> +        h.current = 0;
>          if (!get_bits1(gb)) {
>              ctx.vals[i] = 0;
>              av_log(smk->avctx, AV_LOG_ERROR, "Skipping %s bytes tree\n",
>                     i ? "high" : "low");
>              continue;
>          }
> -        err = smacker_decode_tree(gb, &h[i], 0, 0);
> +        err = smacker_decode_tree(gb, &h, 0, 0);
>          if (err < 0)
>              goto error;
>          skip_bits1(gb);
> -        if (h[i].current > 1) {
> -            err = ff_init_vlc_sparse(&vlc[i], SMKTREE_BITS, h[i].current,
> -                           INIT_VLC_DEFAULT_SIZES(h[i].lengths),
> -                           INIT_VLC_DEFAULT_SIZES(h[i].bits),
> -                                     INIT_VLC_DEFAULT_SIZES(h[i].values),
> -                           INIT_VLC_LE);
> +        if (h.current > 1) {
> +            err = ff_init_vlc_sparse(&vlc[i], SMKTREE_BITS, h.current,
> +                                     INIT_VLC_DEFAULT_SIZES(h.lengths),
> +                                     INIT_VLC_DEFAULT_SIZES(h.bits),
> +                                     INIT_VLC_DEFAULT_SIZES(h.values),
> +                                     INIT_VLC_LE);
>              if (err < 0) {
>                  av_log(smk->avctx, AV_LOG_ERROR, "Cannot build VLC
> table\n");
>                  goto error;
>              }
>          } else
> -            ctx.vals[i] = h[i].values[0];
> +            ctx.vals[i] = h.values[0];
>      }
>
>      escapes[0]  = get_bits(gb, 16);
> @@ -276,9 +267,6 @@ static int smacker_decode_header_tree(SmackVContext
> *smk, GetBitContext *gb, int
>  error:
>      for (int i = 0; i < 2; i++) {
>          ff_free_vlc(&vlc[i]);
> -        av_free(h[i].bits);
> -        av_free(h[i].lengths);
> -        av_free(h[i].values);
>      }
>
>      return err;
> @@ -603,7 +591,6 @@ static int smka_decode_frame(AVCodecContext *avctx, void
> *data,
>      const uint8_t *buf = avpkt->data;
>      int buf_size = avpkt->size;
>      GetBitContext gb;
> -    HuffContext h[4] = { { 0 } };
>      VLC vlc[4]       = { { 0 } };
>      int16_t *samples;
>      uint8_t *samples8;
> @@ -659,31 +646,24 @@ static int smka_decode_frame(AVCodecContext *avctx,
> void *data,
>
>      // Initialize
>      for(i = 0; i < (1 << (bits + stereo)); i++) {
> -        h[i].length = 256;
> -        h[i].current = 0;
> -        h[i].bits    = av_malloc(256 * sizeof(h[i].bits));
> -        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths));
> -        h[i].values  = av_malloc(256 * sizeof(h[i].values));
> -        if (!h[i].bits || !h[i].lengths || !h[i].values) {
> -            ret = AVERROR(ENOMEM);
> -            goto error;
> -        }
> +        HuffContext h;
> +        h.current = 0;
>          skip_bits1(&gb);
> -        if ((ret = smacker_decode_tree(&gb, &h[i], 0, 0)) < 0)
> +        if ((ret = smacker_decode_tree(&gb, &h, 0, 0)) < 0)
>              goto error;
>          skip_bits1(&gb);
> -        if(h[i].current > 1) {
> -            ret = ff_init_vlc_sparse(&vlc[i], SMKTREE_BITS, h[i].current,
> -                           INIT_VLC_DEFAULT_SIZES(h[i].lengths),
> -                                     INIT_VLC_DEFAULT_SIZES(h[i].bits),
> -                                     INIT_VLC_DEFAULT_SIZES(h[i].values),
> +        if (h.current > 1) {
> +            ret = ff_init_vlc_sparse(&vlc[i], SMKTREE_BITS, h.current,
> +                                     INIT_VLC_DEFAULT_SIZES(h.lengths),
> +                                     INIT_VLC_DEFAULT_SIZES(h.bits),
> +                                     INIT_VLC_DEFAULT_SIZES(h.values),
>                                       INIT_VLC_LE);
>              if (ret < 0) {
>                  av_log(avctx, AV_LOG_ERROR, "Cannot build VLC table\n");
>                  goto error;
>              }
>          } else
> -            values[i] = h[i].values[0];
> +            values[i] = h.values[0];
>      }
>      /* this codec relies on wraparound instead of clipping audio */
>      if(bits) { //decode 16-bit data
> @@ -758,9 +738,6 @@ static int smka_decode_frame(AVCodecContext *avctx, void
> *data,
>  error:
>      for(i = 0; i < 4; i++) {
>          ff_free_vlc(&vlc[i]);
> -        av_free(h[i].bits);
> -        av_free(h[i].lengths);
> -        av_free(h[i].values);
>      }
>
>      return ret;
> --
> 2.20.1
>


LGTM

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list