[FFmpeg-devel] [PATCH v3 2/3] lavc/decode: Add internal surface re-allocate method for hwaccel

[Wang, Fei W]

On Mon, 2022-09-19 at 14:08 +0800, Fei Wang wrote:
> On Wed, 2022-09-07 at 22:56 +0100, Mark Thompson wrote:
> > On 23/08/2022 09:19, Fei Wang wrote:
> > > From: Linjie Fu <linjie.fu at intel.com>
> > > 
> > > Add HWACCEL_CAP_INTERNAL_ALLOC flag to indicate hwaccels are able
> > > to
> > > re-allocate surface internally through
> > > ff_decode_get_hw_frames_ctx.
> > > So that hwaccels don't need to reinitialize all hw related
> > > configs
> > > when decode resolution change, just need to re-allocate new
> > > surface
> > > by using new resolution.
> > > 
> > > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > > Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> > > ---
> > >   libavcodec/decode.c   | 36 ++++++++++++++++++++++++++++++++++++
> > >   libavcodec/hwconfig.h |  1 +
> > >   2 files changed, 37 insertions(+)
> > 
> > You can't just not call the user get_format callback and allocate
> > your own surfaces - this breaks direct rendering and other cases
> > where the user wanted to manage the surfaces.
> > 
> > This is also missing any check that the hardware decoder supports
> > the
> > stream post-transition - if the decoder does not support the new
> > size
> > (or any other property of the new stream) then this will try to
> > blindly decode it anyway and fail, where previously it would have
> > correctly fallen back to software decoding.
> > 
> > 
> > None of these patches say what the aim is, but from reading them
> > and
> > seeing that VP9 is the intended target then I am guessing that this
> > is intended to support the case where the stream resizes while
> > still
> > using previous reference frames - is that right?
> 
> Yes, this fixed some vp9 resize streams which reference frames has
> different resolution.
> 
> > If my guess is correct, I think you should (a) mention that fact in
> > the patches, and (b) target the support at specifically that case,
> > and not try to mess with any other reinit cases.
> > 
> > Something like: if you know you are in that case (the decoder
> > itself
> > has this information and could pass it to ff_get_format somehow)
> > and
> > the context supports it (I am still unclear how this support can be
> > determined - the libva documentation is very clear that a context
> > is
> > tied to a particular height/width), then remember the context
> > across
> > the user get_format call and if things match up then re-use it.
> 
> Thanks, the logic looks good. I will check it later to see if any
> blocks on the detail implementation.

Current decode logis is hwaccel->uninit, get_format, hwaccel->init.
While the avctx->internal->hwaccel_priv_data is freed in uninit and
re-alloc in init, so it can't store and re-use vaContext in get_format.

I have modified the other version of V4, which can keep current decode
logic as much as possible and still put alloc surfaces in
hwaccel.init() after call back get_foramt.

Thanks
Fei
> 
> Thanks
> Fei
> > If for some reason you are in that case but it can't work (e.g.
> > because the new size isn't supported by the hardware), then you
> > need
> > a better error message - the stream is actually broken because most
> > frames are not decodable until you reach another recovery point
> > (since the reference frames are in hardware surfaces so the
> > software
> > decoder can't use them).
> > 
> > - Mark
> > _______________________________________________
> > 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".