From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 03BCA40BAE for ; Tue, 28 Dec 2021 12:53:41 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9E94568AF3B; Tue, 28 Dec 2021 14:53:39 +0200 (EET) Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 084AB68A787 for ; Tue, 28 Dec 2021 14:53:34 +0200 (EET) Received: by mail-wr1-f49.google.com with SMTP id a9so38138962wrr.8 for ; Tue, 28 Dec 2021 04:53:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=SjXcXPMPH5HLbbBHFniL4mhpSVlfq55YDjxZzyg5dn0=; b=zElHpTcrlBu4EsBBHWfu1D5WSDKJn0u/u8nOPbM20t1aKJEqL5Jced1Ij8rPylPkkU J2YC7iimD00sQYOcy/VaCcG1MoXSpqGMPX3G7v+idWiim1hpPAvcvDIbMHyMZ8K3zuuI OBK9vo/eUcdULtYVFGSpmUGeBLwCpfVeFUCO31jXypM9Nt6jxSwCW1W5ph821MzCcCaM ILlKiqkBeh4VcsUhxm8y48if1JP9HsNAdHNDeecyl1EDFEB49M/P2ZJoyAif+fbve6br Eo40eAiK91/HKZw4hQTi9NioLzM+t1YidX5k/uHlHGak4RjUZPID7UWYzcgDeb39ZFEM eRVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=SjXcXPMPH5HLbbBHFniL4mhpSVlfq55YDjxZzyg5dn0=; b=cmR0xDxgIRlg1vlQqRYY0l6sFq5Z54x7N7NVKnJTLeMAJFv7h4m44Q6z24lffGxGj1 4uEBKELvcl+pFLr+ETId/oRTcd56YP9DCEIpw/KZja259ezJ757saJ/tlik0y7lElCqa p82DqLwYZ0ODnKwxQqOEq4+ghcd3PPsfV4ZliY3fu5tx5ybaVvSxEcmznKs8zEKsPaXF h9BOfWqp5HGVoioPUpq9Ri9Nv066GA3DpbXFHB2qBacaNhTDD12AcuXhnwlTlMjCJfqQ GaB9g7TUwhb45azoGq2xEFQiborCqYgyill8PUhUYmKjreKPfTqm5IeuFZwKIpGYHOi7 lm4g== X-Gm-Message-State: AOAM533z6g2OzHcgdZS3oEKyGe99xlzPa6iAc2U0xaCm6IxhsCK5onEG fu3oGhxnkUwEZuPNUxM1vxdptG/JJonZxw== X-Google-Smtp-Source: ABdhPJwb0r74AjotyWOyp3g0AzfgI6240hq1T7ah/YN3TmFH2G/mVwDYJwlJh3KHTkqQTK3ekEkppQ== X-Received: by 2002:adf:dd11:: with SMTP id a17mr16283946wrm.174.1640696013422; Tue, 28 Dec 2021 04:53:33 -0800 (PST) Received: from [192.168.0.3] (cpc91224-cmbg18-2-0-cust201.5-4.cable.virginm.net. [81.106.228.202]) by smtp.gmail.com with ESMTPSA id l12sm23180422wmq.2.2021.12.28.04.53.32 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Dec 2021 04:53:32 -0800 (PST) Message-ID: Date: Tue, 28 Dec 2021 12:53:32 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20211116081623.3081766-1-wenbin.chen@intel.com> <20211116081623.3081766-3-wenbin.chen@intel.com> <92735cd0-179b-cbf6-b191-e440be779b66@jkqxz.net> <96dc30bc-2ff0-009b-713c-b41c248d6b6b@jkqxz.net> From: Mark Thompson In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug for mapping qsv frame to opencl X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 28/12/2021 01:17, Soft Works wrote: >> -----Original Message----- >> From: ffmpeg-devel On Behalf Of Mark >> Thompson >> Sent: Tuesday, December 28, 2021 12:46 AM >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug >> for mapping qsv frame to opencl >> >> On 27/12/2021 20:31, Soft Works wrote:>> -----Original Message----- >>>> From: ffmpeg-devel On Behalf Of Mark >>>> Thompson >>>> Sent: Monday, December 27, 2021 7:51 PM >>>> To: ffmpeg-devel@ffmpeg.org >>>> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a >> bug >>>> for mapping qsv frame to opencl >>>> >>>> On 16/11/2021 08:16, Wenbin Chen wrote: >>>>> From: nyanmisaka >>>>> >>>>> mfxHDLPair was added to qsv, so modify qsv->opencl map function as well. >>>>> Now the following commandline works: >>>>> >>>>> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128 \ >>>>> -init_hw_device qsv=qs@va -init_hw_device opencl=ocl@va -filter_hw_device >>>> ocl \ >>>>> -hwaccel qsv -hwaccel_output_format qsv -hwaccel_device qs -c:v h264_qsv >> \ >>>>> -i input.264 -vf >> "hwmap=derive_device=opencl,format=opencl,avgblur_opencl, >>>> \ >>>>> hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32,format=qsv" \ >>>>> -c:v h264_qsv output.264 >>>>> >>>>> Signed-off-by: nyanmisaka >>>>> Signed-off-by: Wenbin Chen >>>>> --- >>>>> libavutil/hwcontext_opencl.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c >>>>> index 26a3a24593..4b6e74ff6f 100644 >>>>> --- a/libavutil/hwcontext_opencl.c >>>>> +++ b/libavutil/hwcontext_opencl.c >>>>> @@ -2249,7 +2249,8 @@ static int opencl_map_from_qsv(AVHWFramesContext >>>> *dst_fc, AVFrame *dst, >>>>> #if CONFIG_LIBMFX >>>>> if (src->format == AV_PIX_FMT_QSV) { >>>>> mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src- >>> data[3]; >>>>> - va_surface = *(VASurfaceID*)mfx_surface->Data.MemId; >>>>> + mfxHDLPair *pair = (mfxHDLPair*)mfx_surface->Data.MemId; >>>>> + va_surface = *(VASurfaceID*)pair->first; >>>>> } else >>>>> #endif >>>>> if (src->format == AV_PIX_FMT_VAAPI) { >>>> >>>> Since these frames can be user-supplied, this implies that the user-facing >>>> API/ABI for AV_PIX_FMT_QSV has changed. >>>> >>>> It looks like this was broken by using HDLPairs when D3D11 was introduced, >>>> which silently changed the existing API for DXVA2 and VAAPI as well. >>>> >>>> Could someone related to that please document it properly (clearly not all >>>> possible valid mfxFrameSurface1s are allowed), and note in APIchanges when >>>> the API change happened? >>> >>> Hi Mark, >>> >>> QSV contexts always need to be backed by a child context, which can be >> DXVA2, >>> D3D11VA or VAAPI. You can create a QSV context either by deriving from one >> of >>> those contexts or when create a new QSV context, it automatically creates >> an >>> appropriate child context - either implicitly (auto mode) or explicitly, >> like >>> the ffmpeg implementation does in most cases. >> >> ... or by using the one the user supplies when they create it. >> >>> When working with "user-supplied" frames on Linux, you need to create a >> VAAPI >>> context with those frames and derive a QSV context from that context. >>> >>> There is no way to create or supply QSV frames directly. >> >> ??? The ability for the user to set up their own version of these things is >> literally the whole point of the split alloc/init API. >> >> >> // Some user stuff involving libmfx - has a D3D or VAAPI backing, but this >> code doesn't need to care about it. >> >> // It has a session and creates some surfaces to use with MemId filled >> compatible with ffmpeg. >> user_session = ...; >> user_surfaces = ...; >> >> // No ffmpeg involved before this, now we want to pass these surfaces we've >> got into ffmpeg. >> >> // Create a device context using the existing session. >> >> mfx_ctx = av_hwdevice_ctx_alloc(MFX); >> >> dc = mfx_ctx->data; >> mfx_dc = dc->hwctx; >> mfx_dc->session = user_session; >> >> av_hwdevice_ctx_init(mfx_ctx); >> >> // Create a frames context out of the surfaces we've got. >> >> mfx_frames = av_hwframes_ctx_alloc(mfx_ctx); >> >> fc = mfx_frames->data; >> fc.pool = user_surfaces.allocator; >> fc.width = user_surfaces.width; >> // etc. >> >> mfx_fc = fc->hwctx; >> mfx_fc.surfaces = user_surfaces.array; >> mfx_fc.nb_surfaces = user_surfaces.count; >> mfx_fc.frame_type = user_surfaces.memtype; >> >> av_hwframe_ctx_init(frames); >> >> // Do stuff with frames. > > I wouldn't consider an mfxSession as an entity that could or should be > shared between implementations. IMO, this is not a valid use case. > A consumer of the mfx API needs to make certain choices regarding > the usage of the API, one of which is the way how frames are allocated > and managed. > This is not something that is meant to be shared between implementations. > Even inside ffmpeg, we don't use a single mfx session. We use separate > sessions for decoding, encoding and filtering that are joined together > via MFXJoinSession. > When an (ffmpeg-)API consumer is creating its own MFX session and its > own frame allocator implementation, it shouldn't be and allowed > scenario to create an ffmpeg hw context using this session. The user is also aware of the thread safety rules. I was giving the example above entirely serially to avoid that, but if they were using libmfx elsewhere in parallel with the above then indeed they would need a bit more care (create another session, some sort of locking to ensure serialisation) when passing it to ffmpeg. > This shouldn't be considered a public API of ffmpeg because it doesn't > make sense to share an mfx session like this. So, to clarify, your opinion is that none of hwcontext_qsv.h should be public API? If it were actually removed (not visible in installed headers) then I agree that would fix the problem. > Besides that, is there any ffmpeg documentation indicating that > the memId field of mfxFrameSurface1 could be casted to VASurfaceId? It is user-visible API, and it matched the behaviour of the libmfx examples for D3D9 (identical pointer to IDirect3DSurface9) and VAAPI (a compatible pointer to a structure with the VASurfaceID as the first member) so doing the obvious thing worked. - Mark _______________________________________________ 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".