On Tue, 2018-09-18 at 16:45 +0000, Li, Zhong wrote:
> > From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf
> > Of Luca
> > Barbato
> > Sent: Wednesday, September 19, 2018 12:11 AM
> > To: libav-devel@libav.org
> > Subject: Re: [libav-devel] [PATCH] qsvenc: AV_PIX_FMT_QSV path
> > should
> > release frame
> > 
> > On 18/09/2018 09:54, Maxym Dmytrychenko wrote:
> > > Fixes high memory usage, now is back to normal.
> > > 
> > > Can be checked as:
> > > -hwaccel qsv -c:v h264_qsv -i ../h264-
> > > conformance/CANL2_Sony_E.jsv
> > > -c:v h264_qsv -b:v 2000k -y qsv.mp4
> > > ---
> > >  libavcodec/qsvenc.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > > 611449cbe..17a0559f3 100644
> > > --- a/libavcodec/qsvenc.c
> > > +++ b/libavcodec/qsvenc.c
> > > @@ -1028,6 +1028,9 @@ static void
> > 
> > clear_unused_frames(QSVEncContext *q)
> > >      QSVFrame *cur = q->work_frames;
> > >      while (cur) {
> > >          if (cur->used && !cur->surface.Data.Locked) {
> > > +            if (cur->frame->format == AV_PIX_FMT_QSV) {
> > > +                av_frame_unref(cur->frame);
> > > +            }
> > >              cur->used = 0;
> > >          }
> > >          cur = cur->next;
> > > 
> > 
> > Please put a note about why this is needed (and safe to do).
> > 
> > lu
> 
> I am still confused by the original commit
> (https://patchwork.libav.org/patch/64323/ ) which introduce the
> regression:
Since this is a regression and the commit which introduced it is
identified, I suggest to explicitly mark it in commit message. For
example with:

Fixes: "qsv: enforcing continuous memory layout"

>          if (cur->used && !cur->surface.Data.Locked) {
> -            av_frame_unref(cur->frame);
>              cur->used = 0;
>          }
> It made sense for qsv decoding + qsv encoding pipeline(both GPU
> memory mode, thus format = AV_PIX_FMT_QSV, and system memory mode,
> thus format != AV_PIX_FMT_QSV). 
> Because if it is unlocked, qsv decoder doesn't need this surface any
> more, then we can release it. 
> Removing "av_frame_unref(cur->frame)" is breaking qsv decoding + qsv
> encoding pipeline.
This is not reflected in the commit message which just claims that
there is "a high memory usage". But what I see is transcoding don't
work returning errors. I believe this should be highlighted int he
commit message.

>  The patch above just fix GPU memory mode case, but doesn’t work for
> system mode case of qsv decoding+ qsv encoding. 
> 
> 
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to