diff mbox

[FFmpeg-devel,2/2] lavc/vp9: support hardware decode with resolution changing on inter frame

Message ID 1577436513-25309-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu Dec. 27, 2019, 8:48 a.m. UTC
VP9 decoder should be able to handle resolution changing on inter
frame without re-initialization. For hardware decoder, re-allocate hardware
frame surface.

Fix #8068 for VA-API.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
Request for comments.
This works for VA-API, however for dxva2 it didn't cover all cases.

Another idea is to register a call-back function in AVHWAccel (such as
ff_vp9_dxva2_hwaccel) to handle surface re-allocation according to
the hardware decoder.

 libavcodec/vp9.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Hendrik Leppkes Dec. 27, 2019, 10:22 a.m. UTC | #1
On Fri, Dec 27, 2019 at 9:59 AM Linjie Fu <linjie.fu@intel.com> wrote:
>
> VP9 decoder should be able to handle resolution changing on inter
> frame without re-initialization. For hardware decoder, re-allocate hardware
> frame surface.
>
> Fix #8068 for VA-API.
>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> Request for comments.
> This works for VA-API, however for dxva2 it didn't cover all cases.
>
> Another idea is to register a call-back function in AVHWAccel (such as
> ff_vp9_dxva2_hwaccel) to handle surface re-allocation according to
> the hardware decoder.
>
>  libavcodec/vp9.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 0fd15ef..a7b4c6a 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -34,6 +34,7 @@
>  #include "vp9dec.h"
>  #include "libavutil/avassert.h"
>  #include "libavutil/pixdesc.h"
> +#include "decode.h"
>
>  #define VP9_SYNCCODE 0x498342
>
> @@ -220,11 +221,20 @@ static int update_size(AVCodecContext *avctx, int w, int h)
>          *fmtp++ = s->pix_fmt;
>          *fmtp = AV_PIX_FMT_NONE;
>
> -        ret = ff_thread_get_format(avctx, pix_fmts);
> -        if (ret < 0)
> -            return ret;
> +        if (avctx->internal->hwaccel_priv_data && s->pix_fmt == s->gf_fmt && (s->w != w || s->h != h)) {
> +            const AVHWDeviceContext *device_ctx =
> +                (AVHWDeviceContext*)avctx->hw_device_ctx->data;
> +            ret = ff_decode_get_hw_frames_ctx(avctx, device_ctx->type);
> +            if (ret < 0)
> +                return ret;
> +        } else {
> +            ret = ff_thread_get_format(avctx, pix_fmts);
> +            if (ret < 0)
> +                return ret;
> +
> +            avctx->pix_fmt = ret;
> +        }
>
> -        avctx->pix_fmt = ret;
>          s->gf_fmt  = s->pix_fmt;
>          s->w = w;
>          s->h = h;

hwaccels are not guaranteed to have a hw_frames ctx, and avcodec is
not guaranteed to manage the surfaces it decodes to at all (they could
be shared with eg. a renderer and allocated externally), as such a
decoder really can't reliably re-allocate this itself - thats what the
get_format callback is for, which informs the user that new dimensions
are required.

- Hendrik
Linjie Fu Dec. 29, 2019, 3:21 a.m. UTC | #2
> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Hendrik Leppkes

> Sent: Friday, December 27, 2019 18:22

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vp9: support hardware

> decode with resolution changing on inter frame

> 

> On Fri, Dec 27, 2019 at 9:59 AM Linjie Fu <linjie.fu@intel.com> wrote:

> >

> > VP9 decoder should be able to handle resolution changing on inter

> > frame without re-initialization. For hardware decoder, re-allocate hardware

> > frame surface.

> >

> > Fix #8068 for VA-API.

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> > Request for comments.

> > This works for VA-API, however for dxva2 it didn't cover all cases.

> >

> > Another idea is to register a call-back function in AVHWAccel (such as

> > ff_vp9_dxva2_hwaccel) to handle surface re-allocation according to

> > the hardware decoder.

> >

> >  libavcodec/vp9.c | 18 ++++++++++++++----

> >  1 file changed, 14 insertions(+), 4 deletions(-)

> >

> > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c

