Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-11-19 Thread Max Dmitrichenko
On Fri, Sep 20, 2019 at 5:12 AM Fu, Linjie  wrote:

> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Fu, Linjie
> > Sent: Wednesday, September 11, 2019 00:02
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > hw_frames_ctx for vp9 without destroy va_context
> >
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> > > Of Mark Thompson
> > > Sent: Tuesday, September 10, 2019 08:02
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > > hw_frames_ctx for vp9 without destroy va_context
> > >
> > > On 09/09/2019 16:40, Fu, Linjie wrote:
> > > >> -Original Message-
> > > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > Behalf
> > > >> Of Fu, Linjie
> > > >> Sent: Friday, August 30, 2019 16:05
> > > >> To: FFmpeg development discussions and patches  > > >> de...@ffmpeg.org>
> > > >> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > recreate
> > > >> hw_frames_ctx for vp9 without destroy va_context
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > >> Behalf
> > > >>> Of Fu, Linjie
> > > >>> Sent: Friday, August 9, 2019 19:47
> > > >>> To: FFmpeg development discussions and patches  > > >>> de...@ffmpeg.org>
> > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > > recreate
> > > >>> hw_frames_ctx for vp9 without destroy va_context
> > > >>>
> > > >>>> -Original Message-
> > > >>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > >>> Behalf
> > > >>>> Of Hendrik Leppkes
> > > >>>> Sent: Friday, August 9, 2019 17:40
> > > >>>> To: FFmpeg development discussions and patches  > > >>>> de...@ffmpeg.org>
> > > >>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > > >> recreate
> > > >>>> hw_frames_ctx for vp9 without destroy va_context
> > > >>>>
> > > >>>> On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie 
> > wrote:
> > > >>>>>
> > > >>>>>> -Original Message-
> > > >>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
> > > On
> > > >>>> Behalf
> > > >>>>>> Of Hendrik Leppkes
> > > >>>>>> Sent: Tuesday, August 6, 2019 16:27
> > > >>>>>> To: FFmpeg development discussions and patches  > > >>>>>> de...@ffmpeg.org>
> > > >>>>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > > >>>> recreate
> > > >>>>>> hw_frames_ctx for vp9 without destroy va_context
> > > >>>>>>
> > > >>>>>> On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  >
> > > wrote:
> > > >>>>>>>
> > > >>>>>>> VP9 allows resolution changes per frame. Currently in VAAPI,
> > > >>> resolution
> > > >>>>>>> changes leads to va context destroy and reinit. This will cause
> > > >>>>>>> reference frame surface lost and produce garbage.
> > > >>>>>>>
> > > >>>>>>> Though refs surface id could be passed to media driver and
> found
> > > in
> > > >>>>>>> RTtbl, vp9RefList[] in hal layer has already been destroyed.
> Thus
> > > the
> > > >>>>>>> new created VaContext could only got an empty RefList.
> > > >>>>>>>
> > > >>>>>>> As libva allows re-create surface separately without changing
> the
> > > >>>>>>> context, this issue could be handled by only recreating
> > > >>> hw_frames_ctx.
> > > >>>>>>>
> > > >>>>>&g

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-09-19 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Wednesday, September 11, 2019 00:02
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Mark Thompson
> > Sent: Tuesday, September 10, 2019 08:02
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > hw_frames_ctx for vp9 without destroy va_context
> >
> > On 09/09/2019 16:40, Fu, Linjie wrote:
> > >> -Original Message-
> > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> > >> Of Fu, Linjie
> > >> Sent: Friday, August 30, 2019 16:05
> > >> To: FFmpeg development discussions and patches  > >> de...@ffmpeg.org>
> > >> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> recreate
> > >> hw_frames_ctx for vp9 without destroy va_context
> > >>
> > >>> -Original Message-
> > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > >> Behalf
> > >>> Of Fu, Linjie
> > >>> Sent: Friday, August 9, 2019 19:47
> > >>> To: FFmpeg development discussions and patches  > >>> de...@ffmpeg.org>
> > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > recreate
> > >>> hw_frames_ctx for vp9 without destroy va_context
> > >>>
> > >>>> -Original Message-
> > >>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > >>> Behalf
> > >>>> Of Hendrik Leppkes
> > >>>> Sent: Friday, August 9, 2019 17:40
> > >>>> To: FFmpeg development discussions and patches  > >>>> de...@ffmpeg.org>
> > >>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > >> recreate
> > >>>> hw_frames_ctx for vp9 without destroy va_context
> > >>>>
> > >>>> On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie 
> wrote:
> > >>>>>
> > >>>>>> -Original Message-
> > >>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
> > On
> > >>>> Behalf
> > >>>>>> Of Hendrik Leppkes
> > >>>>>> Sent: Tuesday, August 6, 2019 16:27
> > >>>>>> To: FFmpeg development discussions and patches  > >>>>>> de...@ffmpeg.org>
> > >>>>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > >>>> recreate
> > >>>>>> hw_frames_ctx for vp9 without destroy va_context
> > >>>>>>
> > >>>>>> On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu 
> > wrote:
> > >>>>>>>
> > >>>>>>> VP9 allows resolution changes per frame. Currently in VAAPI,
> > >>> resolution
> > >>>>>>> changes leads to va context destroy and reinit. This will cause
> > >>>>>>> reference frame surface lost and produce garbage.
> > >>>>>>>
> > >>>>>>> Though refs surface id could be passed to media driver and found
> > in
> > >>>>>>> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus
> > the
> > >>>>>>> new created VaContext could only got an empty RefList.
> > >>>>>>>
> > >>>>>>> As libva allows re-create surface separately without changing the
> > >>>>>>> context, this issue could be handled by only recreating
> > >>> hw_frames_ctx.
> > >>>>>>>
> > >>>>>>> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > >>>>>>> hw_frame_ctx when dynamic resolution changing happens.
> > >>>>>>>
> > >>>>>>> Could be verified by:
> > >>>>>>> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> > >>>>>>>   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y
> > >> 

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-09-10 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Tuesday, September 10, 2019 08:02
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> On 09/09/2019 16:40, Fu, Linjie wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> >> Of Fu, Linjie
> >> Sent: Friday, August 30, 2019 16:05
> >> To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> >> hw_frames_ctx for vp9 without destroy va_context
> >>
> >>> -Original Message-
> >>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >> Behalf
> >>> Of Fu, Linjie
> >>> Sent: Friday, August 9, 2019 19:47
> >>> To: FFmpeg development discussions and patches  >>> de...@ffmpeg.org>
> >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> recreate
> >>> hw_frames_ctx for vp9 without destroy va_context
> >>>
> >>>> -Original Message-
> >>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> >>> Behalf
> >>>> Of Hendrik Leppkes
> >>>> Sent: Friday, August 9, 2019 17:40
> >>>> To: FFmpeg development discussions and patches  >>>> de...@ffmpeg.org>
> >>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> >> recreate
> >>>> hw_frames_ctx for vp9 without destroy va_context
> >>>>
> >>>> On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
> >>>>>
> >>>>>> -Original Message-
> >>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org]
> On
> >>>> Behalf
> >>>>>> Of Hendrik Leppkes
> >>>>>> Sent: Tuesday, August 6, 2019 16:27
> >>>>>> To: FFmpeg development discussions and patches  >>>>>> de...@ffmpeg.org>
> >>>>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> >>>> recreate
> >>>>>> hw_frames_ctx for vp9 without destroy va_context
> >>>>>>
> >>>>>> On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu 
> wrote:
> >>>>>>>
> >>>>>>> VP9 allows resolution changes per frame. Currently in VAAPI,
> >>> resolution
> >>>>>>> changes leads to va context destroy and reinit. This will cause
> >>>>>>> reference frame surface lost and produce garbage.
> >>>>>>>
> >>>>>>> Though refs surface id could be passed to media driver and found
> in
> >>>>>>> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus
> the
> >>>>>>> new created VaContext could only got an empty RefList.
> >>>>>>>
> >>>>>>> As libva allows re-create surface separately without changing the
> >>>>>>> context, this issue could be handled by only recreating
> >>> hw_frames_ctx.
> >>>>>>>
> >>>>>>> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> >>>>>>> hw_frame_ctx when dynamic resolution changing happens.
> >>>>>>>
> >>>>>>> Could be verified by:
> >>>>>>> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> >>>>>>>   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y
> >> vaapi.yuv
> >>>>>>>
> >>>>>>> Signed-off-by: Linjie Fu 
> >>>>>>> ---
> >>>>>>>  libavcodec/decode.c| 10 +-
> >>>>>>>  libavcodec/internal.h  |  1 +
> >>>>>>>  libavcodec/pthread_frame.c |  2 ++
> >>>>>>>  libavcodec/vaapi_decode.c  | 40 ++--
> -
> >> --
> >>> --
> >>>> -
> >>>>>> --
> >>>>>>>  libavcodec/vaapi_vp9.c |  4 
> >>>>>>>  5 files changed, 34 insertions(+), 23 deletions(-)
> >>>

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-09-09 Thread Mark Thompson
On 09/09/2019 16:40, Fu, Linjie wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Fu, Linjie
>> Sent: Friday, August 30, 2019 16:05
>> To: FFmpeg development discussions and patches > de...@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
>> hw_frames_ctx for vp9 without destroy va_context
>>
>>> -Original Message-
>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> Behalf
>>> Of Fu, Linjie
>>> Sent: Friday, August 9, 2019 19:47
>>> To: FFmpeg development discussions and patches >> de...@ffmpeg.org>
>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
>>> hw_frames_ctx for vp9 without destroy va_context
>>>
>>>> -Original Message-
>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>>> Behalf
>>>> Of Hendrik Leppkes
>>>> Sent: Friday, August 9, 2019 17:40
>>>> To: FFmpeg development discussions and patches >>> de...@ffmpeg.org>
>>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
>> recreate
>>>> hw_frames_ctx for vp9 without destroy va_context
>>>>
>>>> On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
>>>>>
>>>>>> -----Original Message-
>>>>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>>>> Behalf
>>>>>> Of Hendrik Leppkes
>>>>>> Sent: Tuesday, August 6, 2019 16:27
>>>>>> To: FFmpeg development discussions and patches >>>>> de...@ffmpeg.org>
>>>>>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
>>>> recreate
>>>>>> hw_frames_ctx for vp9 without destroy va_context
>>>>>>
>>>>>> On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
>>>>>>>
>>>>>>> VP9 allows resolution changes per frame. Currently in VAAPI,
>>> resolution
>>>>>>> changes leads to va context destroy and reinit. This will cause
>>>>>>> reference frame surface lost and produce garbage.
>>>>>>>
>>>>>>> Though refs surface id could be passed to media driver and found in
>>>>>>> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
>>>>>>> new created VaContext could only got an empty RefList.
>>>>>>>
>>>>>>> As libva allows re-create surface separately without changing the
>>>>>>> context, this issue could be handled by only recreating
>>> hw_frames_ctx.
>>>>>>>
>>>>>>> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
>>>>>>> hw_frame_ctx when dynamic resolution changing happens.
>>>>>>>
>>>>>>> Could be verified by:
>>>>>>> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
>>>>>>>   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y
>> vaapi.yuv
>>>>>>>
>>>>>>> Signed-off-by: Linjie Fu 
>>>>>>> ---
>>>>>>>  libavcodec/decode.c| 10 +-
>>>>>>>  libavcodec/internal.h  |  1 +
>>>>>>>  libavcodec/pthread_frame.c |  2 ++
>>>>>>>  libavcodec/vaapi_decode.c  | 40 ++---
>> --
>>> --
>>>> -
>>>>>> --
>>>>>>>  libavcodec/vaapi_vp9.c |  4 
>>>>>>>  5 files changed, 34 insertions(+), 23 deletions(-)
>>>>>>>
>>>>>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>>>>>>> index 0863b82..7b15fa5 100644
>>>>>>> --- a/libavcodec/decode.c
>>>>>>> +++ b/libavcodec/decode.c
>>>>>>> @@ -1254,7 +1254,6 @@ int
>>>>>> ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
>>>>>>>
>>>>>>>  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx-
>>>> data;
>>>>>>>
>>>>>>> -
>>>>>>>  if (frames_ctx->initial_pool_size) {
>>>>>>>  // We guarantee 4 base work surfaces. The function above
>>>> guarantees
>>>>&

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-09-09 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Friday, August 30, 2019 16:05
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Fu, Linjie
> > Sent: Friday, August 9, 2019 19:47
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > hw_frames_ctx for vp9 without destroy va_context
> >
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> > > Of Hendrik Leppkes
> > > Sent: Friday, August 9, 2019 17:40
> > > To: FFmpeg development discussions and patches  > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> recreate
> > > hw_frames_ctx for vp9 without destroy va_context
> > >
> > > On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
> > > >
> > > > > -Original Message-
> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > Behalf
> > > > > Of Hendrik Leppkes
> > > > > Sent: Tuesday, August 6, 2019 16:27
> > > > > To: FFmpeg development discussions and patches  > > > > de...@ffmpeg.org>
> > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > > recreate
> > > > > hw_frames_ctx for vp9 without destroy va_context
> > > > >
> > > > > On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  
> > > > > wrote:
> > > > > >
> > > > > > VP9 allows resolution changes per frame. Currently in VAAPI,
> > resolution
> > > > > > changes leads to va context destroy and reinit. This will cause
> > > > > > reference frame surface lost and produce garbage.
> > > > > >
> > > > > > Though refs surface id could be passed to media driver and found in
> > > > > > RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus 
> > > > > > the
> > > > > > new created VaContext could only got an empty RefList.
> > > > > >
> > > > > > As libva allows re-create surface separately without changing the
> > > > > > context, this issue could be handled by only recreating
> > hw_frames_ctx.
> > > > > >
> > > > > > Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > > > > > hw_frame_ctx when dynamic resolution changing happens.
> > > > > >
> > > > > > Could be verified by:
> > > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> > > > > >   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y
> vaapi.yuv
> > > > > >
> > > > > > Signed-off-by: Linjie Fu 
> > > > > > ---
> > > > > >  libavcodec/decode.c| 10 +-
> > > > > >  libavcodec/internal.h  |  1 +
> > > > > >  libavcodec/pthread_frame.c |  2 ++
> > > > > >  libavcodec/vaapi_decode.c  | 40 ++---
> --
> > --
> > > -
> > > > > --
> > > > > >  libavcodec/vaapi_vp9.c |  4 
> > > > > >  5 files changed, 34 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > > > > index 0863b82..7b15fa5 100644
> > > > > > --- a/libavcodec/decode.c
> > > > > > +++ b/libavcodec/decode.c
> > > > > > @@ -1254,7 +1254,6 @@ int
> > > > > ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
> > > > > >
> > > > > >  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx-
> > >data;
> > > > > >
> > > > > > -
> > > > > >  if (frames_ctx->initial_pool_size) {
> > > > > >  // We guarantee 4 base work surfaces. The function above
> > > guarantees
> > > > > 1
> > > > > >

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-30 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Friday, August 9, 2019 19:47
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Hendrik Leppkes
> > Sent: Friday, August 9, 2019 17:40
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > hw_frames_ctx for vp9 without destroy va_context
> >
> > On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
> > >
> > > > -Original Message-
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> > > > Of Hendrik Leppkes
> > > > Sent: Tuesday, August 6, 2019 16:27
> > > > To: FFmpeg development discussions and patches  > > > de...@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> > recreate
> > > > hw_frames_ctx for vp9 without destroy va_context
> > > >
> > > > On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
> > > > >
> > > > > VP9 allows resolution changes per frame. Currently in VAAPI,
> resolution
> > > > > changes leads to va context destroy and reinit. This will cause
> > > > > reference frame surface lost and produce garbage.
> > > > >
> > > > > Though refs surface id could be passed to media driver and found in
> > > > > RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> > > > > new created VaContext could only got an empty RefList.
> > > > >
> > > > > As libva allows re-create surface separately without changing the
> > > > > context, this issue could be handled by only recreating
> hw_frames_ctx.
> > > > >
> > > > > Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > > > > hw_frame_ctx when dynamic resolution changing happens.
> > > > >
> > > > > Could be verified by:
> > > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> > > > >   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y 
> > > > > vaapi.yuv
> > > > >
> > > > > Signed-off-by: Linjie Fu 
> > > > > ---
> > > > >  libavcodec/decode.c| 10 +-
> > > > >  libavcodec/internal.h  |  1 +
> > > > >  libavcodec/pthread_frame.c |  2 ++
> > > > >  libavcodec/vaapi_decode.c  | 40 ++-
> --
> > -
> > > > --
> > > > >  libavcodec/vaapi_vp9.c |  4 
> > > > >  5 files changed, 34 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > > > index 0863b82..7b15fa5 100644
> > > > > --- a/libavcodec/decode.c
> > > > > +++ b/libavcodec/decode.c
> > > > > @@ -1254,7 +1254,6 @@ int
> > > > ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
> > > > >
> > > > >  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx-
> >data;
> > > > >
> > > > > -
> > > > >  if (frames_ctx->initial_pool_size) {
> > > > >  // We guarantee 4 base work surfaces. The function above
> > guarantees
> > > > 1
> > > > >  // (the absolute minimum), so add the missing count.
> > > >
> > > > Unrelated whitespace change
> > >
> > > There is  a redundant whitespace here, so I removed it within this patch.
> > >
> > > > > @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext
> > *avctx,
> > > > >  return AVERROR_PATCHWELCOME;
> > > > >  }
> > > > >
> > > > > -if (hwaccel->priv_data_size) {
> > > > > +if (hwaccel->priv_data_size && !avctx->internal-
> > >hwaccel_priv_data) {
> > > > >  avctx->internal->hwaccel_priv_data =
> > > > >  av_mallocz(hwaccel->priv_data_size);
> > > > >  if (!avc

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-09 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Hendrik Leppkes
> Sent: Friday, August 9, 2019 17:40
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
> >
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > > Of Hendrik Leppkes
> > > Sent: Tuesday, August 6, 2019 16:27
> > > To: FFmpeg development discussions and patches  > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode:
> recreate
> > > hw_frames_ctx for vp9 without destroy va_context
> > >
> > > On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
> > > >
> > > > VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> > > > changes leads to va context destroy and reinit. This will cause
> > > > reference frame surface lost and produce garbage.
> > > >
> > > > Though refs surface id could be passed to media driver and found in
> > > > RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> > > > new created VaContext could only got an empty RefList.
> > > >
> > > > As libva allows re-create surface separately without changing the
> > > > context, this issue could be handled by only recreating hw_frames_ctx.
> > > >
> > > > Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > > > hw_frame_ctx when dynamic resolution changing happens.
> > > >
> > > > Could be verified by:
> > > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> > > >   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
> > > >
> > > > Signed-off-by: Linjie Fu 
> > > > ---
> > > >  libavcodec/decode.c| 10 +-
> > > >  libavcodec/internal.h  |  1 +
> > > >  libavcodec/pthread_frame.c |  2 ++
> > > >  libavcodec/vaapi_decode.c  | 40 ++---
> -
> > > --
> > > >  libavcodec/vaapi_vp9.c |  4 
> > > >  5 files changed, 34 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > > index 0863b82..7b15fa5 100644
> > > > --- a/libavcodec/decode.c
> > > > +++ b/libavcodec/decode.c
> > > > @@ -1254,7 +1254,6 @@ int
> > > ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
> > > >
> > > >  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
> > > >
> > > > -
> > > >  if (frames_ctx->initial_pool_size) {
> > > >  // We guarantee 4 base work surfaces. The function above
> guarantees
> > > 1
> > > >  // (the absolute minimum), so add the missing count.
> > >
> > > Unrelated whitespace change
> >
> > There is  a redundant whitespace here, so I removed it within this patch.
> >
> > > > @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext
> *avctx,
> > > >  return AVERROR_PATCHWELCOME;
> > > >  }
> > > >
> > > > -if (hwaccel->priv_data_size) {
> > > > +if (hwaccel->priv_data_size && !avctx->internal-
> >hwaccel_priv_data) {
> > > >  avctx->internal->hwaccel_priv_data =
> > > >  av_mallocz(hwaccel->priv_data_size);
> > > >  if (!avctx->internal->hwaccel_priv_data)
> > > > @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx,
> > > const enum AVPixelFormat *fmt)
> > > >  memcpy(choices, fmt, (n + 1) * sizeof(*choices));
> > > >
> > > >  for (;;) {
> > > > -// Remove the previous hwaccel, if there was one.
> > > > -hwaccel_uninit(avctx);
> > > > -
> > > > +// Remove the previous hwaccel, if there was one,
> > > > +// and no need for keeping.
> > > > +if (!avctx->internal->hwaccel_priv_data_keeping)
> > > > +hwaccel_uninit(avctx);
> > > >  user_choice = avctx->get_format(avctx, choices);
> > > >  if (user_

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-09 Thread Hendrik Leppkes
On Tue, Aug 6, 2019 at 10:55 AM Fu, Linjie  wrote:
>
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Hendrik Leppkes
> > Sent: Tuesday, August 6, 2019 16:27
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> > hw_frames_ctx for vp9 without destroy va_context
> >
> > On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
> > >
> > > VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> > > changes leads to va context destroy and reinit. This will cause
> > > reference frame surface lost and produce garbage.
> > >
> > > Though refs surface id could be passed to media driver and found in
> > > RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> > > new created VaContext could only got an empty RefList.
> > >
> > > As libva allows re-create surface separately without changing the
> > > context, this issue could be handled by only recreating hw_frames_ctx.
> > >
> > > Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > > hw_frame_ctx when dynamic resolution changing happens.
> > >
> > > Could be verified by:
> > > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> > >   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > >  libavcodec/decode.c| 10 +-
> > >  libavcodec/internal.h  |  1 +
> > >  libavcodec/pthread_frame.c |  2 ++
> > >  libavcodec/vaapi_decode.c  | 40 ++
> > --
> > >  libavcodec/vaapi_vp9.c |  4 
> > >  5 files changed, 34 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > index 0863b82..7b15fa5 100644
> > > --- a/libavcodec/decode.c
> > > +++ b/libavcodec/decode.c
> > > @@ -1254,7 +1254,6 @@ int
> > ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
> > >
> > >  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
> > >
> > > -
> > >  if (frames_ctx->initial_pool_size) {
> > >  // We guarantee 4 base work surfaces. The function above 
> > > guarantees
> > 1
> > >  // (the absolute minimum), so add the missing count.
> >
> > Unrelated whitespace change
>
> There is  a redundant whitespace here, so I removed it within this patch.
>
> > > @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
> > >  return AVERROR_PATCHWELCOME;
> > >  }
> > >
> > > -if (hwaccel->priv_data_size) {
> > > +if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
> > >  avctx->internal->hwaccel_priv_data =
> > >  av_mallocz(hwaccel->priv_data_size);
> > >  if (!avctx->internal->hwaccel_priv_data)
> > > @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx,
> > const enum AVPixelFormat *fmt)
> > >  memcpy(choices, fmt, (n + 1) * sizeof(*choices));
> > >
> > >  for (;;) {
> > > -// Remove the previous hwaccel, if there was one.
> > > -hwaccel_uninit(avctx);
> > > -
> > > +// Remove the previous hwaccel, if there was one,
> > > +// and no need for keeping.
> > > +if (!avctx->internal->hwaccel_priv_data_keeping)
> > > +hwaccel_uninit(avctx);
> > >  user_choice = avctx->get_format(avctx, choices);
> > >  if (user_choice == AV_PIX_FMT_NONE) {
> > >  // Explicitly chose nothing, give up.
> >
> > There could be a dozen special cases how stuff can go wrong here. What
> > if get_format actually returns a different format then the one
> > currently in use? Or a software format?
> > Just removing this alone is not safe.
>
> Didn't quite get your point.
> IMHO,  avctx->internal->hwaccel_priv_data_keeping won't be set in other cases
> other than vaapi_vp9, so this patch won't break the default pipeline, and
> hwaccel_uninit(avctx) will always be called.
>

The point is that you cannot rely on get_format to return the same
format that it previously did. It could return a software format, or
in some cases possibly even a different hardware format. And you just
don't handle that.

The entire approach here smells a bit of hack. Lets try to think this
through and do it properly. One idea that comes to mind is a new
hwaccel callback that checks if a in-place re-init is possible without
destroying everything. This could be used for a multitude of different
situations, and not just this one special case.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-06 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Hendrik Leppkes
> Sent: Tuesday, August 6, 2019 16:27
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx for vp9 without destroy va_context
> 
> On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
> >
> > VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> > changes leads to va context destroy and reinit. This will cause
> > reference frame surface lost and produce garbage.
> >
> > Though refs surface id could be passed to media driver and found in
> > RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> > new created VaContext could only got an empty RefList.
> >
> > As libva allows re-create surface separately without changing the
> > context, this issue could be handled by only recreating hw_frames_ctx.
> >
> > Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> > hw_frame_ctx when dynamic resolution changing happens.
> >
> > Could be verified by:
> > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> >   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >  libavcodec/decode.c| 10 +-
> >  libavcodec/internal.h  |  1 +
> >  libavcodec/pthread_frame.c |  2 ++
> >  libavcodec/vaapi_decode.c  | 40 ++
> --
> >  libavcodec/vaapi_vp9.c |  4 
> >  5 files changed, 34 insertions(+), 23 deletions(-)
> >
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index 0863b82..7b15fa5 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -1254,7 +1254,6 @@ int
> ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
> >
> >  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
> >
> > -
> >  if (frames_ctx->initial_pool_size) {
> >  // We guarantee 4 base work surfaces. The function above guarantees
> 1
> >  // (the absolute minimum), so add the missing count.
> 
> Unrelated whitespace change

There is  a redundant whitespace here, so I removed it within this patch.

> > @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
> >  return AVERROR_PATCHWELCOME;
> >  }
> >
> > -if (hwaccel->priv_data_size) {
> > +if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
> >  avctx->internal->hwaccel_priv_data =
> >  av_mallocz(hwaccel->priv_data_size);
> >  if (!avctx->internal->hwaccel_priv_data)
> > @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx,
> const enum AVPixelFormat *fmt)
> >  memcpy(choices, fmt, (n + 1) * sizeof(*choices));
> >
> >  for (;;) {
> > -// Remove the previous hwaccel, if there was one.
> > -hwaccel_uninit(avctx);
> > -
> > +// Remove the previous hwaccel, if there was one,
> > +// and no need for keeping.
> > +if (!avctx->internal->hwaccel_priv_data_keeping)
> > +hwaccel_uninit(avctx);
> >  user_choice = avctx->get_format(avctx, choices);
> >  if (user_choice == AV_PIX_FMT_NONE) {
> >  // Explicitly chose nothing, give up.
> 
> There could be a dozen special cases how stuff can go wrong here. What
> if get_format actually returns a different format then the one
> currently in use? Or a software format?
> Just removing this alone is not safe.

Didn't quite get your point.
IMHO,  avctx->internal->hwaccel_priv_data_keeping won't be set in other cases
other than vaapi_vp9, so this patch won't break the default pipeline, and
hwaccel_uninit(avctx) will always be called.

> > diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> > index 5096ffa..7adef08 100644
> > --- a/libavcodec/internal.h
> > +++ b/libavcodec/internal.h
> > @@ -188,6 +188,7 @@ typedef struct AVCodecInternal {
> >   * hwaccel-specific private data
> >   */
> >  void *hwaccel_priv_data;
> > +int hwaccel_priv_data_keeping;
> >
> >  /**
> >   * checks API usage: after codec draining, flush is required to resume
> operation
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 36ac0ac..6032818 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-06 Thread Hendrik Leppkes
On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu  wrote:
>
> VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> changes leads to va context destroy and reinit. This will cause
> reference frame surface lost and produce garbage.
>
> Though refs surface id could be passed to media driver and found in
> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> new created VaContext could only got an empty RefList.
>
> As libva allows re-create surface separately without changing the
> context, this issue could be handled by only recreating hw_frames_ctx.
>
> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> hw_frame_ctx when dynamic resolution changing happens.
>
> Could be verified by:
> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
>   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
>
> Signed-off-by: Linjie Fu 
> ---
>  libavcodec/decode.c| 10 +-
>  libavcodec/internal.h  |  1 +
>  libavcodec/pthread_frame.c |  2 ++
>  libavcodec/vaapi_decode.c  | 40 ++--
>  libavcodec/vaapi_vp9.c |  4 
>  5 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 0863b82..7b15fa5 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1254,7 +1254,6 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
>
>  frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
>
> -
>  if (frames_ctx->initial_pool_size) {
>  // We guarantee 4 base work surfaces. The function above guarantees 1
>  // (the absolute minimum), so add the missing count.

Unrelated whitespace change

> @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
>  return AVERROR_PATCHWELCOME;
>  }
>
> -if (hwaccel->priv_data_size) {
> +if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
>  avctx->internal->hwaccel_priv_data =
>  av_mallocz(hwaccel->priv_data_size);
>  if (!avctx->internal->hwaccel_priv_data)
> @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> AVPixelFormat *fmt)
>  memcpy(choices, fmt, (n + 1) * sizeof(*choices));
>
>  for (;;) {
> -// Remove the previous hwaccel, if there was one.
> -hwaccel_uninit(avctx);
> -
> +// Remove the previous hwaccel, if there was one,
> +// and no need for keeping.
> +if (!avctx->internal->hwaccel_priv_data_keeping)
> +hwaccel_uninit(avctx);
>  user_choice = avctx->get_format(avctx, choices);
>  if (user_choice == AV_PIX_FMT_NONE) {
>  // Explicitly chose nothing, give up.

There could be a dozen special cases how stuff can go wrong here. What
if get_format actually returns a different format then the one
currently in use? Or a software format?
Just removing this alone is not safe.

> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 5096ffa..7adef08 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -188,6 +188,7 @@ typedef struct AVCodecInternal {
>   * hwaccel-specific private data
>   */
>  void *hwaccel_priv_data;
> +int hwaccel_priv_data_keeping;
>
>  /**
>   * checks API usage: after codec draining, flush is required to resume 
> operation
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 36ac0ac..6032818 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -283,6 +283,7 @@ static int update_context_from_thread(AVCodecContext 
> *dst, AVCodecContext *src,
>  dst->sample_fmt = src->sample_fmt;
>  dst->channel_layout = src->channel_layout;
>  dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data;
> +dst->internal->hwaccel_priv_data_keeping = 
> src->internal->hwaccel_priv_data_keeping;
>
>  if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx ||
>  (dst->hw_frames_ctx && dst->hw_frames_ctx->data != 
> src->hw_frames_ctx->data)) {
> @@ -340,6 +341,7 @@ static int update_context_from_user(AVCodecContext *dst, 
> AVCodecContext *src)
>  dst->frame_number = src->frame_number;
>  dst->reordered_opaque = src->reordered_opaque;
>  dst->thread_safe_callbacks = src->thread_safe_callbacks;
> +dst->internal->hwaccel_priv_data_keeping = 
> src->internal->hwaccel_priv_data_keeping;
>
>  if (src->slice_count && src->slice_offset) {
>  if (dst->slice_count < src->slice_count) {
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index 69512e1..13f0cf0 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
>  VAStatus vas;
>  int err;
>
> -ctx->va_config  = VA_INVALID_ID;
> -ctx->va_context = VA_INVALID_ID;
> +if (!ctx->va_config && 

Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context

2019-08-05 Thread Fu, Linjie
> -Original Message-
> From: Fu, Linjie
> Sent: Friday, July 12, 2019 04:18
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie 
> Subject: [PATCH,v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9
> without destroy va_context
> 
> VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> changes leads to va context destroy and reinit. This will cause
> reference frame surface lost and produce garbage.
> 
> Though refs surface id could be passed to media driver and found in
> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> new created VaContext could only got an empty RefList.
> 
> As libva allows re-create surface separately without changing the
> context, this issue could be handled by only recreating hw_frames_ctx.
> 
> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> hw_frame_ctx when dynamic resolution changing happens.
> 
> Could be verified by:
> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
>   ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
> 
> Signed-off-by: Linjie Fu 
> ---
Ping?
This patch set fixes 1000+ dynamic resolution cases and makes great sense.
Please feel free to comment.

Thanks,
linjie
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2019-07-11 Thread Linjie Fu
VP9 allows resolution changes per frame. Currently in VAAPI, resolution
changes leads to va context destroy and reinit. This will cause
reference frame surface lost and produce garbage.

Though refs surface id could be passed to media driver and found in
RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
new created VaContext could only got an empty RefList.

As libva allows re-create surface separately without changing the
context, this issue could be handled by only recreating hw_frames_ctx.

Set hwaccel_priv_data_keeping flag for vp9 to only recreating
hw_frame_ctx when dynamic resolution changing happens.

Could be verified by:
ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
  ./resolutions.ivf -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv

Signed-off-by: Linjie Fu 
---
 libavcodec/decode.c| 10 +-
 libavcodec/internal.h  |  1 +
 libavcodec/pthread_frame.c |  2 ++
 libavcodec/vaapi_decode.c  | 40 ++--
 libavcodec/vaapi_vp9.c |  4 
 5 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 0863b82..7b15fa5 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1254,7 +1254,6 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
 
 frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
 
-
 if (frames_ctx->initial_pool_size) {
 // We guarantee 4 base work surfaces. The function above guarantees 1
 // (the absolute minimum), so add the missing count.
@@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
 return AVERROR_PATCHWELCOME;
 }
 
-if (hwaccel->priv_data_size) {
+if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
 avctx->internal->hwaccel_priv_data =
 av_mallocz(hwaccel->priv_data_size);
 if (!avctx->internal->hwaccel_priv_data)
@@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
 memcpy(choices, fmt, (n + 1) * sizeof(*choices));
 
 for (;;) {
-// Remove the previous hwaccel, if there was one.
-hwaccel_uninit(avctx);
-
+// Remove the previous hwaccel, if there was one,
+// and no need for keeping.
+if (!avctx->internal->hwaccel_priv_data_keeping)
+hwaccel_uninit(avctx);
 user_choice = avctx->get_format(avctx, choices);
 if (user_choice == AV_PIX_FMT_NONE) {
 // Explicitly chose nothing, give up.
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 5096ffa..7adef08 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -188,6 +188,7 @@ typedef struct AVCodecInternal {
  * hwaccel-specific private data
  */
 void *hwaccel_priv_data;
+int hwaccel_priv_data_keeping;
 
 /**
  * checks API usage: after codec draining, flush is required to resume 
operation
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 36ac0ac..6032818 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -283,6 +283,7 @@ static int update_context_from_thread(AVCodecContext *dst, 
AVCodecContext *src,
 dst->sample_fmt = src->sample_fmt;
 dst->channel_layout = src->channel_layout;
 dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data;
+dst->internal->hwaccel_priv_data_keeping = 
src->internal->hwaccel_priv_data_keeping;
 
 if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx ||
 (dst->hw_frames_ctx && dst->hw_frames_ctx->data != 
src->hw_frames_ctx->data)) {
@@ -340,6 +341,7 @@ static int update_context_from_user(AVCodecContext *dst, 
AVCodecContext *src)
 dst->frame_number = src->frame_number;
 dst->reordered_opaque = src->reordered_opaque;
 dst->thread_safe_callbacks = src->thread_safe_callbacks;
+dst->internal->hwaccel_priv_data_keeping = 
src->internal->hwaccel_priv_data_keeping;
 
 if (src->slice_count && src->slice_offset) {
 if (dst->slice_count < src->slice_count) {
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index 69512e1..13f0cf0 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
 VAStatus vas;
 int err;
 
-ctx->va_config  = VA_INVALID_ID;
-ctx->va_context = VA_INVALID_ID;
+if (!ctx->va_config && !ctx->va_context){
+ctx->va_config  = VA_INVALID_ID;
+ctx->va_context = VA_INVALID_ID;
+}
 
 #if FF_API_STRUCT_VAAPI_CONTEXT
 if (avctx->hwaccel_context) {
@@ -642,7 +644,6 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
 // present, so set it here to avoid the behaviour changing.
 ctx->hwctx->driver_quirks =
 AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS;
-
 }
 #endif
 
@@ -655,7 +656,8 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)