Re: [FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P

2020-02-01 Thread Mark Thompson
On 15/01/2020 06:55, Linjie Fu wrote:
> There is a race condition for AV_PIX_FMT_YUV420P when mapping from pix_fmt
> to fourcc, both VA_FOURCC_I420 and VA_FOURCC_YV12 could be found by pix_fmt.

The title and this comment are very confusing.  As far as I can see this has 
nothing to do with a race condition, since only one thread is ever involved.

> Currently, vaapi_get_image_format will go through the query results of
> pix_fmt and returned the first matched result according to the declared
> order in driver.This may leads to a wrong image_format.
> 
> Modify to find image_format via fourcc.
> 
> Fix vaapi CSC to I420:
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo
> -pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf
> 'format=nv12,hwupload,scale_vaapi=format=yuv420p,hwdownload,format=yuv420p'
> -f rawvideo -vsync passthrough -vframes 10 -y aa.yuv
> 
> Reviewed-by: Zhong Li 
> Signed-off-by: Linjie Fu 
> ---
>  libavutil/hwcontext_vaapi.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index cd215a0..199f425 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -63,6 +63,7 @@ typedef struct VAAPIDevicePriv {
>  typedef struct VAAPISurfaceFormat {
>  enum AVPixelFormat pix_fmt;
>  VAImageFormat image_format;
> +uint32_t fourcc;
>  } VAAPISurfaceFormat;
>  
>  typedef struct VAAPIDeviceContext {
> @@ -178,15 +179,21 @@ static int vaapi_get_image_format(AVHWDeviceContext 
> *hwdev,
>VAImageFormat **image_format)
>  {
>  VAAPIDeviceContext *ctx = hwdev->internal->priv;
> +VAAPIFormatDescriptor *desc;
>  int i;
>  
> +desc = vaapi_format_from_pix_fmt(pix_fmt);

This would now only find the first entry in the map, so YV12 and YV16 could 
never work, nor could the IYUV alias for old drivers.

The point of this function is that it finds the first matching format supported 
by the driver being used, but I can see that that goes wrong when a driver 
declares support for the same format under multiple different names which are 
not interchangable.

What conflicting formats are actually being advertised in the broken case?

> +if (!desc || !image_format)
> +goto fail;
> +
>  for (i = 0; i < ctx->nb_formats; i++) {
> -if (ctx->formats[i].pix_fmt == pix_fmt) {
> -if (image_format)
> -*image_format = >formats[i].image_format;
> +if (ctx->formats[i].fourcc == desc->fourcc) {
> +*image_format = >formats[i].image_format;
>  return 0;
>  }
>  }
> +
> +fail:
>  return AVERROR(EINVAL);
>  }
>  
> @@ -375,6 +382,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
>  av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> %s.\n",
> fourcc, av_get_pix_fmt_name(pix_fmt));
>  ctx->formats[ctx->nb_formats].pix_fmt  = pix_fmt;
> +ctx->formats[ctx->nb_formats].fourcc   = fourcc;
>  ctx->formats[ctx->nb_formats].image_format = image_list[i];
>  ++ctx->nb_formats;
>  }
> 

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P

2020-01-14 Thread Linjie Fu
There is a race condition for AV_PIX_FMT_YUV420P when mapping from pix_fmt
to fourcc, both VA_FOURCC_I420 and VA_FOURCC_YV12 could be found by pix_fmt.

Currently, vaapi_get_image_format will go through the query results of
pix_fmt and returned the first matched result according to the declared
order in driver.This may leads to a wrong image_format.

Modify to find image_format via fourcc.

Fix vaapi CSC to I420:
ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo
-pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf
'format=nv12,hwupload,scale_vaapi=format=yuv420p,hwdownload,format=yuv420p'
-f rawvideo -vsync passthrough -vframes 10 -y aa.yuv

Reviewed-by: Zhong Li 
Signed-off-by: Linjie Fu 
---
 libavutil/hwcontext_vaapi.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index cd215a0..199f425 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -63,6 +63,7 @@ typedef struct VAAPIDevicePriv {
 typedef struct VAAPISurfaceFormat {
 enum AVPixelFormat pix_fmt;
 VAImageFormat image_format;
+uint32_t fourcc;
 } VAAPISurfaceFormat;
 
 typedef struct VAAPIDeviceContext {
@@ -178,15 +179,21 @@ static int vaapi_get_image_format(AVHWDeviceContext 
*hwdev,
   VAImageFormat **image_format)
 {
 VAAPIDeviceContext *ctx = hwdev->internal->priv;
+VAAPIFormatDescriptor *desc;
 int i;
 
+desc = vaapi_format_from_pix_fmt(pix_fmt);
+if (!desc || !image_format)
+goto fail;
+
 for (i = 0; i < ctx->nb_formats; i++) {
-if (ctx->formats[i].pix_fmt == pix_fmt) {
-if (image_format)
-*image_format = >formats[i].image_format;
+if (ctx->formats[i].fourcc == desc->fourcc) {
+*image_format = >formats[i].image_format;
 return 0;
 }
 }
+
+fail:
 return AVERROR(EINVAL);
 }
 
@@ -375,6 +382,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
 av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> %s.\n",
fourcc, av_get_pix_fmt_name(pix_fmt));
 ctx->formats[ctx->nb_formats].pix_fmt  = pix_fmt;
+ctx->formats[ctx->nb_formats].fourcc   = fourcc;
 ctx->formats[ctx->nb_formats].image_format = image_list[i];
 ++ctx->nb_formats;
 }
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P

2019-08-14 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Fu, Linjie
> Sent: Wednesday, August 14, 2019 9:59 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race
> map for YUV420P
> 
> > -Original Message-
> > From: Fu, Linjie
> > Sent: Thursday, August 1, 2019 12:45
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P
> >
> > There is a race condition for AV_PIX_FMT_YUV420P when mapping from
> > pix_fmt to fourcc, both VA_FOURCC_I420 and VA_FOURCC_YV12 could be
> > find by pix_fmt.

find -> found

> > Currently, vaapi_get_image_format will go through the query results of
> > pix_fmt and returned the first matched result according to the
> > declared order in driver.This may leads to a wrong image_format.
> >
> > Modify to find image_format via fourcc.
> >
> > Fix vaapi CSC to I420:
> > ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo
> > -pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf
> >
> 'format=nv12,hwupload,scale_vaapi=format=yuv420p,hwdownload,format=
> > yuv420p'
> > -f rawvideo -vsync passthrough -vframes 10 -y aa.yuv
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >  libavutil/hwcontext_vaapi.c | 14 +++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> > index cf11764..64f14de 100644
> > --- a/libavutil/hwcontext_vaapi.c
> > +++ b/libavutil/hwcontext_vaapi.c
> > @@ -63,6 +63,7 @@ typedef struct VAAPIDevicePriv {  typedef struct
> > VAAPISurfaceFormat {
> >  enum AVPixelFormat pix_fmt;
> >  VAImageFormat image_format;
> > +unsigned int fourcc;
> >  } VAAPISurfaceFormat;
> >
> >  typedef struct VAAPIDeviceContext {
> > @@ -171,15 +172,21 @@ static int
> > vaapi_get_image_format(AVHWDeviceContext *hwdev,
> >VAImageFormat
> **image_format)  {
> >  VAAPIDeviceContext *ctx = hwdev->internal->priv;
> > +VAAPIFormatDescriptor *desc;
> >  int i;
> >
> > +desc = vaapi_format_from_pix_fmt(pix_fmt);
> > +if (!desc || !image_format)
> > +goto fail;
> > +
> >  for (i = 0; i < ctx->nb_formats; i++) {
> > -if (ctx->formats[i].pix_fmt == pix_fmt) {
> > -if (image_format)
> > -*image_format = >formats[i].image_format;
> > +if (ctx->formats[i].fourcc == desc->fourcc) {
> > +*image_format = >formats[i].image_format;
> >  return 0;
> >  }
> >  }
> > +
> > +fail:
> >  return AVERROR(EINVAL);
> >  }
> >
> > @@ -368,6 +375,7 @@ static int vaapi_device_init(AVHWDeviceContext
> > *hwdev)
> >  av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> %s.\n",
> > fourcc, av_get_pix_fmt_name(pix_fmt));
> >  ctx->formats[ctx->nb_formats].pix_fmt  = pix_fmt;
> > +ctx->formats[ctx->nb_formats].fourcc   = fourcc;
> >  ctx->formats[ctx->nb_formats].image_format =
> image_list[i];
> >  ++ctx->nb_formats;
> >  }
> > --
> > 2.7.4
> A gentle ping.

Patch LGTM

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P

2019-08-13 Thread Fu, Linjie
> -Original Message-
> From: Fu, Linjie
> Sent: Thursday, August 1, 2019 12:45
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie 
> Subject: [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P
> 
> There is a race condition for AV_PIX_FMT_YUV420P when mapping from
> pix_fmt
> to fourcc, both VA_FOURCC_I420 and VA_FOURCC_YV12 could be find by
> pix_fmt.
> 
> Currently, vaapi_get_image_format will go through the query results of
> pix_fmt and returned the first matched result according to the declared
> order in driver.This may leads to a wrong image_format.
> 
> Modify to find image_format via fourcc.
> 
> Fix vaapi CSC to I420:
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo
> -pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf
> 'format=nv12,hwupload,scale_vaapi=format=yuv420p,hwdownload,format=
> yuv420p'
> -f rawvideo -vsync passthrough -vframes 10 -y aa.yuv
> 
> Signed-off-by: Linjie Fu 
> ---
>  libavutil/hwcontext_vaapi.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index cf11764..64f14de 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -63,6 +63,7 @@ typedef struct VAAPIDevicePriv {
>  typedef struct VAAPISurfaceFormat {
>  enum AVPixelFormat pix_fmt;
>  VAImageFormat image_format;
> +unsigned int fourcc;
>  } VAAPISurfaceFormat;
> 
>  typedef struct VAAPIDeviceContext {
> @@ -171,15 +172,21 @@ static int
> vaapi_get_image_format(AVHWDeviceContext *hwdev,
>VAImageFormat **image_format)
>  {
>  VAAPIDeviceContext *ctx = hwdev->internal->priv;
> +VAAPIFormatDescriptor *desc;
>  int i;
> 
> +desc = vaapi_format_from_pix_fmt(pix_fmt);
> +if (!desc || !image_format)
> +goto fail;
> +
>  for (i = 0; i < ctx->nb_formats; i++) {
> -if (ctx->formats[i].pix_fmt == pix_fmt) {
> -if (image_format)
> -*image_format = >formats[i].image_format;
> +if (ctx->formats[i].fourcc == desc->fourcc) {
> +*image_format = >formats[i].image_format;
>  return 0;
>  }
>  }
> +
> +fail:
>  return AVERROR(EINVAL);
>  }
> 
> @@ -368,6 +375,7 @@ static int vaapi_device_init(AVHWDeviceContext
> *hwdev)
>  av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> %s.\n",
> fourcc, av_get_pix_fmt_name(pix_fmt));
>  ctx->formats[ctx->nb_formats].pix_fmt  = pix_fmt;
> +ctx->formats[ctx->nb_formats].fourcc   = fourcc;
>  ctx->formats[ctx->nb_formats].image_format = image_list[i];
>  ++ctx->nb_formats;
>  }
> --
> 2.7.4
A gentle ping.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] lavu/hwcontext_vaapi: cope with race map for YUV420P

2019-07-31 Thread Linjie Fu
There is a race condition for AV_PIX_FMT_YUV420P when mapping from pix_fmt
to fourcc, both VA_FOURCC_I420 and VA_FOURCC_YV12 could be find by pix_fmt.

Currently, vaapi_get_image_format will go through the query results of
pix_fmt and returned the first matched result according to the declared
order in driver.This may leads to a wrong image_format.

Modify to find image_format via fourcc.

Fix vaapi CSC to I420:
ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo
-pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf
'format=nv12,hwupload,scale_vaapi=format=yuv420p,hwdownload,format=yuv420p'
-f rawvideo -vsync passthrough -vframes 10 -y aa.yuv

Signed-off-by: Linjie Fu 
---
 libavutil/hwcontext_vaapi.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index cf11764..64f14de 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -63,6 +63,7 @@ typedef struct VAAPIDevicePriv {
 typedef struct VAAPISurfaceFormat {
 enum AVPixelFormat pix_fmt;
 VAImageFormat image_format;
+unsigned int fourcc;
 } VAAPISurfaceFormat;
 
 typedef struct VAAPIDeviceContext {
@@ -171,15 +172,21 @@ static int vaapi_get_image_format(AVHWDeviceContext 
*hwdev,
   VAImageFormat **image_format)
 {
 VAAPIDeviceContext *ctx = hwdev->internal->priv;
+VAAPIFormatDescriptor *desc;
 int i;
 
+desc = vaapi_format_from_pix_fmt(pix_fmt);
+if (!desc || !image_format)
+goto fail;
+
 for (i = 0; i < ctx->nb_formats; i++) {
-if (ctx->formats[i].pix_fmt == pix_fmt) {
-if (image_format)
-*image_format = >formats[i].image_format;
+if (ctx->formats[i].fourcc == desc->fourcc) {
+*image_format = >formats[i].image_format;
 return 0;
 }
 }
+
+fail:
 return AVERROR(EINVAL);
 }
 
@@ -368,6 +375,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
 av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> %s.\n",
fourcc, av_get_pix_fmt_name(pix_fmt));
 ctx->formats[ctx->nb_formats].pix_fmt  = pix_fmt;
+ctx->formats[ctx->nb_formats].fourcc   = fourcc;
 ctx->formats[ctx->nb_formats].image_format = image_list[i];
 ++ctx->nb_formats;
 }
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".