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

[Xiang, Haihao]

On Thu, 2022-07-21 at 20:30 +0000, Soft Works wrote:
> > -----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)?

Agree, and this is fixed in the new version.

Thanks
Haihao