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 70BEF48C39 for ; Thu, 22 Feb 2024 09:53:53 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5DFED68D212; Thu, 22 Feb 2024 11:53:50 +0200 (EET) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 932B968D1F0 for ; Thu, 22 Feb 2024 11:53:43 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1708595629; x=1740131629; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=+QNyIpFtySi6x4I+hY9DuOLxHOE/me10tUTlBnUVOIk=; b=BD5peBdLPu5NkGmHWBHSFSj/cfTTd3vxiUCaMLJqY+xRUKOtuY09mOpi 6W+hGe8BvMeKuHM2D+xt/oxgcSUTrSzAfToP8dufZ2kKgoRZFei4rNjRk 4uKWK9t4VZwoh8EP/jx1fiO/s1OQx+eSJlKyaexQ/QpOgy+t1CLaV46EY 0lX2uIwmlDAXWY1Y597NGe6pa2yp5PsE2ueJNPDh3EcDk3uLtUJrn+ILx vK1csKuSrMPIZntRIX8XyKMZJPYyIy8NYaTZLr6tnUctK67VL7kDazr8t xqcccDj8VRlNL2e1b61TNperddEeKOrabzBblB3Abjqm3b2jOWmFtbrr+ A==; X-IronPort-AV: E=McAfee;i="6600,9927,10991"; a="2948877" X-IronPort-AV: E=Sophos;i="6.06,177,1705392000"; d="scan'208";a="2948877" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Feb 2024 01:53:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,177,1705392000"; d="scan'208";a="10122636" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orviesa004.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 22 Feb 2024 01:53:42 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 22 Feb 2024 01:53:40 -0800 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 22 Feb 2024 01:53:40 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Thu, 22 Feb 2024 01:53:39 -0800 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.168) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Thu, 22 Feb 2024 01:53:39 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jvv7PaJB6V76U3X8Nm54xEnZMhG/Tl5vAaR+0iF45WpSocB66zM5CcAiBhgm+ryOuq8h9p/E1+PsAFfeW8WJ4hAJDfFCEwBB4CdUllhzz2z/xyh+V0doIH0f1IdD3qOZFKaPZo9B13Kvq+Fhb5nYldez0tcqQDxwVuD82Iw84bEpZYXB6ZE6CH8ja3FLwVMbaD00dYg4KV46HJvnYaXYwmSISx9/j3wbMZQqYaPVkVlhZDNW7SCZd9fJfKs64s/BoQW+tnuX78dw06J+SSTDLfC5Ns0ITtiHDa+8zG1FPyc83mtnILBW/AQSqSd2zNTrSf+cRuh9E146sr/HVt6zIA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3D47NiLwepGa7R462RsJZhXDVUWTCQATUrJ2A9F54wE=; b=A+ORayV+rrvypWxasKThuIyYwC3QauXQVkEzHDHdhAAh8Uv40cCeNKH/8/ApzHyHRBU4Zu01U5PbAuKgrg6GalLnHJtK18S8taZWs76NfxZ7ZR7vpV7h6qtseex0C+2Il/i2IITpPujxRCPl0peL2fOc4aIvSJ5K70dEDNo8acJJPWj5FV1gYmXmKOlJaoMo530m1W0SmjjEOZ5Am76aCO6r84NtGJWkIxdDlWDJzmZDWs1NFCtuma2eLnwilRK+Bj+3aLOow+dXDVHnFqFiVLAeFD5iAq2tLvYxviMe3rr/ZZGLV3nLx1Kq0D9oqFiCEoOeQIP10ItlDt4YrmgnWA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from CH3PR11MB8659.namprd11.prod.outlook.com (2603:10b6:610:1cf::5) by BN9PR11MB5356.namprd11.prod.outlook.com (2603:10b6:408:11d::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7316.21; Thu, 22 Feb 2024 09:53:36 +0000 Received: from CH3PR11MB8659.namprd11.prod.outlook.com ([fe80::22d3:5f9d:8fc0:1a2d]) by CH3PR11MB8659.namprd11.prod.outlook.com ([fe80::22d3:5f9d:8fc0:1a2d%2]) with mapi id 15.20.7316.018; Thu, 22 Feb 2024 09:53:35 +0000 From: "Wu, Tong1" To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH v5 2/9] avcodec/vaapi_encode: introduce a base layer for vaapi encode Thread-Index: AQHaYqF5nZIh31VA1EGJ5YxJNWQYSrEWItEg Date: Thu, 22 Feb 2024 09:53:35 +0000 Message-ID: References: <20240218084529.554-1-tong1.wu@intel.com> <20240218084529.554-2-tong1.wu@intel.com> <2c0e5585-454b-4873-90df-d48b554d713b@jkqxz.net> In-Reply-To: <2c0e5585-454b-4873-90df-d48b554d713b@jkqxz.net> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CH3PR11MB8659:EE_|BN9PR11MB5356:EE_ x-ms-office365-filtering-correlation-id: 9851c265-6332-48f8-4a2f-08dc338c232a x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: dY5oQEsJwuyc6Fq7v5R4hA5c9rgCroPdfjINGCti9Dx3CBphg+HHql1Cdy+gMH3GfpDNTUnCI7zx/XnMSAhwzxRxD2qHmUNOHn8i9kNaW5Bpe70QtyMtXA65jILZnjAbT8EHZMF5U6F9rZb9lTpEsDw1vzRlFCBQMK50x8lU7wROGze6o19YiI9vk+9ZItZLdCbQd0qIfPyMQt5IdB15hIckZJSCPB93mfIuARA+mpV6RkFs/HSaghcxv2dTZH4aopuoiGOxSyntJNTPTr3opuFW8fVJdTPGG6o/ZrnZmgSxyjrUYC6GkDpILUAvklVQ475zne0JRDv17obS27LJLGsPvQIO36SQUBaLJixa64LFCUYOQuaTe702dN5aiw6s8u9paJZ+Kjv6ocxkZa6X/B6Vyco0hOmTU413UQyI63ncl5cFHIBKkdabqDblb7XclmKm7CWijL0ejcMPzVViAL/nEjiQI2FfQHca1X4p+RF1WAM3DYJdKOvn+JZi9Yl5q7UgmBZPZxuv/LAIoIdxO7aeXxByWM8N+oQ4Q9rUZSHIF7KfmrzrK5CsE9CdH/l/BhnCCKJTd+liUpfu9qCTL/OZVSRGeE9u0S5wRLqX7eA= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH3PR11MB8659.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(38070700009); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?brv4ql7lyGbV6rPA2ht1rgLFJ8SW5iBznH1nxJkqly9YkV7s3Hvs3mbSuF54?= =?us-ascii?Q?sxZgNKIBf3OfR8j1DLOx+CGAgJYwn+ugf9xhjz09h2wdRVEPoaoE2thS92y+?= =?us-ascii?Q?zMF3KFIpHXNiKUQjwKtOe1H3xw2yqkKd7NWym95pj2jmqY9J5JyGZ/Eyhe8u?= =?us-ascii?Q?wI26KqWRF4K36UNyMa+fqBzOtVNEdY7C2slJdHis4BjEQe5XgTFFrCJaO4Xk?= =?us-ascii?Q?Fo2gcP0omN0E8EUb0SISJBhSVOk9lIFB7DHNyvIwdqt5Qiv5kDn1s6LYLtQf?= =?us-ascii?Q?W73NP3w5dtstoWcnVCUKzX/5MxbXRQGiNGzoqJCX8EHHFwzh08oCZaldMsKw?= =?us-ascii?Q?C2+WQqxS3gdykoGnmP/2Yv1wbCdBXpBCxwDEmpysC+dS9GTuqHKGPGmpqysa?= =?us-ascii?Q?PMEkucYPpWglbrF9bsv501Gs26DENFbfgTtio1pFjn5JVVgwq/lzP3rqRMrb?= =?us-ascii?Q?L00p6R9XtnX0B0z1h6OjUEHDnlE7NJEKXx/p2Wq6b9E+7QuXpCnNG2HTTV1+?= =?us-ascii?Q?8aWnuWxerX8J+K2FFdz0qnwEl3eWbQAlVlssmNntbUkN5qBvFUWwGjfmsYAV?= =?us-ascii?Q?S7QID3jSpAyJ/pULXWcymnl+VJXhNMsMDh8xgJ3gjXeB6aeuKw7XuAyBLr/8?= =?us-ascii?Q?sEy8nJgWKYalUuMuHdJThUuQjKT6MXnBHZ/WVfwTYUy8DBL/eWyVW9HOZ8vu?= =?us-ascii?Q?UOIPnH6JYea82YjrEzeRhiUjrKn5FBx1n6sR+DQsrGv4Z1i4fHq09+6lIrUm?= =?us-ascii?Q?2otnZM5FPUv/iLrrBX7oPX83eVSE20ZSiBldvHq4HLSqfo1Q1QUwQ6P1Dicx?= =?us-ascii?Q?1xyTJVU20AQ+bmSl7zGXLgGU9BmlKc9lSPVt1kQT5ebFuJmfx904uEWui6qj?= =?us-ascii?Q?32+1IZKFHcSfjUgx59uDmP66UrzOOzg9VHafu+DGDsNU912u/b12dfoj9sKn?= =?us-ascii?Q?FQLcby2DcJJpDCYffEDnDdaJiaeEfP7LtAL5zfQ05KHkJE9DGVGdKtHLxn7d?= =?us-ascii?Q?0Ur/OE2aDi9GAMLbyd6VzgvA9vFzjlte0QwQxRzMKzMynfivijYM1WhrymwZ?= =?us-ascii?Q?JwLCsM4szAtWCKzYJp+/rznFivEvM/Sux88jiY8eETXUx5MruJxjAE3U7yav?= =?us-ascii?Q?JkEWEo7kjMvRTHTQrn+s1FRaQP1o6b8tQDccXZbXamAwOaUArYq34XiSoo+5?= =?us-ascii?Q?NOg+x9BoL8B10LaKD6BqATeXNHN+AXWP1FSTDk/tZBr9iCWU2AjOTDcf/HyU?= =?us-ascii?Q?ZOnOGKarDirSGBnvPDKDZ0Q3NTyzir5VpzVvrNbXKx/qO9h0/HeeSz0H1Dzu?= =?us-ascii?Q?w7k862j5jiTasZglt5lOgdsLoXIwmWsXQJWY5kjinvArAn6xzHq5NndQxEFi?= =?us-ascii?Q?PciMvLTHWec8v+UJUygQgFMLZo90BmvzTsMduF32+BVzvfetiYMDSRU5lpPN?= =?us-ascii?Q?Qglq4hEk0SiMu9760p7oBsCbXKJrpKe6tBAUBO8m+FdgqXYumj4+X4ZHZfwW?= =?us-ascii?Q?p12mNoQl72uYYzaXjd5V+ybIDud93TgOFmdHg/rSEY7J80RwDNuA60OSXV1J?= =?us-ascii?Q?zBxiJcFSZKlKG5pDa3Wk9KrUGXDh6IwuCEbSkPgg?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8659.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9851c265-6332-48f8-4a2f-08dc338c232a X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Feb 2024 09:53:35.7820 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 7j8zht8ruROLGT+W4I03SELEHBddJ8K1MgB8tLDYY9xF88g/G2V4G0O0ZJoBew7MxKGdnGZz82SMLkq5r3VuAw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5356 X-OriginatorOrg: intel.com Subject: Re: [FFmpeg-devel] [PATCH v5 2/9] avcodec/vaapi_encode: introduce a base layer for vaapi encode 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: Hi Mark. Thanks for your careful review. >On 18/02/2024 08:45, tong1.wu-at-intel.com@ffmpeg.org wrote: >> From: Tong Wu >> >> Since VAAPI and future D3D12VA implementation may share the same dpb >> logic and some other functions. A base layer encode context is introduced >> as vaapi context's base and extract the related funtions to a common >> file such as receive_packet, init, close, etc. >> >> Signed-off-by: Tong Wu >> --- >> libavcodec/Makefile | 2 +- >> libavcodec/hw_base_encode.c | 637 ++++++++++++++++++++++ >> libavcodec/hw_base_encode.h | 277 ++++++++++ >> libavcodec/vaapi_encode.c | 924 +++++++------------------------- >> libavcodec/vaapi_encode.h | 229 +------- >> libavcodec/vaapi_encode_av1.c | 73 +-- >> libavcodec/vaapi_encode_h264.c | 201 +++---- >> libavcodec/vaapi_encode_h265.c | 163 +++--- >> libavcodec/vaapi_encode_mjpeg.c | 22 +- >> libavcodec/vaapi_encode_mpeg2.c | 53 +- >> libavcodec/vaapi_encode_vp8.c | 28 +- >> libavcodec/vaapi_encode_vp9.c | 70 +-- >> 12 files changed, 1427 insertions(+), 1252 deletions(-) >> create mode 100644 libavcodec/hw_base_encode.c >> create mode 100644 libavcodec/hw_base_encode.h >> >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile >> index 470d7cb9b1..23946f6ea3 100644 >> --- a/libavcodec/Makefile >> +++ b/libavcodec/Makefile >> @@ -164,7 +164,7 @@ OBJS-$(CONFIG_STARTCODE) += startcode.o >> OBJS-$(CONFIG_TEXTUREDSP) += texturedsp.o >> OBJS-$(CONFIG_TEXTUREDSPENC) += texturedspenc.o >> OBJS-$(CONFIG_TPELDSP) += tpeldsp.o >> -OBJS-$(CONFIG_VAAPI_ENCODE) += vaapi_encode.o >> +OBJS-$(CONFIG_VAAPI_ENCODE) += vaapi_encode.o >hw_base_encode.o >> OBJS-$(CONFIG_AV1_AMF_ENCODER) += amfenc_av1.o >> OBJS-$(CONFIG_VC1DSP) += vc1dsp.o >> OBJS-$(CONFIG_VIDEODSP) += videodsp.o >> diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c >> ... > >I'm going to trust that the contents of this file are only moved around with no >functional changes. If that isn't the case please do say. You are absolutely right. > >> diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h >> new file mode 100644 >> index 0000000000..70982c97b2 >> --- /dev/null >> +++ b/libavcodec/hw_base_encode.h >> @@ -0,0 +1,277 @@ >> +/* >> + * Copyright (c) 2024 Intel Corporation > >I would avoid adding this. Most of these files have content mostly copied from >other files which are not exclusively copyright by Intel Corporation. Will delete. > >> + * >> + * 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 AVCODEC_HW_BASE_ENCODE_H >> +#define AVCODEC_HW_BASE_ENCODE_H >> + >> +#include "libavutil/hwcontext.h" >> +#include "libavutil/fifo.h" >> + >> +#include "avcodec.h" >> +#include "hwconfig.h" > >This file doesn't do anything with hwconfig stuff? > Will delete. >> + >> +enum { >> + MAX_DPB_SIZE = 16, >> + MAX_PICTURE_REFERENCES = 2, >> + MAX_REORDER_DELAY = 16, >> + MAX_ASYNC_DEPTH = 64, >> + MAX_REFERENCE_LIST_NUM = 2, >> +}; >> + >> +enum { >> + PICTURE_TYPE_IDR = 0, >> + PICTURE_TYPE_I = 1, >> + PICTURE_TYPE_P = 2, >> + PICTURE_TYPE_B = 3, >> +}; >> + >> +enum { >> + // Codec supports controlling the subdivision of pictures into slices. >> + FLAG_SLICE_CONTROL = 1 << 0, >> + // Codec only supports constant quality (no rate control). >> + FLAG_CONSTANT_QUALITY_ONLY = 1 << 1, >> + // Codec is intra-only. >> + FLAG_INTRA_ONLY = 1 << 2, >> + // Codec supports B-pictures. >> + FLAG_B_PICTURES = 1 << 3, >> + // Codec supports referencing B-pictures. >> + FLAG_B_PICTURE_REFERENCES = 1 << 4, >> + // Codec supports non-IDR key pictures (that is, key pictures do >> + // not necessarily empty the DPB). >> + FLAG_NON_IDR_KEY_PICTURES = 1 << 5, >> + // Codec output packet without timestamp delay, which means the >> + // output packet has same PTS and DTS. >> + FLAG_TIMESTAMP_NO_DELAY = 1 << 6, >> +}; >> + >> +enum { >> + RC_MODE_AUTO, >> + RC_MODE_CQP, >> + RC_MODE_CBR, >> + RC_MODE_VBR, >> + RC_MODE_ICQ, > >I thought ICQ was an Intel name which leaked into libva? This probably >shouldn't be in a generic API, maybe something about constant-quality. > >> + RC_MODE_QVBR, >> + RC_MODE_AVBR, > >More generally, I think you want to document what these modes are intended >to be. When they mapped directly to VAAPI then it was clear that the >responsibility was "whatever libva gives you", but when there are multiple >possibilities which do not use exactly the same options it is less clear. I am going to remove this enumeration from the common header and remove ICQ and AVBR totally from D3D12 side as they are not supported yet. > >> + RC_MODE_MAX = RC_MODE_AVBR, >> +}; >> + >> +typedef struct HWBaseEncodePicture { >> + struct HWBaseEncodePicture *next; >> + >> + int64_t display_order; >> + int64_t encode_order; >> + int64_t pts; >> + int64_t duration; >> + int force_idr; >> + >> + void *opaque; >> + AVBufferRef *opaque_ref; >> + >> + int type; >> + int b_depth; >> + int encode_issued; >> + int encode_complete; >> + >> + AVFrame *input_image; >> + AVFrame *recon_image; >> + >> + void *priv_data; >> + >> + // Whether this picture is a reference picture. >> + int is_reference; >> + >> + // The contents of the DPB after this picture has been decoded. >> + // This will contain the picture itself if it is a reference picture, >> + // but not if it isn't. >> + int nb_dpb_pics; >> + struct HWBaseEncodePicture *dpb[MAX_DPB_SIZE]; >> + // The reference pictures used in decoding this picture. If they are >> + // used by later pictures they will also appear in the DPB. ref[0][] for >> + // previous reference frames. ref[1][] for future reference frames. >> + int nb_refs[MAX_REFERENCE_LIST_NUM]; >> + struct HWBaseEncodePicture >*refs[MAX_REFERENCE_LIST_NUM][MAX_PICTURE_REFERENCES]; >> + // The previous reference picture in encode order. Must be in at least >> + // one of the reference list and DPB list. >> + struct HWBaseEncodePicture *prev; >> + // Reference count for other pictures referring to this one through >> + // the above pointers, directly from incomplete pictures and indirectly >> + // through completed pictures. >> + int ref_count[2]; >> + int ref_removed[2]; >> +} HWBaseEncodePicture; >> + >> +typedef struct HWEncodeType { >> + HWBaseEncodePicture * (*alloc)(AVCodecContext *avctx, AVFrame >*frame); >> + >> + int (*issue)(AVCodecContext *avctx, HWBaseEncodePicture *base_pic); > >The semantics of this are a bit blurred across the boundary of this call. I think >ideally you want the picture structure to be const here to make it clear? > >(Move the recon_image allocation and issued-flag setting out of the callee, in >particular.) > >> + >> + int (*output)(AVCodecContext *avctx, HWBaseEncodePicture *base_pic, >AVPacket *pkt); > >Feels like the picture structure should again be const for the same reason. > >> + >> + int (*free)(AVCodecContext *avctx, HWBaseEncodePicture *base_pic); > >The VAAPI implementation of this is definitely doing lots of things that should >be in the base layer. > >> +} HWEncodeType; > >I think the naming of this wants to be clear that they are operations on a single >picture (either the type name or the fields). > >Some documentation of the precise semantics of the callbacks would definitely >help as well to be sure that the split is correct. Will do as suggested. > >> + >> +typedef struct HWBaseEncodeContext { >> + const AVClass *class; >> + >> + // Hardware-specific hooks. >> + const struct HWEncodeType *hw; >> + >> + // Global options. >> + >> + // Number of I frames between IDR frames. >> + int idr_interval; >> + >> + // Desired B frame reference depth. >> + int desired_b_depth; >> + >> + // Explicitly set RC mode (otherwise attempt to pick from >> + // available modes). >> + int explicit_rc_mode; >> + >> + // Explicitly-set QP, for use with the "qp" options. >> + // (Forces CQP mode when set, overriding everything else.) >> + int explicit_qp; >> + >> + // The required size of surfaces. This is probably the input >> + // size (AVCodecContext.width|height) aligned up to whatever >> + // block size is required by the codec. >> + int surface_width; >> + int surface_height; >> + >> + // The block size for slice calculations. >> + int slice_block_width; >> + int slice_block_height; >> + >> + // RC quality level - meaning depends on codec and RC mode. >> + // In CQP mode this sets the fixed quantiser value. >> + int rc_quality; >> + >> + AVBufferRef *device_ref; >> + AVHWDeviceContext *device; >> + >> + // The hardware frame context containing the input frames. >> + AVBufferRef *input_frames_ref; >> + AVHWFramesContext *input_frames; >> + >> + // The hardware frame context containing the reconstructed frames. >> + AVBufferRef *recon_frames_ref; >> + AVHWFramesContext *recon_frames; >> + >> + // Current encoding window, in display (input) order. >> + HWBaseEncodePicture *pic_start, *pic_end; >> + // The next picture to use as the previous reference picture in >> + // encoding order. Order from small to large in encoding order. >> + HWBaseEncodePicture *next_prev[MAX_PICTURE_REFERENCES]; >> + int nb_next_prev; >> + >> + // Next input order index (display order). >> + int64_t input_order; >> + // Number of frames that output is behind input. >> + int64_t output_delay; >> + // Next encode order index. >> + int64_t encode_order; >> + // Number of frames decode output will need to be delayed. >> + int64_t decode_delay; >> + // Next output order index (in encode order). >> + int64_t output_order; >> + >> + // Timestamp handling. >> + int64_t first_pts; >> + int64_t dts_pts_diff; >> + int64_t ts_ring[MAX_REORDER_DELAY * 3 + >> + MAX_ASYNC_DEPTH]; >> + >> + // Frame type decision. >> + int gop_size; >> + int closed_gop; >> + int gop_per_idr; >> + int p_per_i; >> + int max_b_depth; >> + int b_per_p; >> + int force_idr; >> + int idr_counter; >> + int gop_counter; >> + int end_of_stream; >> + int p_to_gpb; >> + >> + // Whether the driver supports ROI at all. >> + int roi_allowed; >> + >> + // The encoder does not support cropping information, so warn about >> + // it the first time we encounter any nonzero crop fields. >> + int crop_warned; >> + // If the driver does not support ROI then warn the first time we >> + // encounter a frame with ROI side data. >> + int roi_warned; >> + >> + AVFrame *frame; >> + >> + // Whether the HW supports sync buffer function. >> + // If supported, encode_fifo/async_depth will be used together. >> + // Used for output buffer synchronization. >> + int async_encode; >> + >> + // Store buffered pic >> + AVFifo *encode_fifo; >> + // Max number of frame buffered in encoder. >> + int async_depth; >> + >> + /** Tail data of a pic, now only used for av1 repeat frame header. */ >> + AVPacket *tail_pkt; >> +} HWBaseEncodeContext; >> + >> +int ff_hw_base_encode_receive_packet(AVCodecContext *avctx, AVPacket >*pkt); >> + >> +int ff_hw_base_encode_init(AVCodecContext *avctx); >> + >> +int ff_hw_base_encode_close(AVCodecContext *avctx); >> + >> +#define HW_BASE_ENCODE_COMMON_OPTIONS \ >> + { "idr_interval", \ >> + "Distance (in I-frames) between IDR frames", \ >> + OFFSET(common.base.idr_interval), AV_OPT_TYPE_INT, \ >> + { .i64 = 0 }, 0, INT_MAX, FLAGS }, \ >> + { "b_depth", \ >> + "Maximum B-frame reference depth", \ >> + OFFSET(common.base.desired_b_depth), AV_OPT_TYPE_INT, \ >> + { .i64 = 1 }, 1, INT_MAX, FLAGS }, \ >> + { "async_depth", "Maximum processing parallelism. " \ >> + "Increase this to improve single channel performance.", \ >> + OFFSET(common.base.async_depth), AV_OPT_TYPE_INT, \ >> + { .i64 = 2 }, 1, MAX_ASYNC_DEPTH, FLAGS } >> + >> +#define HW_BASE_ENCODE_RC_MODE(name, desc) \ >> + { #name, desc, 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_ ## name }, \ >> + 0, 0, FLAGS, "rc_mode" } >> +#define HW_BASE_ENCODE_RC_OPTIONS \ >> + { "rc_mode",\ >> + "Set rate control mode", \ >> + OFFSET(common.base.explicit_rc_mode), AV_OPT_TYPE_INT, \ >> + { .i64 = RC_MODE_AUTO }, RC_MODE_AUTO, RC_MODE_MAX, >FLAGS, .unit = "rc_mode" }, \ >> + { "auto", "Choose mode automatically based on other parameters", \ >> + 0, AV_OPT_TYPE_CONST, { .i64 = RC_MODE_AUTO }, 0, 0, FLAGS, .unit = >"rc_mode" }, \ >> + HW_BASE_ENCODE_RC_MODE(CQP, "Constant-quality"), \ >> + HW_BASE_ENCODE_RC_MODE(CBR, "Constant-bitrate"), \ >> + HW_BASE_ENCODE_RC_MODE(VBR, "Variable-bitrate"), \ >> + HW_BASE_ENCODE_RC_MODE(ICQ, "Intelligent constant-quality"), \ >> + HW_BASE_ENCODE_RC_MODE(QVBR, "Quality-defined variable-bitrate"), >\ >> + HW_BASE_ENCODE_RC_MODE(AVBR, "Average variable-bitrate") > >This set of options is rather nasty because the set which makes sense depends >on the API (see how your later D3D12 patch can't map some of them but has >to deal with them anyway). Maybe avoid extracting this to the common layer? > Sure I'll move it back to where it was. Thanks, Tong >> + >> +#endif /* AVCODEC_HW_BASE_ENCODE_H */ >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c >> ... > >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". _______________________________________________ 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".