[FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Yan Wang
> Sent: Monday, July 8, 2019 15:54
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx without destroy va_context
> 
> 
> On 7/8/2019 2:45 PM, Yan Wang wrote:
> >
> > On 7/7/2019 9:49 PM, Fu, Linjie wrote:
> >>> -----Original Message-----
> >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf
> >>> Of Mark Thompson
> >>> Sent: Sunday, July 7, 2019 19:51
> >>> To: ffmpeg-devel at ffmpeg.org
> >>> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate
> >>> hw_frames_ctx without destroy va_context
> >>>
> >>> On 07/07/2019 17:38, Linjie Fu wrote:
> >>>> VP9 allows resolution changes per frame. Currently in VAAPI,
> >>>> resolution
> >>>> changes leads to va context destroy and reinit.
> >>> Which is correct - it needs to remake the context because the old
> >>> one is for
> >>> the wrong resolution.
> >> It seems that we don't need to remake context, remaking the surface
> >> is enough
> >> for resolution changing.
> >> Currently in libva, surface is able to be recreated separately
> >> without remaking context.
> >> And this way is used in libyami to cope with resolution changing cases.
> >>
> >>>> This will cause
> >>>> reference frame surface lost and produce garbage.
> >>> This isn't right - the reference frame surfaces aren't lost. See
> >>> VP9Context.s.refs[], which holds references to the frames (including
> >>> their
> >>> hardware frame contexts) until the stream indicates that they can be
> >>> discarded by overwriting their reference frame slots.
> >> I'm not quite sure about this, at least the result shows they are not
> >> used correctly
> >> in current way.
> >> Will look deeper into it.
> >
> > In vaapi_vp9_start_frame() of libavcodec/vaapi_vp9.c, it only passes
> > VASurfaceID into pic_param.reference_frames[i].
> >
> > But when destroy va_context, the surface/render target based on this
> > VASurfaceID has been destroyed.
> 
> Update: the surface isn't destroyed when destroy va_context. But every
> va_context maintains one independent map table: m_ddiDecodeCtx->RTtbl.
> 
> So the new context cannot find this surface in its map table.
> 
> My previous suggested solution should be available still.
>

Yes, tracing the code and could find that:

1.  surface is not destroyed until the decode finishes and calls avcodec_close;
2. refs[i] is passed to driver, however in DdiMediaBase::GetRenderTargetID, driver failed to
find the expected surface in the re-created m_ddiDecodeCtx->RTtbl, and returns invalid.
		if(rtTbl->pRT[i] == surface) {
			return i;
		}
 		return DDI_CODEC_INVALID_FRAME_INDEX;

One possible way is to register the refs[] surfaces in the new created context ->RTtbl and make it
findable.

Thanks for reviews and all comments.
linjie