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 AB2704B3A3 for ; Tue, 4 Jun 2024 18:58:29 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CEAA668D6EA; Tue, 4 Jun 2024 21:58:26 +0300 (EEST) Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 620BA68D4EE for ; Tue, 4 Jun 2024 21:58:20 +0300 (EEST) Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-35a264cb831so5414196f8f.2 for ; Tue, 04 Jun 2024 11:58:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20230601.gappssmtp.com; s=20230601; t=1717527499; x=1718132299; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=y7ODWgFILQGJ0n8h6v/LSwgTrPUuELbeTroj6EPyOM0=; b=jUDzyYSbEsRJ11u5a43GL7KfOXJkPLr6pvldOosCiq40CBlJTnLftR1RRUg7zUFHR1 /uwcvtdvarVCa3YmYrbYWKsnmku0W2jD7DtE2nxITPj0tdG9ViYtzWoOwWujh6zStDl7 iJusvGB7RbDbZxcypow69HYp986P3/Uchedp5VrSKUGe6gIdInXjLl5QPant8IN0KrPB 1vVd6H2kPs72dlQTGBnz+hbd1NFwq1fPT/W0nre9qCZo0GRhtpgb3d52lgzCQkILNQTR yxhMk6aSz0y9FcieLfAZFmOB3AWXXFv+nmKUZdjXyQpkx+gysSF0Hyh3BGxSiRskYOqi 8MMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717527499; x=1718132299; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=y7ODWgFILQGJ0n8h6v/LSwgTrPUuELbeTroj6EPyOM0=; b=ZFj76zB+W3AsQT3c61DHnLlNHXCOde2ku1UncrMaAHtZM5uqoClQFY06v61l5yz13i YoEgSJ33wT4ttMlpwcOZ8ag1QwhmGqG2uZ6vG0Y7fNdBnvcpdeeirEewjkasRMqAEPb9 LABHP3ErHTvxGGDqgxsOfyO7lisA9lxDiBqlqHp+ujUoUUAzn+/4TrYCnTsiScPQW+jA lg+yZ7+8hG9NSjhGgKYvUvDwYC/qLsW6kHOpukfSajfNaUurzpUXn+1c2iXzOPlodj2B 1GktKIfnkwQAwrC0HMpeXhdXM279GiUOSnkTLlmTr/b2K+EeEnnb+0hjHbzva6foLzU3 quJA== X-Gm-Message-State: AOJu0Yxao2ccaxxQ9JpFxUd5QdLsvjgxOU7bcdkg4g8zQAfZt7ZnVYrx QVsXUWNzCMGKORbbjDxD8bIIT8IA0JRpG1aezfCfNEzcr6km6nxGJ1Ok0kpIiA0bo6qfg6WBkKj J X-Google-Smtp-Source: AGHT+IHsoVjlZiAfiH980A5kcGMAzLL2BZ1H5VrWTTD03qVtX/oc5fc7e9fwhydFnLAA052zTEc34Q== X-Received: by 2002:adf:fac3:0:b0:355:340:d6ab with SMTP id ffacd0b85a97d-35e84068679mr223523f8f.22.1717527499091; Tue, 04 Jun 2024 11:58:19 -0700 (PDT) Received: from [192.168.0.15] (cpc92302-cmbg19-2-0-cust1183.5-4.cable.virginm.net. [82.1.212.160]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4214bf49109sm19480985e9.1.2024.06.04.11.58.18 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 04 Jun 2024 11:58:18 -0700 (PDT) Message-ID: Date: Tue, 4 Jun 2024 19:58:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20240530130826.374-1-ovchinnikov.dmitrii@gmail.com> From: Mark Thompson In-Reply-To: <20240530130826.374-1-ovchinnikov.dmitrii@gmail.com> Subject: Re: [FFmpeg-devel] [PATCH 01/10, v3] avutil: add hwcontext_amf. 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 30/05/2024 14:08, Dmitrii Ovchinnikov wrote: > Adds hwcontext_amf, which allows to use shared AMF > context for the encoder, decoder and AMF-based filters, > without copy to the host memory. > It will also allow you to use some optimisations in > the interaction of components (for example, SAV) and make a more > manageable and optimal setup for using GPU devices with AMF > in the case of a fully AMF pipeline. > It will be a significant performance uplift when full AMF pipeline > with filters is used. > > We also plan to add Compression artefact removal filter in near feature. > v2: cleanup header files > v3: an unnecessary class has been removed. > --- > libavutil/Makefile | 4 + > libavutil/hwcontext.c | 4 + > libavutil/hwcontext.h | 1 + > libavutil/hwcontext_amf.c | 585 +++++++++++++++++++++++++++++ > libavutil/hwcontext_amf.h | 64 ++++ > libavutil/hwcontext_amf_internal.h | 44 +++ > libavutil/hwcontext_internal.h | 1 + > libavutil/pixdesc.c | 4 + > libavutil/pixfmt.h | 5 + > 9 files changed, 712 insertions(+) > create mode 100644 libavutil/hwcontext_amf.c > create mode 100644 libavutil/hwcontext_amf.h > create mode 100644 libavutil/hwcontext_amf_internal.h > > ... > + > +static void amf_dummy_free(void *opaque, uint8_t *data) > +{ > + > +} > + > +static AVBufferRef *amf_pool_alloc(void *opaque, size_t size) > +{ > + AVHWFramesContext *hwfc = (AVHWFramesContext *)opaque; > + AVBufferRef *buf; > + > + buf = av_buffer_create(NULL, NULL, amf_dummy_free, hwfc, AV_BUFFER_FLAG_READONLY); > + if (!buf) { > + av_log(hwfc, AV_LOG_ERROR, "Failed to create buffer for AMF context.\n"); > + return NULL; > + } > + return buf; > +} You're still allocating nothing here? I think what this means is that you don't actually want to implement frames context creation at all because it doesn't do anything. If it is not possible to make an AMFSurface as anything other than an output from an AMF component then this would make sense, the decoder can allocate the internals. Look at the DRM hwcontext for an example that works like this - the DRM objects can only be made as outputs from devices or by mapping, so there is no frame context implementation. > + > ... > diff --git a/libavutil/hwcontext_amf.h b/libavutil/hwcontext_amf.h > new file mode 100644 > index 0000000000..ef2118dd4e > --- /dev/null > +++ b/libavutil/hwcontext_amf.h > @@ -0,0 +1,64 @@ > +/* > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > + > +#ifndef AVUTIL_HWCONTEXT_AMF_H > +#define AVUTIL_HWCONTEXT_AMF_H > + > +#include "pixfmt.h" > +#include "hwcontext.h" > +#include > +#include > +#include > +#include > + > +/** > + * This struct is allocated as AVHWDeviceContext.hwctx > + */ > +typedef struct AVAMFDeviceContext { > + HMODULE library; What is this type? (It looks like a Windows type, but I thought this was cross-platform.) > + AMFFactory *factory; > + AMFDebug *debug; > + AMFTrace *trace; > + void *trace_writer; Are all of these objects necessary to operation of the AMF device? Please remove elements which are not necessary and add them to the private context if they are otherwise useful. > + > + int64_t version; ///< version of AMF runtime Why is the version necessary to expose in the public API? Is it not possible to call the QueryVersion function after starting? > + AMFContext *context; > + int mem_type; Is mem_type really necessary to expose in the piblic API? Can the user not determine this by some API call? > +} AVAMFDeviceContext; > + > +enum AMF_SURFACE_FORMAT av_amf_av_to_amf_format(enum AVPixelFormat fmt); > +enum AVPixelFormat av_amf_to_av_format(enum AMF_SURFACE_FORMAT fmt); > + All of the following functions should not be public symbols. You want to implement the hwcontext functions so that these all work without needing special implementation for AMF, they should not be individually callable because that is not useful. > +int av_amf_context_create(AVAMFDeviceContext * context, > + void* avcl, > + const char *device, > + AVDictionary *opts, int flags); Use device_create. > +int av_amf_context_init(AVAMFDeviceContext* internal, void* avcl); Use device_init. > +void av_amf_context_free(void *opaque, uint8_t *data); Use device_uninit (or maybe an AVBuffer destructor?). > +int av_amf_context_derive(AVAMFDeviceContext * internal, > + AVHWDeviceContext *child_device_ctx, AVDictionary *opts, > + int flags); Use device_derive. > + > +int av_amf_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst, > + const AVFrame *src); Use transfer_data_from. > + > +int av_amf_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst, > + const AVFrame *src); Use transfer_data_to. > + > +#endif /* AVUTIL_HWCONTEXT_AMF_H */ > ... Thanks, - 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".