> > index 0fd15ef..a7b4c6a 100644

> > --- a/libavcodec/vp9.c

> > +++ b/libavcodec/vp9.c

> > @@ -34,6 +34,7 @@

> >  #include "vp9dec.h"

> >  #include "libavutil/avassert.h"

> >  #include "libavutil/pixdesc.h"

> > +#include "decode.h"

> >

> >  #define VP9_SYNCCODE 0x498342

> >

> > @@ -220,11 +221,20 @@ static int update_size(AVCodecContext *avctx,

> int w, int h)

> >          *fmtp++ = s->pix_fmt;

> >          *fmtp = AV_PIX_FMT_NONE;

> >

> > -        ret = ff_thread_get_format(avctx, pix_fmts);

> > -        if (ret < 0)

> > -            return ret;

> > +        if (avctx->internal->hwaccel_priv_data && s->pix_fmt == s->gf_fmt

> && (s->w != w || s->h != h)) {

> > +            const AVHWDeviceContext *device_ctx =

> > +                (AVHWDeviceContext*)avctx->hw_device_ctx->data;

> > +            ret = ff_decode_get_hw_frames_ctx(avctx, device_ctx->type);

> > +            if (ret < 0)

> > +                return ret;

> > +        } else {

> > +            ret = ff_thread_get_format(avctx, pix_fmts);

> > +            if (ret < 0)

> > +                return ret;

> > +

> > +            avctx->pix_fmt = ret;

> > +        }

> >

> > -        avctx->pix_fmt = ret;

> >          s->gf_fmt  = s->pix_fmt;

> >          s->w = w;

> >          s->h = h;

> 

> hwaccels are not guaranteed to have a hw_frames ctx, and avcodec is

> not guaranteed to manage the surfaces it decodes to at all (they could

> be shared with eg. a renderer and allocated externally), as such a


Thanks, and would you please help to share more information about how to reproduce?
1. no hw_frame_ctx, only hw_device_ctx
2. surfaces shared with a rendered and allocated externally
(externally provided hw_frames_ctx)

I'd like to step into these and try to find a proper/general solution.

> decoder really can't reliably re-allocate this itself - thats what the

> get_format callback is for, which informs the user that new dimensions

> are required.


It's vp9 (and AV1) specific, so I didn't intend to modify in the general path(ff_get_format) firstly.

Instead of fixing in codec(update_size() in vp9.c),  if we are going to handle this in user
(ff_get_format() in decode.c) to manage the surface, IMHO a modification in API may be needed
to register a reinit function in AVHWAccel.
See:
https://github.com/intel-media-ci/ffmpeg/pull/153

thx

