[FFmpeg-devel] [PATCH, v2 1/2] lavf/qsvvpp: allocate continuous memory

[Li, Zhong]

> -----Original Message-----
> From: Fu, Linjie
> Sent: Monday, June 3, 2019 1:17 PM
> To: Li, Zhong <zhong.li at intel.com>; FFmpeg development discussions and
> patches <ffmpeg-devel at ffmpeg.org>
> Subject: RE: [FFmpeg-devel] [PATCH, v2 1/2] lavf/qsvvpp: allocate
> continuous memory
> 
> > -----Original Message-----
> > From: Li, Zhong
> > Sent: Friday, May 31, 2019 17:20
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel at ffmpeg.org>
> > Cc: Fu, Linjie <linjie.fu at intel.com>
> > Subject: RE: [FFmpeg-devel] [PATCH, v2 1/2] lavf/qsvvpp: allocate
> > continuous memory
> >
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> > Behalf
> > > Of Linjie Fu
> > > Sent: Thursday, May 30, 2019 1:01 AM
> > > To: ffmpeg-devel at ffmpeg.org
> > > Cc: Fu, Linjie <linjie.fu at intel.com>
> > > Subject: [FFmpeg-devel] [PATCH, v2 1/2] lavf/qsvvpp: allocate
> > > continuous memory
> > >
> > > Mediasdk calls CMRT to copy from video to system memory and requires
> > > memory to be continuously allocated across Y and UV.
> > >
> > > Add a new path to allocate continuous memory when using system out.
> > > Use av_image_fill_pointers to arrange data according to pixfmt.
> > >
> > > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > > ---
> > > [v2]: use av_image_fill_pointers
> > >
> > >  libavfilter/qsvvpp.c | 32 +++++++++++++++++++++++++++-----
> > >  1 file changed, 27 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c index
> > > 06efdf5089..6eeed7e632 100644
> > > --- a/libavfilter/qsvvpp.c
> > > +++ b/libavfilter/qsvvpp.c
> > > @@ -27,6 +27,7 @@
> > >  #include "libavutil/hwcontext_qsv.h"
> > >  #include "libavutil/time.h"
> > >  #include "libavutil/pixdesc.h"
> > > +#include "libavutil/imgutils.h"
> > >
> > >  #include "internal.h"
> > >  #include "qsvvpp.h"
> > > @@ -51,6 +52,7 @@ struct QSVVPPContext {
> > >      enum AVPixelFormat  out_sw_format;   /* Real output format
> */
> > >      mfxVideoParam       vpp_param;
> > >      mfxFrameInfo       *frame_infos;     /* frame info for each
> > > input */
> > > +    AVBufferPool       *pool;
> > >
> > >      /* members related to the input/output surface */
> > >      int                 in_mem_mode;
> > > @@ -375,10 +377,24 @@ static QSVFrame
> *query_frame(QSVVPPContext *s,
> > > AVFilterLink *outlink)
> > >          out_frame->surface = (mfxFrameSurface1
> > > *)out_frame->frame->data[3];
> > >      } else {
> > >          /* Get a frame with aligned dimensions.
> > > -         * Libmfx need system memory being 128x64 aligned */
> > > -        out_frame->frame = ff_get_video_buffer(outlink,
> > > -
> > > FFALIGN(outlink->w, 128),
> > > -
> > > FFALIGN(outlink->h, 64));
> > > +         * Libmfx need system memory being 128x64 aligned
> > > +         * and continuously allocated across Y and UV */
> > > +        out_frame->frame = av_frame_alloc();
> > > +        if (!out_frame->frame) {
> > > +            return NULL;
> >
> > Should be better to return AVERROR(ENOMEM)?
> 
> Will refine.
> 
> >
> > > +        }
> > > +
> > > +        out_frame->frame->linesize[0] = FFALIGN(outlink->w, 128);
> > > +        out_frame->frame->linesize[1] =
> out_frame->frame->linesize[0];
> > > +        out_frame->frame->buf[0]      =
> av_buffer_pool_get(s->pool);
> > > +        out_frame->frame->format      = outlink->format;
> > > +
> > > +        if (!out_frame->frame->buf[0])
> > > +            return NULL;
> >
> > Same as frame alloc.
> 
> Will refine.
> > 1. What is the benefit to use a pool? Comparing with directly alloc a
> > buffer use ()?
> Directly allocate seems to be better.

I am thinking using av_frame_get_buffer() will make life easier or not. 
It can make sure memory continuous and call av_buffer_alloc() and av_image_fill_pointers() internally.