Re: [libav-devel] [PATCH 1/2] hwcontext_vaapi: Add driver quirks to the hwdevice

2016-06-17 Thread Mark Thompson 
On 17/06/16 09:46, Anton Khirnov wrote:
> Quoting Mark Thompson (2016-06-12 19:03:54)
>> The driver being used is detected inside av_hwdevice_ctx_init(), and
>> the quirks field then set from a table of known drivers.
>>
>> Also adds the Intel i965 driver quirk (it does not destroy parameter
>> buffers used in a call to vaRenderPicture()) and detects that driver
>> to set it.
>> ---
>> Another approach...
>>
>> Moving the quirks into the device properties will allow us to fix the broken 
>> behaviour in the decode hwaccel as well, once the hwdevice is available 
>> there 
>> ().
>>
>> I am getting a suitable AMD card (i.e. something with non-i965 VAAPI) next 
>> week to test this; I won't commit anything before trying that.
>>
>>  libavutil/hwcontext_vaapi.c | 34 ++
>>  libavutil/hwcontext_vaapi.h | 15 +++
>>  2 files changed, 49 insertions(+)
>>
>> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
>> index 4563e14..642763f 100644
>> --- a/libavutil/hwcontext_vaapi.c
>> +++ b/libavutil/hwcontext_vaapi.c
>> @@ -281,6 +281,18 @@ fail:
>>  return err;
>>  }
>>
>> +static struct {
> 
> static const?

Yep, thanks.

>> +const char *friendly_name;
>> +const char *match_string;
>> +unsigned int quirks;
>> +} vaapi_driver_quirks_table[] = {
>> +{
>> +"Intel i965 (Quick Sync)",
>> +"i965",
>> +AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS,
>> +},
>> +};
>> +
>>  static int vaapi_device_init(AVHWDeviceContext *hwdev)
>>  {
>>  VAAPIDeviceContext *ctx = hwdev->internal->priv;
>> @@ -288,6 +300,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
>>  AVHWFramesConstraints *constraints = NULL;
>>  VAImageFormat *image_list = NULL;
>>  VAStatus vas;
>> +const char *vendor_string;
>>  int err, i, j, image_count;
>>  enum AVPixelFormat pix_fmt;
>>  unsigned int fourcc;
>> @@ -340,6 +353,27 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
>>  }
>>  }
>>
>> +// Detect the driver in use and set quirk flags if necessary.
>> +vendor_string = vaQueryVendorString(hwctx->display);
>> +hwctx->driver_quirks = 0;
>> +if (vendor_string) {
>> +for (i = 0; i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table); i++) {
>> +if (strstr(vendor_string,
>> +   vaapi_driver_quirks_table[i].match_string)) {
>> +av_log(hwdev, AV_LOG_VERBOSE, "Matched \"%s\" as known 
>> driver "
>> +   "\"%s\".\n", vendor_string,
>> +   vaapi_driver_quirks_table[i].friendly_name);
>> +hwctx->driver_quirks |=
>> +vaapi_driver_quirks_table[i].quirks;
>> +break;
>> +}
>> +}
>> +if (!(i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table))) {
>> +av_log(hwdev, AV_LOG_VERBOSE, "Unknown driver \"%s\", assuming "
>> +   "standard behaviour.\n", vendor_string);
>> +}
>> +}
>> +
>>  av_free(image_list);
>>  av_hwframe_constraints_free(&constraints);
>>  return 0;
>> diff --git a/libavutil/hwcontext_vaapi.h b/libavutil/hwcontext_vaapi.h
>> index 1c87f5d..fc0b56e 100644
>> --- a/libavutil/hwcontext_vaapi.h
>> +++ b/libavutil/hwcontext_vaapi.h
>> @@ -33,6 +33,15 @@
>>   * with the data pointer set to a VASurfaceID.
>>   */
>>
>> +enum {
>> +/**
>> + * The driver does not destroy parameter buffers when they are used by
>> + * vaRenderPicture().  Additional code will be required to destroy them
>> + * separately afterwards.
>> + */
>> +AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS = 0x01,
> 
> Long name is long. Maybe just
> AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS?

The long name does describe the problem completely, so it's obvious at use-time
what's going on.  I guess I don't really mind.

> nit: using (1 << n) for flags is more readable if we end up having a
> bunch of them.

Sure.

>> +};
>> +
>>  /**
>>   * VAAPI connection details.
>>   *
>> @@ -43,6 +52,12 @@ typedef struct AVVAAPIDeviceContext {
>>   * The VADisplay handle, to be filled by the user.
>>   */
>>  VADisplay display;
>> +/**
>> + * Driver quirks to apply - this is filled by av_hwdevice_ctx_init(),
>> + * with reference to a table of known drivers.  The user may need to
>> + * refer to this when performing any related operations using VAAPI.
>> + */
>> +unsigned int driver_quirks;
> 
> Perhaps we should allow the user to set this, and fall back to
> autodetection by default. That probably means we'll need a special flag
> indicating "force no quirks"