- linjie
Hendrik Leppkes Dec. 29, 2019, 8:58 a.m. UTC | #3
On Sun, Dec 29, 2019 at 4:21 AM Fu, Linjie <linjie.fu@intel.com> wrote:
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Hendrik Leppkes
> > Sent: Friday, December 27, 2019 18:22
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vp9: support hardware
> > decode with resolution changing on inter frame
> >
> > On Fri, Dec 27, 2019 at 9:59 AM Linjie Fu <linjie.fu@intel.com> wrote:
> > >
> > > VP9 decoder should be able to handle resolution changing on inter
> > > frame without re-initialization. For hardware decoder, re-allocate hardware
> > > frame surface.
> > >
> > > Fix #8068 for VA-API.
> > >
> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > > ---
> > > Request for comments.
> > > This works for VA-API, however for dxva2 it didn't cover all cases.
> > >
> > > Another idea is to register a call-back function in AVHWAccel (such as
> > > ff_vp9_dxva2_hwaccel) to handle surface re-allocation according to
> > > the hardware decoder.
> > >
> > >  libavcodec/vp9.c | 18 ++++++++++++++----
> > >  1 file changed, 14 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > > index 0fd15ef..a7b4c6a 100644
> > > --- a/libavcodec/vp9.c
> > > +++ b/libavcodec/vp9.c
> > > @@ -34,6 +34,7 @@
> > >  #include "vp9dec.h"
> > >  #include "libavutil/avassert.h"
> > >  #include "libavutil/pixdesc.h"
> > > +#include "decode.h"
> > >
> > >  #define VP9_SYNCCODE 0x498342
> > >
> > > @@ -220,11 +221,20 @@ static int update_size(AVCodecContext *avctx,
> > int w, int h)
> > >          *fmtp++ = s->pix_fmt;
> > >          *fmtp = AV_PIX_FMT_NONE;
> > >
> > > -        ret = ff_thread_get_format(avctx, pix_fmts);
> > > -        if (ret < 0)
> > > -            return ret;
> > > +        if (avctx->internal->hwaccel_priv_data && s->pix_fmt == s->gf_fmt
> > && (s->w != w || s->h != h)) {
> > > +            const AVHWDeviceContext *device_ctx =
> > > +                (AVHWDeviceContext*)avctx->hw_device_ctx->data;
> > > +            ret = ff_decode_get_hw_frames_ctx(avctx, device_ctx->type);
> > > +            if (ret < 0)
> > > +                return ret;
> > > +        } else {
> > > +            ret = ff_thread_get_format(avctx, pix_fmts);
> > > +            if (ret < 0)
> > > +                return ret;
> > > +
> > > +            avctx->pix_fmt = ret;
> > > +        }
> > >
> > > -        avctx->pix_fmt = ret;
> > >          s->gf_fmt  = s->pix_fmt;
> > >          s->w = w;
> > >          s->h = h;
> >
> > hwaccels are not guaranteed to have a hw_frames ctx, and avcodec is
> > not guaranteed to manage the surfaces it decodes to at all (they could
> > be shared with eg. a renderer and allocated externally), as such a
>
> Thanks, and would you please help to share more information about how to reproduce?
> 1. no hw_frame_ctx, only hw_device_ctx
> 2. surfaces shared with a rendered and allocated externally
> (externally provided hw_frames_ctx)
>
> I'd like to step into these and try to find a proper/general solution.
>
> > decoder really can't reliably re-allocate this itself - thats what the
> > get_format callback is for, which informs the user that new dimensions
> > are required.
>
> It's vp9 (and AV1) specific, so I didn't intend to modify in the general path(ff_get_format) firstly.
>
> Instead of fixing in codec(update_size() in vp9.c),  if we are going to handle this in user
> (ff_get_format() in decode.c) to manage the surface, IMHO a modification in API may be needed
> to register a reinit function in AVHWAccel.
> See:
> https://github.com/intel-media-ci/ffmpeg/pull/153
>

If a new callback is needed, then it would need to be a public
user-facing callback, not in AVHWAccel (or maybe in addition to),
otherwise you don't really handle the problem i'm talking about -
namely that avcodec has no control over the hardware frames at all in
some circumstances.

But why can't get_format be used for this? Its already in use for
re-sizing buffers today.

- Hendrik
diff mbox

Patch

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 0fd15ef..a7b4c6a 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -34,6 +34,7 @@ 
 #include "vp9dec.h"
 #include "libavutil/avassert.h"
 #include "libavutil/pixdesc.h"
+#include "decode.h"
 
 #define VP9_SYNCCODE 0x498342
 
@@ -220,11 +221,20 @@  static int update_size(AVCodecContext *avctx, int w, int h)
         *fmtp++ = s->pix_fmt;
         *fmtp = AV_PIX_FMT_NONE;
 
-        ret = ff_thread_get_format(avctx, pix_fmts);
-        if (ret < 0)
-            return ret;
+        if (avctx->internal->hwaccel_priv_data && s->pix_fmt == s->gf_fmt && (s->w != w || s->h != h)) {
+            const AVHWDeviceContext *device_ctx =
+                (AVHWDeviceContext*)avctx->hw_device_ctx->data;
+            ret = ff_decode_get_hw_frames_ctx(avctx, device_ctx->type);
+            if (ret < 0)
+                return ret;
+        } else {
+            ret = ff_thread_get_format(avctx, pix_fmts);
+            if (ret < 0)
+                return ret;
+
+            avctx->pix_fmt = ret;
+        }
 
-        avctx->pix_fmt = ret;
         s->gf_fmt  = s->pix_fmt;
         s->w = w;
         s->h = h;