Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Xiang, Haihao" <haihao.xiang-at-intel.com@ffmpeg.org>
To: "anton@khirnov.net" <anton@khirnov.net>,
	"ffmpeg-devel@ffmpeg.org" <ffmpeg-devel@ffmpeg.org>
Cc: "Galin, Artem" <artem.galin@intel.com>
Subject: Re: [FFmpeg-devel] [PATCH v10 10/13] lavu/hwcontext_qsv: make qsv hwdevice works with oneVPL
Date: Fri, 22 Jul 2022 02:54:51 +0000
Message-ID: <23fa98e9bd1515b01231d9c6741b27bbe930fc5e.camel@intel.com> (raw)
In-Reply-To: <DM8P223MB036596141C7D484B48048FDCBA919@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>

On Thu, 2022-07-21 at 20:30 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Tuesday, July 19, 2022 9:19 AM
> > To: anton@khirnov.net; ffmpeg-devel@ffmpeg.org
> > Cc: Galin, Artem <artem.galin@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

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

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

  reply	other threads:[~2022-07-22  2:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-12  6:27 [FFmpeg-devel] [PATCH v10 00/10] make QSV works with the Intel's oneVPL Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 01/13] configure: ensure --enable-libmfx uses libmfx 1.x Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 02/13] configure: fix the check for MFX_CODEC_VP9 Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 03/13] qsv: remove mfx/ prefix from mfx headers Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 04/13] qsv: load user plugin for MFX_VERSION < 2.0 Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 05/13] qsv: build audio related code when " Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 06/13] qsvenc: support multi-frame encode " Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 07/13] qsvenc: support MFX_RATECONTROL_LA_EXT " Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 08/13] qsv: support OPAQUE memory " Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 09/13] lavu/hwcontext_qsv: add loader field to AVQSVDeviceContext Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 10/13] lavu/hwcontext_qsv: make qsv hwdevice works with oneVPL Xiang, Haihao
2022-07-18 13:02   ` Anton Khirnov
2022-07-19  7:18     ` Xiang, Haihao
2022-07-20  8:42       ` Xiang, Haihao
2022-07-21 20:30       ` Soft Works
2022-07-22  2:54         ` Xiang, Haihao [this message]
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 11/13] lavc/qsv: create mfx session using oneVPL for decoding/encoding Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 12/13] lavfi/qsv: create mfx session using oneVPL for qsv filters Xiang, Haihao
2022-07-12  6:27 ` [FFmpeg-devel] [PATCH v10 13/13] configure: add --enable-libvpl option Xiang, Haihao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23fa98e9bd1515b01231d9c6741b27bbe930fc5e.camel@intel.com \
    --to=haihao.xiang-at-intel.com@ffmpeg.org \
    --cc=anton@khirnov.net \
    --cc=artem.galin@intel.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git