[FFmpeg-devel] [PATCH v10 10/13] lavu/hwcontext_qsv: make qsv hwdevice works with oneVPL

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Tuesday, July 19, 2022 9:19 AM
> To: anton at khirnov.net; ffmpeg-devel at ffmpeg.org
> Cc: Galin, Artem <artem.galin at intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH v10 10/13] lavu/hwcontext_qsv:
> make qsv hwdevice works with oneVPL
> 
> On Mon, 2022-07-18 at 15:02 +0200, Anton Khirnov wrote:
> > Quoting Xiang, Haihao (2022-07-12 08:27:32)
> > > +static int qsv_va_update_config(void *ctx, mfxHDL handle,
> mfxConfig cfg)
> > > +{
> > > +#if CONFIG_VAAPI
> > > +#if VA_CHECK_VERSION(1, 5, 0)
> > > +#define LOCAL_VADISPLAYPCIID VADisplayPCIID
> > > +#else
> > > +#define LOCAL_VADISPLAYPCIID 21
> > > +#endif
> > > +    mfxStatus sts;
> > > +    VADisplay dpy = handle;
> > > +    VAStatus vas;
> > > +    VADisplayAttribute attr = {
> > > +        .type = LOCAL_VADISPLAYPCIID
> > > +    };
> > > +    mfxVariant impl_value;
> > > +
> > > +    vas = vaGetDisplayAttributes(dpy, &attr, 1);
> > > +    if (vas == VA_STATUS_SUCCESS && attr.flags !=
> > > VA_DISPLAY_ATTRIB_NOT_SUPPORTED) {
> > > +        impl_value.Type = MFX_VARIANT_TYPE_U16;
> > > +        impl_value.Data.U16 = (attr.value & 0xFFFF);
> > > +        sts = MFXSetConfigFilterProperty(cfg,
> > > +                                         (const mfxU8
> > > *)"mfxExtendedDeviceId.DeviceID", impl_value);
> > > +        if (sts != MFX_ERR_NONE) {
> > > +            av_log(ctx, AV_LOG_ERROR, "Error adding a MFX
> configuration"
> > > +                   "DeviceID property: %d.\n", sts);
> > > +            goto fail;
> > > +        }
> > > +    } else
> > > +        av_log(ctx, AV_LOG_WARNING, "Cannot get device id from
> the driver,
> > > the default "
> > > +               "MFX implementation will be loaded for this
> device. Please
> > > consider to "
> > > +               "upgrade the driver to support VAAPI 1.5.0. \n");
> >
> > I would still prefer to fail here. The user requested a specific
> device,
> > disregarding that request is evil.
> 
> Thanks for the comment. There is only one available device for most
> users, so
> the default one and the given one from user should be the same,
> otherwise it
> won't work. I don't want to make them in trouble if they don't have a
> driver to
> support the new interface. However I agree with you it is a little
> evil to
> ignore the request. I'll update the patch to return error here.

I'm not a fan of that kind of automagic behavior. Quick success 
experiences are surely desirable in general, but we also need to 
consider the effects of such behavior - in this case, that would
mean: It doesn't really matter what a user specifies for the parameter,
because it will always work anyway.

In turn, users may start to think that their wrong command with the 
wrong ID would be right, and then, in a subsequent command
use that wrong ID again in different context, where it might fail,
while in turn maximizing confusion.

When it is possible to internally retrieve potentially valid 
values, why not output something useful like: "XXID failed, you
might want to try A, B or C" (or similar)?

Kind regards,
softworkz