[FFmpeg-devel] [PATCH v4 1/3] avutil/hwcontext_qsv: derive QSV frames to D3D11VA frames

[Wu, Tong1]

> On Sat, 2022-05-07 at 02:42 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Xiang, Haihao
> > > Sent: Saturday, May 7, 2022 4:36 AM
> > > To: ffmpeg-devel at ffmpeg.org
> > > Cc: Wu, Tong1 <tong1.wu at intel.com>
> > > Subject: Re: [FFmpeg-devel] [PATCH v4 1/3] avutil/hwcontext_qsv:
> > > derive QSV frames to D3D11VA frames
> > >
> > > On Fri, 2022-05-06 at 05:57 +0000, Tong Wu wrote:
> > > > Fixes:
> > > > $ ffmpeg.exe -y -hwaccel qsv -init_hw_device d3d11va=d3d11 \
> > > > -init_hw_device qsv=qsv at d3d11 -c:v h264_qsv -i input.h264 \ -vf
> > > > "hwmap=derive_device=d3d11va,format=d3d11" -f null -
> > > >
> > > > Signed-off-by: Tong Wu <tong1.wu at intel.com>
> > > > ---
> > > >  libavutil/hwcontext_qsv.c | 16 +++++++++++++---
> > > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > > > index b28dcffe2a..bf150c8553 100644
> > > > --- a/libavutil/hwcontext_qsv.c
> > > > +++ b/libavutil/hwcontext_qsv.c
> > > > @@ -1281,12 +1281,22 @@ static int
> > >
> > > qsv_frames_derive_from(AVHWFramesContext
> > > > *dst_ctx,
> > > >  #if CONFIG_D3D11VA
> > > >      case AV_HWDEVICE_TYPE_D3D11VA:
> > > >          {
> > > > +            D3D11_TEXTURE2D_DESC texDesc;
> > > > +            dst_ctx->initial_pool_size = src_ctx-
> > > > initial_pool_size;
> > > >              AVD3D11VAFramesContext *dst_hwctx = dst_ctx->hwctx;
> > > > -            mfxHDLPair *pair = (mfxHDLPair*)src_hwctx-
> > > > > surfaces[i].Data.MemId;
> > > >
> > > > -            dst_hwctx->texture = (ID3D11Texture2D*)pair->first;
> > > > +            dst_hwctx->texture_infos = av_calloc(src_hwctx-
> > > > nb_surfaces,
> > > > +
> > > > + sizeof(*dst_hwctx-
> > > > > texture_infos));
> > >
> > > Please check whether the pointer is NULL
> > >
> > > >              if (src_hwctx->frame_type &
> > >
> > > MFX_MEMTYPE_SHARED_RESOURCE)
> > > >                  dst_hwctx->MiscFlags = D3D11_RESOURCE_MISC_SHARED;
> > > > -            dst_hwctx->BindFlags =
> > >
> > > qsv_get_d3d11va_bind_flags(src_hwctx-
> > > > > frame_type);
> > > >
> > > > +            for (i = 0; i < src_hwctx->nb_surfaces; i++) {
> > > > +                mfxHDLPair *pair = (mfxHDLPair*)src_hwctx-
> > > > > surfaces[i].Data.MemId;
> > > >
> > > > +                dst_hwctx->texture_infos[i].texture =
> > >
> > > (ID3D11Texture2D*)pair-
> > > > > first;
> > > >
> > > > +                dst_hwctx->texture_infos[i].index = pair->second
> > > > + ==
> > > > (mfxMemId)MFX_INFINITE ? (intptr_t)0 : (intptr_t)pair->second;
> > > > +                if (i == 0) {
> > > > +                    ID3D11Texture2D_GetDesc(dst_hwctx-
> > > > > texture_infos[i].texture, &texDesc);
> > > >
> > > > +                }
> > >
> > > Move this out of the for-loop ? You may call
> > > ID3D11Texture2D_GetDesc()
> > > below:
> > >
> > > ID3D11Texture2D_GetDesc(dst_hwctx->texture_infos[0].texture,
> > > &texDesc);
> >
> > This could crash when src_hwctx->nb_surfaces is 0
> 
> src_hwctx->nb_surface should be greater than 0, see qsv_init_pool() and
> qsv_frames_derive_to().
> 
> On the other hand, dst_hwctx->texture_infos is NULL if src_hwctx-
> >nb_surfaces is 0. I commented that we should check whether the pointer is
> NULL. In addition, texDesc.BindFlags would be uninitialized if src_hwctx-
> >nb_surfaces is 0, so we couldn't use it in the following assignment.
> 
> dst_hwctx->BindFlags = texDesc.BindFlags;
> 
> Thanks
> Haihao
> 
Yes that's right. I am going to modify the code as suggested and resubmit them.

Regards,
Tong