[FFmpeg-devel] [PATCH 1/2] vp9: add hwaccel hooks

[Ronald S. Bultje]

Hi,

On Thu, Dec 3, 2015 at 6:13 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Thu, Dec 3, 2015 at 11:13 AM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> > ---
> >  libavcodec/vp9.c | 128
> +++++++++++++++++++++++++++++++++++++++----------------
> >  libavcodec/vp9.h |   3 ++
> >  2 files changed, 94 insertions(+), 37 deletions(-)
> >
> > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > index d8888c0..87e80d8 100644
> > --- a/libavcodec/vp9.c
> > +++ b/libavcodec/vp9.c
> > @@ -168,6 +168,15 @@ static const uint8_t bwh_tab[2][N_BS_SIZES][2] = {
> >      }
> >  };
> >
> > +static void vp9_unref_frame(AVCodecContext *ctx, VP9Frame *f)
> > +{
> > +    ff_thread_release_buffer(ctx, &f->tf);
> > +    av_buffer_unref(&f->extradata);
> > +    av_buffer_unref(&f->hwaccel_priv_buf);
> > +    f->segmentation_map = NULL;
> > +    f->hwaccel_picture_private = NULL;
> > +}
> > +
> >  static int vp9_alloc_frame(AVCodecContext *ctx, VP9Frame *f)
> >  {
> >      VP9Context *s = ctx->priv_data;
> > @@ -177,21 +186,28 @@ static int vp9_alloc_frame(AVCodecContext *ctx,
> VP9Frame *f)
> >          return ret;
> >      sz = 64 * s->sb_cols * s->sb_rows;
> >      if (!(f->extradata = av_buffer_allocz(sz * (1 + sizeof(struct
> VP9mvrefPair))))) {
> > -        ff_thread_release_buffer(ctx, &f->tf);
> > -        return AVERROR(ENOMEM);
> > +        goto fail;
> >      }
> >
> >      f->segmentation_map = f->extradata->data;
> >      f->mv = (struct VP9mvrefPair *) (f->extradata->data + sz);
> >
> > +    if (ctx->hwaccel) {
> > +        const AVHWAccel *hwaccel = ctx->hwaccel;
> > +        av_assert0(!f->hwaccel_picture_private);
> > +        if (hwaccel->frame_priv_data_size) {
> > +            f->hwaccel_priv_buf =
> av_buffer_allocz(hwaccel->frame_priv_data_size);
> > +            if (!f->hwaccel_priv_buf)
> > +                goto fail;
> > +            f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
> > +        }
> > +    }
> > +
> >      return 0;
> > -}
> >
> > -static void vp9_unref_frame(AVCodecContext *ctx, VP9Frame *f)
> > -{
> > -    ff_thread_release_buffer(ctx, &f->tf);
> > -    av_buffer_unref(&f->extradata);
> > -    f->segmentation_map = NULL;
> > +fail:
> > +    vp9_unref_frame(ctx, f);
> > +    return AVERROR(ENOMEM);
> >  }
> >
> >  static int vp9_ref_frame(AVCodecContext *ctx, VP9Frame *dst, VP9Frame
> *src)
> > @@ -201,19 +217,31 @@ static int vp9_ref_frame(AVCodecContext *ctx,
> VP9Frame *dst, VP9Frame *src)
> >      if ((res = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0) {
> >          return res;
> >      } else if (!(dst->extradata = av_buffer_ref(src->extradata))) {
> > -        vp9_unref_frame(ctx, dst);
> > -        return AVERROR(ENOMEM);
> > +        goto fail;
> >      }
> >
> >      dst->segmentation_map = src->segmentation_map;
> >      dst->mv = src->mv;
> >      dst->uses_2pass = src->uses_2pass;
> >
> > +    if (src->hwaccel_picture_private) {
> > +        dst->hwaccel_priv_buf = av_buffer_ref(src->hwaccel_priv_buf);
> > +        if (!dst->hwaccel_priv_buf)
> > +            goto fail;
> > +        dst->hwaccel_picture_private = dst->hwaccel_priv_buf->data;
> > +    }
> > +
> >      return 0;
> > +
> > +fail:
> > +    vp9_unref_frame(ctx, dst);
> > +    return AVERROR(ENOMEM);
> >  }
> >
> >  static int update_size(AVCodecContext *ctx, int w, int h)
> >  {
> > +#define HWACCEL_MAX (0)
> > +    enum AVPixelFormat pix_fmts[HWACCEL_MAX + 2], *fmtp = pix_fmts;
> >      VP9Context *s = ctx->priv_data;
> >      uint8_t *p;
> >      int bytesperpixel = s->bytesperpixel, res;
> > @@ -225,7 +253,16 @@ static int update_size(AVCodecContext *ctx, int w,
> int h)
> >
> >      if ((res = ff_set_dimensions(ctx, w, h)) < 0)
> >          return res;
> > -    s->last_fmt  = ctx->pix_fmt = s->pix_fmt;
> > +    s->last_fmt  = s->pix_fmt;
> > +
> > +    *fmtp++ = s->pix_fmt;
> > +    *fmtp = AV_PIX_FMT_NONE;
> > +
> > +    res = ff_thread_get_format(ctx, pix_fmts);
> > +    if (res < 0)
> > +        return res;
> > +
> > +    ctx->pix_fmt = res;
>
> I should probably move setting s->last_fmt below the get_format call
> to avoid re-introducing the bug of state de-syncing when some other
> error occured.


The state is in s->pix_fmt, not s->last_fmt. s->last_fmt is just "the
previous state" to compare the current state to. If your point is that the
state may be "nothing" on failure, then maybe we should set s->last_fmt to
AV_PIX_FMT_NONE on error (and on init) to make sure it is reinit'ed again...

Ronald