[FFmpeg-devel] [PATCH 5/7] vp9: allocate 'b', 'block/uvblock' and 'eob/uveob' dynamically.

Ronald S. Bultje rsbultje at gmail.com
Tue Nov 26 13:19:54 CET 2013


Hi,


On Tue, Nov 26, 2013 at 4:41 AM, Clément Bœsch <u at pkh.me> wrote:

> On Mon, Nov 25, 2013 at 09:44:48PM -0500, Ronald S. Bultje wrote:
> > ---
> >  libavcodec/vp9.c | 40 ++++++++++++++++++++++++++++------------
> >  1 file changed, 28 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > index df9b197..6a99aeb 100644
> > --- a/libavcodec/vp9.c
> > +++ b/libavcodec/vp9.c
> > @@ -98,7 +98,7 @@ typedef struct VP9Context {
> >      VP56RangeCoder c;
> >      VP56RangeCoder *c_b;
> >      unsigned c_b_size;
> > -    VP9Block b;
> > +    VP9Block *b_base, *b;
> >      int row, row7, col, col7;
> >      uint8_t *dst[3];
> >      ptrdiff_t y_stride, uv_stride;
> > @@ -228,10 +228,8 @@ typedef struct VP9Context {
> >      DECLARE_ALIGNED(32, uint8_t, edge_emu_buffer)[71*80];
> >
> >      // block reconstruction intermediates
> > -    DECLARE_ALIGNED(32, int16_t, block)[4096];
> > -    DECLARE_ALIGNED(32, int16_t, uvblock)[2][1024];
> > -    uint8_t eob[256];
> > -    uint8_t uveob[2][64];
> > +    int16_t *block_base, *block, *uvblock_base[2], *uvblock[2];
> > +    uint8_t *eob_base, *uveob_base[2], *eob, *uveob[2];
> >      VP56mv min_mv, max_mv;
> >      DECLARE_ALIGNED(32, uint8_t, tmp_y)[64*64];
> >      DECLARE_ALIGNED(32, uint8_t, tmp_uv)[2][32*32];
> > @@ -330,6 +328,15 @@ static int update_size(AVCodecContext *ctx, int w,
> int h)
> >      assign(s->above_mv_ctx,        VP56mv(*)[2],          16);
> >  #undef assign
> >
> > +    av_free(s->b_base);
> > +    av_free(s->block_base);
> > +        s->b_base = av_malloc(sizeof(VP9Block));
> > +        s->block_base = av_mallocz((64 * 64 + 128) * 3);
>
> indent, unchecked alloc


Indent is intentional, later patches use this.

Will fix the unchecked alloc.

> +        s->uvblock_base[0] = s->block_base + 64 * 64;
> > +        s->uvblock_base[1] = s->uvblock_base[0] + 32 * 32;
> > +        s->eob_base = (uint8_t *) (s->uvblock_base[1] + 32 * 32);
> > +        s->uveob_base[0] = s->eob_base + 256;
> > +        s->uveob_base[1] = s->uveob_base[0] + 64;
> >      return 0;
> >  }
> >
> > @@ -908,7 +915,7 @@ static void find_ref_mvs(VP9Context *s,
> >          [BS_4x4]   = {{  0, -1 }, { -1,  0 }, { -1, -1 }, {  0, -2 },
> >                        { -2,  0 }, { -1, -2 }, { -2, -1 }, { -2, -2 }},
> >      };
> > -    VP9Block *const b = &s->b;
> > +    VP9Block *b = s->b;
> >      int row = s->row, col = s->col, row7 = s->row7;
> >      const int8_t (*p)[2] = mv_ref_blk_off[b->bs];
> >  #define INVALID_MV 0x80008000U
> > @@ -1121,7 +1128,7 @@ static av_always_inline int
> read_mv_component(VP9Context *s, int idx, int hp)
> >  static void fill_mv(VP9Context *s,
> >                      VP56mv *mv, int mode, int sb)
> >  {
> > -    VP9Block *const b = &s->b;
> > +    VP9Block *b = s->b;
> >
>
> I'm not sure why you need to remove the constness of the block here, I
> don't see any access change to it in the diff. Can you elaborate?
>

type *const name != const type *name - it's not the data that is const, but
the pointer. This matters if we're accessing a block that is embedded in a
struct that we're reading the variable from (i.e. type *const name =
&a->b;), it's a pretty typical code construct (although really a compiler
should know this by itself). Since it's now no longer embedded in the
struct (var = &a->b; -> var = a->b;), the const is now unnecessary.

Ronald


More information about the ffmpeg-devel mailing list