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
>> (<https://git.libav.org/?p=libav.git;a=blob;f=libavcodec/vaapi.c#l184>).
>>
>> 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 bump and an APIchanges entry.
Yes. The patch here was really intended as a placeholder for doing something
sensible that people could express general sentiments on while I wait for an
opportunity for more testing. Hopefully I will be able to test AMD/gallium kit
and sort this out tomorrow.
Thanks,
- Mark
(Relately, looks like we will soon have another target for encode too:
<https://lists.freedesktop.org/archives/mesa-dev/2016-June/120502.html>.)
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
