Message ID | 1577436513-25309-1-git-send-email-linjie.fu@intel.com |
---|---|
State | New |
Headers | show |
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
> -----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
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 --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;
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(-)