I wondered about that.  A flags field on the generic hwdevice for "[don't]
autodetect quirks" might be nicer, rather than a specific flag here meaning
"I've already filled this"?

> Also, this needs a minor bum

Re: [libav-devel] [PATCH 1/2] hwcontext_vaapi: Add driver quirks to the hwdevice

2016-06-17 Thread Anton Khirnov 
Quoting Mark Thompson (2016-06-12 19:03:54)
> The driver being used is detected inside av_hwdevice_ctx_init(), and
> the quirks field then set from a table of known drivers.
> 
> Also adds the Intel i965 driver quirk (it does not destroy parameter
> buffers used in a call to vaRenderPicture()) and detects that driver
> to set it.
> ---
> Another approach...
> 
> Moving the quirks into the device properties will allow us to fix the broken 
> behaviour in the decode hwaccel as well, once the hwdevice is available there 
> ().
> 
> I am getting a suitable AMD card (i.e. something with non-i965 VAAPI) next 
> week to test this; I won't commit anything before trying that.
> 
>  libavutil/hwcontext_vaapi.c | 34 ++
>  libavutil/hwcontext_vaapi.h | 15 +++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 4563e14..642763f 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -281,6 +281,18 @@ fail:
>  return err;
>  }
> 
> +static struct {

static const?

> +const char *friendly_name;
> +const char *match_string;
> +unsigned int quirks;
> +} vaapi_driver_quirks_table[] = {
> +{
> +"Intel i965 (Quick Sync)",
> +"i965",
> +AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS,
> +},
> +};
> +
>  static int vaapi_device_init(AVHWDeviceContext *hwdev)
>  {
>  VAAPIDeviceContext *ctx = hwdev->internal->priv;
> @@ -288,6 +300,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
>  AVHWFramesConstraints *constraints = NULL;
>  VAImageFormat *image_list = NULL;
>  VAStatus vas;
> +const char *vendor_string;
>  int err, i, j, image_count;
>  enum AVPixelFormat pix_fmt;
>  unsigned int fourcc;
> @@ -340,6 +353,27 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
>  }
>  }
> 
> +// Detect the driver in use and set quirk flags if necessary.
> +vendor_string = vaQueryVendorString(hwctx->display);
> +hwctx->driver_quirks = 0;
> +if (vendor_string) {
> +for (i = 0; i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table); i++) {
> +if (strstr(vendor_string,
> +   vaapi_driver_quirks_table[i].match_string)) {
> +av_log(hwdev, AV_LOG_VERBOSE, "Matched \"%s\" as known 
> driver "
> +   "\"%s\".\n", vendor_string,
> +   vaapi_driver_quirks_table[i].friendly_name);
> +hwctx->driver_quirks |=
> +vaapi_driver_quirks_table[i].quirks;
> +break;
> +}
> +}
> +if (!(i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table))) {
> +av_log(hwdev, AV_LOG_VERBOSE, "Unknown driver \"%s\", assuming "
> +   "standard behaviour.\n", vendor_string);
> +}
> +}
> +
>  av_free(image_list);
>  av_hwframe_constraints_free(&constraints);
>  return 0;
> diff --git a/libavutil/hwcontext_vaapi.h b/libavutil/hwcontext_vaapi.h
> index 1c87f5d..fc0b56e 100644
> --- a/libavutil/hwcontext_vaapi.h
> +++ b/libavutil/hwcontext_vaapi.h
> @@ -33,6 +33,15 @@
>   * with the data pointer set to a VASurfaceID.
>   */
> 
> +enum {
> +/**
> + * The driver does not destroy parameter buffers when they are used by
> + * vaRenderPicture().  Additional code will be required to destroy them
> + * separately afterwards.
> + */
> +AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS = 0x01,

Long name is long. Maybe just
AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS?

nit: using (1 << n) for flags is more readable if we end up having a
bunch of them.

> +};
> +
>  /**
>   * VAAPI connection details.
>   *
> @@ -43,6 +52,12 @@ typedef struct AVVAAPIDeviceContext {
>   * The VADisplay handle, to be filled by the user.
>   */
>  VADisplay display;
> +/**
> + * Driver quirks to apply - this is filled by av_hwdevice_ctx_init(),
> + * with reference to a table of known drivers.  The user may need to
> + * refer to this when performing any related operations using VAAPI.
> + */
> +unsigned int driver_quirks;

