[FFmpeg-devel] [PATCH, v2] lavc/qsvdec: Add GPU-accelerated memory copy support

[Fu, Linjie]

> -----Original Message-----
> From: Li, Zhong <zhong.li at intel.com>
> Sent: Sunday, September 29, 2019 11:57
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: ChaoX A Liu <chaox.a.liu at intel.com>; Fu, Linjie <linjie.fu at intel.com>
> Subject: RE: [FFmpeg-devel] [PATCH, v2] lavc/qsvdec: Add GPU-accelerated
> memory copy support
> 
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Linjie Fu
> > Sent: Friday, September 27, 2019 1:47 PM
> > To: ffmpeg-devel at ffmpeg.org
> > Cc: ChaoX A Liu <chaox.a.liu at intel.com>; Fu, Linjie <linjie.fu at intel.com>
> > Subject: [FFmpeg-devel] [PATCH, v2] lavc/qsvdec: Add GPU-accelerated
> memory
> > copy support
> >
> > GPU copy enables or disables GPU accelerated copying between video and
> > system memory. This may lead to a notable performance improvement.
> > Memory must be sequent and aligned with 128x64.
> > (first introduced in FFmpeg 3.3.1)
> 
> This line should be removed. FFmpeg 3.3.1 mainline never support GPU copy.
> 

Double confirmed and removed.

> >
> > CMD:
> > ffmpeg -init_hw_device qsv=hw -filter_hw_device hw -c:v h264_qsv
> >                     -gpu_copy on -i input.h264 -f null -
> > or:
> > ffmpeg -c:v h264_qsv -gpu_copy on -i input.h264 -f null -
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > Signed-off-by: ChaoX A Liu <chaox.a.liu at intel.com>
> > ---
> > Rebased and send again.
> >
> >  libavcodec/qsv.c          | 31 +++++++++++++++++-------
> >  libavcodec/qsv_internal.h |  7 +++---
> >  libavcodec/qsvdec.c       | 50 ++++++++++++++++++++++++++++++++++--
> ---
> >  libavcodec/qsvdec.h       |  2 ++
> >  libavcodec/qsvdec_h2645.c | 10 ++++++++  libavcodec/qsvdec_other.c |  5
> ++++
> >  libavcodec/qsvenc.c       |  8 ++++---
> >  7 files changed, 92 insertions(+), 21 deletions(-)
> >
> > diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c index
> 994c9ebcb0..9e66fbc9da
> > 100644
> > --- a/libavcodec/qsv.c
> > +++ b/libavcodec/qsv.c
> > @@ -412,15 +412,19 @@ static int
> ff_qsv_set_display_handle(AVCodecContext
> > *avctx, QSVSession *qs)  #endif
> //AVCODEC_QSV_LINUX_SESSION_HANDLE
> >
> >  int ff_qsv_init_internal_session(AVCodecContext *avctx, QSVSession *qs,
> > -                                 const char *load_plugins)
> > +                                 const char *load_plugins, int
> > + gpu_copy)
> >  {
> > -    mfxIMPL impl   = MFX_IMPL_AUTO_ANY;
> > -    mfxVersion ver = { { QSV_VERSION_MINOR, QSV_VERSION_MAJOR } };
> > +    mfxIMPL          impl = MFX_IMPL_AUTO_ANY;
> > +    mfxVersion        ver = { { QSV_VERSION_MINOR,
> QSV_VERSION_MAJOR } };
> > +    mfxInitParam init_par = { MFX_IMPL_AUTO_ANY };
> >
> >      const char *desc;
> >      int ret;
> >
> > -    ret = MFXInit(impl, &ver, &qs->session);
> > +    init_par.GPUCopy        = gpu_copy;
> 
> GPUCopy field is introduced from API 1.16, would better to check it to avoid
> compile issue with old API.

> > @@ -802,8 +811,12 @@ int ff_qsv_init_session_frames(AVCodecContext
> *avctx,
> > mfxSession *psession,
> >
> >      int ret;
> >
> > +    if (gpu_copy == MFX_GPUCOPY_ON)
> > +        av_log(avctx, AV_LOG_WARNING, "GPU-accelerated memory copy "
> > +                                    "only works in
> > + MFX_IOPATTERN_OUT_SYSTEM_MEMORY.\n");
> 
> This looks weird：
> 1.  the waring log will always turn on if gpu_copy is true no matter what
> iopattern.
>      So would be better:
>     if (gpu_copy == MFX_GPUCOPY_ON && iopattern != system memory)
>         print a warning.
> 
> 2. It is only added for ff_qsv_init_session_frames(), but looks like should be
> apply for qsv_init_session()
> 

Thanks, will update and resend the patch soon.

> >
> > +static int ff_qsv_get_continuous_buffer(AVCodecContext *avctx,
> AVFrame
> > +*frame, AVBufferPool *pool) {
> > +    int ret = 0;
> > +
> > +    ff_decode_frame_props(avctx, frame);
> > +
> > +    frame->width       = avctx->width;
> > +    frame->height      = avctx->height;
> > +    frame->linesize[0] = FFALIGN(avctx->width, 128);
> > +    frame->linesize[1] = frame->linesize[0];
> > +    frame->buf[0]      = av_buffer_pool_get(pool);
> > +    if (!frame->buf[0])
> > +        return AVERROR(ENOMEM);
> > +
> > +    frame->data[0] = frame->buf[0]->data;
> > +    frame->data[1] = frame->data[0] +
> > +                            frame->linesize[0] * FFALIGN(avctx->height,
> > + 64);
> > +
> > +    ret = ff_attach_decode_data(frame);
> 
> Could you please explain why need this function? I don't see private_ref is
> needed from qsv decoding.

private_ref is required if a decoder declares the capability of AV_CODEC_CAP_DR1.
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/decode.c#L654

- linjie