Message ID | 20220506055711.1671-1-tong1.wu@intel.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v4,1/3] avutil/hwcontext_qsv: derive QSV frames to D3D11VA frames | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
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@d3d11 -c:v h264_qsv -i input.h264 \ > -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null - > > Signed-off-by: Tong Wu <tong1.wu@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); Thanks Haihao > + } > + dst_hwctx->BindFlags = texDesc.BindFlags; > } > break; > #endif
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Xiang, Haihao > Sent: Saturday, May 7, 2022 4:36 AM > To: ffmpeg-devel@ffmpeg.org > Cc: Wu, Tong1 <tong1.wu@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@d3d11 -c:v h264_qsv -i input.h264 \ > > -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null - > > > > Signed-off-by: Tong Wu <tong1.wu@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 sw
On Sat, 2022-05-07 at 02:42 +0000, Soft Works wrote: > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Xiang, Haihao > > Sent: Saturday, May 7, 2022 4:36 AM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Wu, Tong1 <tong1.wu@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@d3d11 -c:v h264_qsv -i input.h264 \ > > > -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null - > > > > > > Signed-off-by: Tong Wu <tong1.wu@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
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Xiang, Haihao > Sent: Saturday, May 7, 2022 7:25 AM > To: ffmpeg-devel@ffmpeg.org > Cc: Wu, Tong1 <tong1.wu@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH v4 1/3] avutil/hwcontext_qsv: > derive QSV frames to D3D11VA frames > > On Sat, 2022-05-07 at 02:42 +0000, Soft Works wrote: > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Xiang, Haihao > > > Sent: Saturday, May 7, 2022 4:36 AM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Wu, Tong1 <tong1.wu@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@d3d11 -c:v h264_qsv -i input.h264 \ > > > > -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null - > > > > > > > > Signed-off-by: Tong Wu <tong1.wu@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; Yea that’s right. That would have to go into the if block, but you're right, the whole thing would fail if nb_surfaces is 0 rather it wouldn't even get that far that this function would be called, so it really doesn't matter, yet the getdesc and the BindFlags assignment should better be together (wherever). Thanks, softworkz
> On Sat, 2022-05-07 at 02:42 +0000, Soft Works wrote: > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > > Xiang, Haihao > > > Sent: Saturday, May 7, 2022 4:36 AM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Wu, Tong1 <tong1.wu@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@d3d11 -c:v h264_qsv -i input.h264 \ -vf > > > > "hwmap=derive_device=d3d11va,format=d3d11" -f null - > > > > > > > > Signed-off-by: Tong Wu <tong1.wu@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
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)); 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); + } + } + dst_hwctx->BindFlags = texDesc.BindFlags; } break; #endif
Fixes: $ ffmpeg.exe -y -hwaccel qsv -init_hw_device d3d11va=d3d11 \ -init_hw_device qsv=qsv@d3d11 -c:v h264_qsv -i input.h264 \ -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null - Signed-off-by: Tong Wu <tong1.wu@intel.com> --- libavutil/hwcontext_qsv.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)