diff mbox series

[FFmpeg-devel] libavutil/hwcontext_qsv: fix a bug for mapping qsv frame to vaapi

Message ID 20210909091301.2629659-1-wenbin.chen@intel.com
State New
Headers show
Series [FFmpeg-devel] libavutil/hwcontext_qsv: fix a bug for mapping qsv frame to vaapi
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Wenbin Chen Sept. 9, 2021, 9:13 a.m. UTC
Command below failed.
ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
-init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
-filter_hw_device va -c:v h264_qsv
-i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264

Cause: Assign pair->first directly to data[3] in vaapi frame.
pair->first is *VASurfaceID while data[3] in vaapi frame is
VASurfaceID. I fix this line of code. Now the command above works.

Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
---
 libavutil/hwcontext_qsv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Almer Sept. 9, 2021, 12:48 p.m. UTC | #1
On 9/9/2021 6:13 AM, Wenbin Chen wrote:
> Command below failed.
> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
> -init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
> -filter_hw_device va -c:v h264_qsv
> -i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264
> 
> Cause: Assign pair->first directly to data[3] in vaapi frame.
> pair->first is *VASurfaceID while data[3] in vaapi frame is
> VASurfaceID. I fix this line of code. Now the command above works.
> 
> Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> ---
>   libavutil/hwcontext_qsv.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index d431e71eab..6539cae619 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -781,7 +781,7 @@ static int qsv_map_from(AVHWFramesContext *ctx,
>       case AV_HWDEVICE_TYPE_VAAPI:
>       {
>           mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
> -        child_data = pair->first;
> +        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;

You can probably remove the intptr_t casting.

Also, shouldn't this same fix be done for all three child device types 
used in this function? Which for that matter, child_data seems to be set 
for a d3d11va child device, but then never used.

>           break;
>       }
>   #endif
>
Soft Works Sept. 9, 2021, 6 p.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Wenbin Chen
> Sent: Thursday, 9 September 2021 11:13
> To: ffmpeg-devel@ffmpeg.org
> Cc: Wenbin Chen <wenbin.chen@intel.com>
> Subject: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: fix a bug
> for mapping qsv frame to vaapi
> 
> Command below failed.
> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
> -init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
> -filter_hw_device va -c:v h264_qsv
> -i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264
> 
> Cause: Assign pair->first directly to data[3] in vaapi frame.
> pair->first is *VASurfaceID while data[3] in vaapi frame is
> VASurfaceID. I fix this line of code. Now the command above works.
> 
> Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> ---
>  libavutil/hwcontext_qsv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index d431e71eab..6539cae619 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -781,7 +781,7 @@ static int qsv_map_from(AVHWFramesContext *ctx,
>      case AV_HWDEVICE_TYPE_VAAPI:
>      {
>          mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
> -        child_data = pair->first;
> +        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;
>          break;
>      }
>  #endif
> --

Hi Wenbin,

please let me check. That line is from my original (and current) code
regarding QSV-D3D11 and I don't have issues using hwmap with QSV and 
VAAPI. I'll need to compare the commited D3D11 patch with my fork.
Probably something is missing at another other place.

Regards,
softworkz
Wenbin Chen Sept. 10, 2021, 2:19 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> James Almer
> Sent: Thursday, September 9, 2021 8:48 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: fix a bug for
> mapping qsv frame to vaapi
> 
> On 9/9/2021 6:13 AM, Wenbin Chen wrote:
> > Command below failed.
> > ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
> > -init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
> > -filter_hw_device va -c:v h264_qsv
> > -i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264
> >
> > Cause: Assign pair->first directly to data[3] in vaapi frame.
> > pair->first is *VASurfaceID while data[3] in vaapi frame is
> > VASurfaceID. I fix this line of code. Now the command above works.
> >
> > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> > ---
> >   libavutil/hwcontext_qsv.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > index d431e71eab..6539cae619 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -781,7 +781,7 @@ static int qsv_map_from(AVHWFramesContext *ctx,
> >       case AV_HWDEVICE_TYPE_VAAPI:
> >       {
> >           mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
> > -        child_data = pair->first;
> > +        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;
> 
> You can probably remove the intptr_t casting.

If intptr_t is removed, it will report compile warning
"cast to pointer from integer of different size"
 
> 
> Also, shouldn't this same fix be done for all three child device types
> used in this function? Which for that matter, child_data seems to be set
> for a d3d11va child device, but then never used.

Dxva and d3d11 frames store the pointer to surface while vaapi frames store surface, 
so this part of code for dxva and d3d11va should be correct.

D3d11 and dxva share dxva codec, the child_data is used in this part of code.
> 
> >           break;
> >       }
> >   #endif
> >
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Xiang, Haihao Sept. 12, 2021, 1:57 p.m. UTC | #4
On Fri, 2021-09-10 at 02:19 +0000, Chen, Wenbin wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > James Almer
> > Sent: Thursday, September 9, 2021 8:48 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: fix a bug for
> > mapping qsv frame to vaapi
> > 
> > On 9/9/2021 6:13 AM, Wenbin Chen wrote:
> > > Command below failed.
> > > ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
> > > -init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
> > > -filter_hw_device va -c:v h264_qsv
> > > -i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264
> > > 
> > > Cause: Assign pair->first directly to data[3] in vaapi frame.
> > > pair->first is *VASurfaceID while data[3] in vaapi frame is
> > > VASurfaceID. I fix this line of code. Now the command above works.
> > > 
> > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> > > ---
> > >   libavutil/hwcontext_qsv.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > > index d431e71eab..6539cae619 100644
> > > --- a/libavutil/hwcontext_qsv.c
> > > +++ b/libavutil/hwcontext_qsv.c
> > > @@ -781,7 +781,7 @@ static int qsv_map_from(AVHWFramesContext *ctx,
> > >       case AV_HWDEVICE_TYPE_VAAPI:
> > >       {
> > >           mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
> > > -        child_data = pair->first;
> > > +        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;
> > 
> > You can probably remove the intptr_t casting.
> 
> If intptr_t is removed, it will report compile warning
> "cast to pointer from integer of different size"

How about to add a comment here? If so, others can understand why this casting
is needed.

Thanks
Haihao


>  
> > 
> > Also, shouldn't this same fix be done for all three child device types
> > used in this function? Which for that matter, child_data seems to be set
> > for a d3d11va child device, but then never used.
> 
> Dxva and d3d11 frames store the pointer to surface while vaapi frames store
> surface, 
> so this part of code for dxva and d3d11va should be correct.
> 
> D3d11 and dxva share dxva codec, the child_data is used in this part of code.
> > 
> > >           break;
> > >       }
> > >   #endif
> > > 
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Wenbin Chen Sept. 13, 2021, 5:49 a.m. UTC | #5
> On Fri, 2021-09-10 at 02:19 +0000, Chen, Wenbin wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > James Almer
> > > Sent: Thursday, September 9, 2021 8:48 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_qsv: fix a bug
> for
> > > mapping qsv frame to vaapi
> > >
> > > On 9/9/2021 6:13 AM, Wenbin Chen wrote:
> > > > Command below failed.
> > > > ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128
> > > > -init_hw_device qsv=qs@va -hwaccel qsv -hwaccel_device qs
> > > > -filter_hw_device va -c:v h264_qsv
> > > > -i 1080P.264 -vf "hwmap,format=vaapi" -c:v h264_vaapi output.264
> > > >
> > > > Cause: Assign pair->first directly to data[3] in vaapi frame.
> > > > pair->first is *VASurfaceID while data[3] in vaapi frame is
> > > > VASurfaceID. I fix this line of code. Now the command above works.
> > > >
> > > > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> > > > ---
> > > >   libavutil/hwcontext_qsv.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > > > index d431e71eab..6539cae619 100644
> > > > --- a/libavutil/hwcontext_qsv.c
> > > > +++ b/libavutil/hwcontext_qsv.c
> > > > @@ -781,7 +781,7 @@ static int qsv_map_from(AVHWFramesContext
> *ctx,
> > > >       case AV_HWDEVICE_TYPE_VAAPI:
> > > >       {
> > > >           mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
> > > > -        child_data = pair->first;
> > > > +        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;
> > >
> > > You can probably remove the intptr_t casting.
> >
> > If intptr_t is removed, it will report compile warning
> > "cast to pointer from integer of different size"
> 
> How about to add a comment here? If so, others can understand why this
> casting
> is needed.
> 
> Thanks
> Haihao

Ok, I will send path V2

> 
> 
> >
> > >
> > > Also, shouldn't this same fix be done for all three child device types
> > > used in this function? Which for that matter, child_data seems to be set
> > > for a d3d11va child device, but then never used.
> >
> > Dxva and d3d11 frames store the pointer to surface while vaapi frames
> store
> > surface,
> > so this part of code for dxva and d3d11va should be correct.
> >
> > D3d11 and dxva share dxva codec, the child_data is used in this part of
> code.
> > >
> > > >           break;
> > > >       }
> > > >   #endif
> > > >
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index d431e71eab..6539cae619 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -781,7 +781,7 @@  static int qsv_map_from(AVHWFramesContext *ctx,
     case AV_HWDEVICE_TYPE_VAAPI:
     {
         mfxHDLPair *pair = (mfxHDLPair*)surf->Data.MemId;
-        child_data = pair->first;
+        child_data = (uint8_t*)(intptr_t)*(VASurfaceID*)pair->first;
         break;
     }
 #endif