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 363E640C8F for ; Wed, 29 Dec 2021 23:04:29 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 04B8868A778; Thu, 30 Dec 2021 01:04:27 +0200 (EET) Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1A5B568A0EA for ; Thu, 30 Dec 2021 01:04:20 +0200 (EET) Received: by mail-wr1-f50.google.com with SMTP id q16so47053674wrg.7 for ; Wed, 29 Dec 2021 15:04:20 -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=xyeBlAj8LmoV4qquhwcIVR8PwMNvOOjygju44HT89u0=; b=kn2f8xFjTR1/Q3z4eUJlyjrbvjVxX1W0h1AzJw3VfiS7YM31dH/OUFpbR4SfvePqFc hca8tpMSIVLOBM3I1TRKQivbaK73tY+HjFWLR7JtPYlfpQzddxi+MCvm8L4BJOy0xns2 JuqqlFL2rdnUKnXLDysQSYuzyHaPPxZ4/x4MHHxzAM14TRQvtkOySf/0kWseOvTY4lNA JU82cW1ydd1mEzbP+jFDhbRjorl2CITqGphwIbeSx1KD8dEixQoNmp2DVeHl2cUI+1po Fvkdl76A4z8YbVghoaGDzu74WyJVPONw4Ay+8b302Gi1A2t6cWVcGK0dBLWUvEV84qb1 vIoA== 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=xyeBlAj8LmoV4qquhwcIVR8PwMNvOOjygju44HT89u0=; b=PZe3Cvl295gZNE+mtDfNJFDRDhoYC3zT5h7HkRxtwnC/XP39Zyxvm8+lOZ19kK1qBP ExuTVX/ui4qjTJzW1u7S3S++XfCQIODB8fizQXKUXqVRY6SwfjPjH92nVl0bf9bjrRO9 XHCCaIVde5xoaVD0mmkz2033Kuudh2cyGZnwRn3eW24XwA5Hz3iNWd0ykU7LCy8ElF3J Y8WS4al0ghvD2qIaGD/fDtodNVHMai/DHDhvi+113011Maf65W56lCZrKax8y0lP2qoL GJYQcymkgnP0fLrs5eUnkxiIfi4+zn+SOeA3857m/Me+i7Hp9sJKCXSf1C0UWqvoSh3V LGQA== X-Gm-Message-State: AOAM533RwaPVa2IYY5Y0Gc2Cx9yh2HGnhObWFFIRSieBQiISlyX22v16 XBMJEbwaW7Ig+SKY6pQFKODyy9crC1KwvQ== X-Google-Smtp-Source: ABdhPJwPPR2iuN3jp30eplgGdOVtkzxyXJ3pQGTYPdbkjn+L5X+IET9otNrQLF38Fo5DaJsN7KeHsA== X-Received: by 2002:adf:8282:: with SMTP id 2mr22223098wrc.519.1640819059337; Wed, 29 Dec 2021 15:04:19 -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 u3sm28475965wrs.0.2021.12.29.15.04.18 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Dec 2021 15:04:19 -0800 (PST) Message-ID: <0d168258-b2d3-f60e-87af-28b01e809038@jkqxz.net> Date: Wed, 29 Dec 2021 23:04:18 +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: From: Mark Thompson In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions 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 25/11/2021 02:41, Soft Works wrote: > The test /libavutil/tests/hwdevice checks that when deriving a device > from a source device and then deriving back to the type of the source > device, the result is matching the original source device, i.e. the > derivation mechanism doesn't create a new device in this case. > > Previously, this test was usually passed, but only due to two different > kind of flaws: > > 1. The test covers only a single level of derivation (and back) > > It derives device Y from device X and then Y back to the type of X and > checks whether the result matches X. > > What it doesn't check for, are longer chains of derivation like: > > CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4 > > In that case, the second derivation returns the first device (CUDA3 == > CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new > OpenCL4 context instead of returning OpenCL2, because there was no link > from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1) Yes, this is exactly what I expect. Because of how these APIs work, device derivation is always one-way - you can make an OpenCL device from a D3D11 one, but not the other direction. I don't think there is any case which allows both directions (nor should there be, in general), so the possibilities form an acyclic graph. I do expect the user to know here what the possible-derivation graph looks like for the cases they want to use (since only a few cases are meaningful), though maybe it would be worth explicitly documenting it somewhere. There are then two non-overlapping operations combined in the device derivation function (when A -> B is the meaningful direction): * Given a context of type A, create a context of type B using the same physical device. * Given a context of type B, retrieve the original context of type A it was created from (if it exists). (Maybe they should have been separate functions to avoid this confusion, but it did fit rather nicely in how hwmap works. Saying that derivation from A should always return the same B is not intended, nor do I think it should be. The problem with doing that is that components can no longer tell whether the device they have is shared with someone else, whether they are in ffmpeg, in unrelated libraries, or in user code. What that actually means depends on the API, but in general it would ban doing anything which might affect another user of the same device, such as setting any global options or calling anything which might not be thread-safe. (Note that the global options argument in av_hwdevice_create_derived_opts() already violates this constraint, making the patch wrong as-is even if you do think it is reasonable to impose this constraint on all users.) > If the test would check for two levels of derivation, it would have > failed. > > This patch fixes those (yet untested) cases by introducing forward > references (derived_device) in addition to the existing back references > (source_device). > > 2. hwcontext_qsv didn't properly set the source_device > > In case of QSV, hwcontext_qsv creates a source context internally > (vaapi, dxva2 or d3d11va) without calling av_hwdevice_ctx_create_derived > and without setting source_device. What would go wrong if it did? > This way, the hwcontext test ran successful, but what practically > happened, was that - for example - deriving vaapi from qsv didn't return > the original underlying vaapi device and a new one was created instead: Er... how would it create a new device? The only type VAAPI can derive from is DRM. > Exactly what the test is intended to detect and prevent. It just > couldn't do so, because the original device was hidden (= not set as the > source_device of the QSV device). > > This patch properly makes these setting and fixes all derivation > scenarios. > > (at a later stage, /libavutil/tests/hwdevice should be extended to check > longer derivation chains as well) > > Signed-off-by: softworkz > --- > v4: Fixed braces code style > > libavutil/hwcontext.c | 38 ++++++++++++++++++++++++++++++++++ > libavutil/hwcontext.h | 1 + > libavutil/hwcontext_internal.h | 6 ++++++ > libavutil/hwcontext_qsv.c | 13 +++++++++--- > 4 files changed, 55 insertions(+), 3 deletions(-) - 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".