Perhaps we should allow the user to set this, and fall back to
autodetection by default. That probably means we'll need a special flag
indicating "force no quirks"

Also, this needs a minor bump and an APIchanges entry.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 1/2] hwcontext_vaapi: Add driver quirks to the hwdevice

2016-06-12 Thread Mark Thompson 
The driver being used is detected inside av_hwdevice_ctx_init(), and
the quirks field then set from a table of known drivers.

Also adds the Intel i965 driver quirk (it does not destroy parameter
buffers used in a call to vaRenderPicture()) and detects that driver
to set it.
---
Another approach...

Moving the quirks into the device properties will allow us to fix the broken 
behaviour in the decode hwaccel as well, once the hwdevice is available there 
().

I am getting a suitable AMD card (i.e. something with non-i965 VAAPI) next week 
to test this; I won't commit anything before trying that.

 libavutil/hwcontext_vaapi.c | 34 ++
 libavutil/hwcontext_vaapi.h | 15 +++
 2 files changed, 49 insertions(+)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 4563e14..642763f 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -281,6 +281,18 @@ fail:
 return err;
 }

+static struct {
+const char *friendly_name;
+const char *match_string;
+unsigned int quirks;
+} vaapi_driver_quirks_table[] = {
+{
+"Intel i965 (Quick Sync)",
+"i965",
+AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS,
+},
+};
+
 static int vaapi_device_init(AVHWDeviceContext *hwdev)
 {
 VAAPIDeviceContext *ctx = hwdev->internal->priv;
@@ -288,6 +300,7 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
 AVHWFramesConstraints *constraints = NULL;
 VAImageFormat *image_list = NULL;
 VAStatus vas;
+const char *vendor_string;
 int err, i, j, image_count;
 enum AVPixelFormat pix_fmt;
 unsigned int fourcc;
@@ -340,6 +353,27 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
 }
 }

+// Detect the driver in use and set quirk flags if necessary.
+vendor_string = vaQueryVendorString(hwctx->display);
+hwctx->driver_quirks = 0;
+if (vendor_string) {
+for (i = 0; i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table); i++) {
+if (strstr(vendor_string,
+   vaapi_driver_quirks_table[i].match_string)) {
+av_log(hwdev, AV_LOG_VERBOSE, "Matched \"%s\" as known driver "
+   "\"%s\".\n", vendor_string,
+   vaapi_driver_quirks_table[i].friendly_name);
+hwctx->driver_quirks |=
+vaapi_driver_quirks_table[i].quirks;
+break;
+}
+}
+if (!(i < FF_ARRAY_ELEMS(vaapi_driver_quirks_table))) {
+av_log(hwdev, AV_LOG_VERBOSE, "Unknown driver \"%s\", assuming "
+   "standard behaviour.\n", vendor_string);
+}
+}
+
 av_free(image_list);
 av_hwframe_constraints_free(&constraints);
 return 0;
diff --git a/libavutil/hwcontext_vaapi.h b/libavutil/hwcontext_vaapi.h
index 1c87f5d..fc0b56e 100644
--- a/libavutil/hwcontext_vaapi.h
+++ b/libavutil/hwcontext_vaapi.h
@@ -33,6 +33,15 @@
  * with the data pointer set to a VASurfaceID.
  */

+enum {
+/**
+ * The driver does not destroy parameter buffers when they are used by
+ * vaRenderPicture().  Additional code will be required to destroy them
+ * separately afterwards.
+ */
+AV_VAAPI_DRIVER_QUIRK_RENDER_DOES_NOT_DESTROY_PARAM_BUFFERS = 0x01,
+};
+
 /**
  * VAAPI connection details.
  *
@@ -43,6 +52,12 @@ typedef struct AVVAAPIDeviceContext {
  * The VADisplay handle, to be filled by the user.
  */
 VADisplay display;
+/**
+ * Driver quirks to apply - this is filled by av_hwdevice_ctx_init(),
+ * with reference to a table of known drivers.  The user may need to
+ * refer to this when performing any related operations using VAAPI.
+ */
+unsigned int driver_quirks;
 } AVVAAPIDeviceContext;

 /**
-- 
2.8.1

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