From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id CECE54E96A
for <ffmpegdev@gitmailbox.com>; Fri, 9 May 2025 11:31:33 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6421368C37D;
Fri, 9 May 2025 14:31:29 +0300 (EEST)
Received: from NAM11-CO1-obe.outbound.protection.outlook.com
(mail-co1nam11olkn2086.outbound.protection.outlook.com [40.92.18.86])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BC44468C30B
for <ffmpeg-devel@ffmpeg.org>; Fri, 9 May 2025 14:31:22 +0300 (EEST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;
b=jjDQq/QUj62xde8GQFD6HTzCrxd3FNiduPv1DoNXCYsXIYDDkPYiMc0QMAJ+RUaEJRz+XTrkw+aumuDOzCi/cvfeBjeOGzaqL8/BWEI5anBPxCqpME4exUpuE/xpDpRVv5rh7nkPY0BCXx21j+VMcYQRMHrdaB7fSuvGEwocLuRnT9S+kFwDump0dr7eYWS6h/riTvwv9IlpHLznY8HxOBREeta6ALT+9AWll9Tmnyj8/CQ5TvPTozieXbpfOFAnLeG5vslgDP+Hbns7UeNmanRco4FocW1HUuXWykhQDz4zas0CdwOt/l8mnIzxnAGRKlR/pJuKGmttiB+Vo9pg4Q==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;
s=arcselector10001;
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=51T3+mN3SfVv3RG9Xi4OaiPjq5PhNYbTkIqkItY+NwU=;
b=ff7ekdMKxYSZtvMgsbd+bzsNYY0pq7iUrE226Veu5QOlvbri90shqdtJbYQbNQyC61LWSGsqIxqYEZV6PFe2PgxdVO1KC3sn9b4AnxsHqUqhAHrWPYeZkzvDQc36kAvej9/JqHUXtaBZA1d3v8ipdnQidvrwFhA+s/yXat7Ec6eLbDYAN3fkOMWqChwOgT0xSBeW7Gv4O5XMHHKytvBC+xtugTgCWbq/QirL4PiR2ZLlegLLD3i7Ze88xJ3lvn957DIueOKqt0qlX9eTeKJkEqoScqFIBcxHRnPqrJ/jQoPEOvvWBKU6UP1HclP1WeoAiYDr3yEdQ4haqP4N08QaHA==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none;
dkim=none; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com;
s=selector1;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=51T3+mN3SfVv3RG9Xi4OaiPjq5PhNYbTkIqkItY+NwU=;
b=P0bIVjv1k3G8RFJN52qiizqMrrFYP4wjcxUG2fpUq5QvHVaXUUyJhWnJggrCCBB43AUeoEHYfC79oNznEKNQ9wAgxdF2h+trSbTQLjlFKMB4r0+K1VWRJNYX+lAUAb+5XOOIT9mzv39NVpROXbixCCPBdopEqcizSBc6KDfQhT91UCe+3NYoaVmuYSZKEE6gbIpDrDksi7xVtNaydS0FuWRrrZJaBc7eozsv3/w+lfr+ZryvjcX0eSxCK9l5t02iFoT/ufmRo2CUmcFOo69CODz720ugQUrPDbX+eKIIrLOxwAfZaC4xRiURvJGh2pocU0cm/Xlsw2SGlHe/nKsscw==
Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by
MN0P223MB1213.NAMP223.PROD.OUTLOOK.COM (2603:10b6:208:4c4::16) with Microsoft
SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
15.20.8699.24; Fri, 9 May 2025 11:31:18 +0000
Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM
([fe80::bf09:8e9:b07f:98a7]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM
([fe80::bf09:8e9:b07f:98a7%4]) with mapi id 15.20.8722.020; Fri, 9 May 2025
11:31:18 +0000
From: "softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>
To: Stefano Sabatini <stefasab@gmail.com>
Thread-Topic: [FFmpeg-devel] [PATCH v10 06/15] fftools/textformat: Introduce
AVTextFormatOptions for avtext_context_open()
Thread-Index: AQHbvKBcnhhzxZap10eWd4vtA4PxOrPH4KCAgAAD0fCAAWVsAIAA3EKw
Date: Fri, 9 May 2025 11:31:18 +0000
Message-ID: <DM8P223MB03651604ECEFE6D53E06688DBA8AA@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
References: <pull.66.v9.ffstaging.FFmpeg.1746260565.ffmpegagent@gmail.com>
<pull.66.v10.ffstaging.FFmpeg.1746327446.ffmpegagent@gmail.com>
<3aa16bc39f30554808c946673dd3862380314788.1746327446.git.ffmpegagent@gmail.com>
<aBv1V3pmE0iRt7Lq@mariano>
<DM8P223MB0365C851CD4090830CB64D7FBA8BA@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
<aB0kXscb6RbcloYb@mariano>
In-Reply-To: <aB0kXscb6RbcloYb@mariano>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: DM8P223MB0365:EE_|MN0P223MB1213:EE_
x-ms-office365-filtering-correlation-id: 9d051a51-a4b3-499e-412f-08dd8eed0407
x-microsoft-antispam: BCL:0;
ARA:14566002|19110799006|461199028|15080799009|8062599006|7092599006|8060799009|3412199025|440099028|12091999003|102099032;
x-microsoft-antispam-message-info: =?us-ascii?Q?Zt86etrQ4uS5UItlvEYZK1cbm05GAfBwgtdmfug9WKZ+htFXikBd8W+rQRQn?=
=?us-ascii?Q?vaOJmDJQvDX/23TNRqq4q3wZTGcrJ7CcOI6aSXNOyYGgDpwI4QfefUOnOHRK?=
=?us-ascii?Q?y07HjZn3xZcUWOFeWqj1GS2Z0ScD9TYyrZJHaieqy9wWlQkIp21u6uUUG4p3?=
=?us-ascii?Q?SmGKVZOpcy94x4Mi4rifWgzgET+gSffO7cPdng6kqBtVvOJTcVYVfofKuKsE?=
=?us-ascii?Q?+zf8m7xhVnbxnpUpi+9KPM5rCdxvu0nDcJezNgiBhEl6UCtVjN1WssPn1RGt?=
=?us-ascii?Q?6qdmtj5Ohx8vnOsz/JVyM6Apav8NsWGx6VbusUW2E06+txlf3F7Txtl/nQxG?=
=?us-ascii?Q?+I+zeZ176lcSYUlJ9oe1D8kAgMUfipJwLj84uOGavp2w+BlcqPWMkFmh+1D1?=
=?us-ascii?Q?qPgskiTIfCnN2ldP7brLMg5OEZ5nIQw60lDfgOJI4Bps7zAJlo4TWNlJhduN?=
=?us-ascii?Q?1J+bW/9DaP69bXhKfHmN0yKtAUgqz5hp1LWm7u/0danAN9Sp/0yn/aDD60uH?=
=?us-ascii?Q?c6up7n/QEx38z+XaP7113FLE4Yc/i5gs0TwFFM/QMc7fWwDY+19JWTs8s50m?=
=?us-ascii?Q?ok75uIH0xL4ew2bFFyN7BQT7a9HkqU8MF39WhoAsWFpuG/H4F5HQg89Rj9j2?=
=?us-ascii?Q?ym/NDZPPSCIOeMasZU3PG4BcqNrw063bOqYc1XQMcZXVW83Mqr/vOFSLLnDu?=
=?us-ascii?Q?lksJ5R7RVN3SI60DwxI3i8sdo4C/UY/g6HT2/F8xd2bkLqw+mPH0iRmV5Y6z?=
=?us-ascii?Q?Kn+LCB0zEiy4nTgDf7asfMzy3e9duXVTQLqWnS+4heIoiC/5XwZTV2H0FbRQ?=
=?us-ascii?Q?lBkJ64glPhxOQX1DGgKc/Bd18LTAzO8Vc8FV7FLdLP3O/oKQQM5NvYtJtDBn?=
=?us-ascii?Q?T6yQACMAb0skgQ4hrSXPMTCLQ2503c5pc9HhNObXvbG3Viq9jhB4UhmS1vJC?=
=?us-ascii?Q?Q5kjJ7yc/8YWPJ2yfdhlAViF73+8VzMUHdHgcOMngJYBewQvTdQPt/+HYgzD?=
=?us-ascii?Q?WEIpU+eopfQ7ebyL8M9FygTuJnoUqzUoAPh9FQLDNFTMSAfzUziKdZB40K/z?=
=?us-ascii?Q?gNWP0xPQkTzyLLNgQYBBt/I9qJN+XzC/UHmu6zDDR9puIY3fdI5i4Uj42WC9?=
=?us-ascii?Q?eZjbxpmULZiSpN4d8OJqXT2IZZZFzu+msQ=3D=3D?=
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?97WGkBY3A5KvXSWPL+9r7o3PmuVwOIiVVd9dtW3e5Isqh9XkPunCewZlxZqI?=
=?us-ascii?Q?DWiqwdU1kUnkjy9tbXHy/b5r1MmtsQHTYLARgpjb544IzLZABS9YUpJEGbcF?=
=?us-ascii?Q?RJU7xrjfKFUgNRkDmig0Q63N3UL7mnJ0ue5KrBTEUfEH3gr7+2IhFtTXBqWJ?=
=?us-ascii?Q?ApObZtg3Xd6OTU9BGmjbkPSltbIhlXI8yiawqOj4ozE7wfSVoS7t8rsY4Ptq?=
=?us-ascii?Q?qHWDoLKqixrf05IHkQ/xXI5VColahwiNrUd7uMs0X6pzcJEwWl3IHdLjKdif?=
=?us-ascii?Q?aNjxRQN6VuD+mqGdU4hErXS7AOiLgrgsQRnDJcmBJmglrtba+EQUN/oKXRHC?=
=?us-ascii?Q?ZY5yj8lZgi7jWYhibMz0KCgW4puRUsQIrsdsGOeAAK3UMEw0+DK4e7wsKGcd?=
=?us-ascii?Q?dQ630zo0TJzhLJxJvzif3Q42URW2LkNS9NqIgHtcXkWv8j7jrr+Ramxx5qBh?=
=?us-ascii?Q?y12C55puqX8dMf+dtQj8LzwWeP6uVDwKFNesx4Q0vXS4ShJhUPKMam5ehacp?=
=?us-ascii?Q?Az9PJQowGl1kaToinr15CY6KEUGFpZoyk0FPWAVxaoc7EavoPhnSCYHmGUVp?=
=?us-ascii?Q?pe9HtfNBgiAO/c6osNK2vdryL/7Uz+pQKRVwVIRlSffJV4hPuJvn367VD1aa?=
=?us-ascii?Q?uC3jHvO/NKfdzbxpUbHfZEdJ0Ig4epn9e6kEQUchMgPLcjNA0ku8qYvRsjo2?=
=?us-ascii?Q?V+JSGhC1nXzyd1J9Gam7lniH7MgJo2WZ6MIO2AOh+75B4quXJc+xdhsCeDcs?=
=?us-ascii?Q?xiO1ldzF8e5bVPGAMppwvR+Yo5qAhmamzgVpFxCiKdnFMeNTYTTIcFTy4HKt?=
=?us-ascii?Q?bn3+YsmOsm0FnxRDpf37MFy/XAJlTPAz1KSBe8qoAX8IB7pJ8/FxojJc0vUl?=
=?us-ascii?Q?sHJB5fDGUkiIXB1dNYascRspVJZLlaBKt9NYwxWwB9PLcpM//ZnY/FuuJfjR?=
=?us-ascii?Q?MDp3EQvlTWeOIGG2ss0G2GhNo4dX8B4jXIBaOHTIRu9IpWu5f2UHUu6dw3Hi?=
=?us-ascii?Q?tj+lCAvf42TqzWEWLNro5JiWfzOo4Cike9OTthYy8kcgvn4qYueiabeVCx//?=
=?us-ascii?Q?xumvWGJar3OWplcXsCOKhcNAVW2XQ6Tf82BBckJvmVMKn9BuFIv0YOkIqS83?=
=?us-ascii?Q?Fr8nq5PTlPnjgQXetIo7Vg8y1kkFgw5Rb2CVmNLwNP/6IEwMdTAtgOg2Qiu4?=
=?us-ascii?Q?0YAdbu2iIXyYoCiscn54T36tTpcTk6uy+nUjTmtGavJRayAB/vm6ytSzx3o?=
=?us-ascii?Q?=3D?=
MIME-Version: 1.0
X-OriginatorOrg: sct-15-20-7719-20-msonline-outlook-92255.templateTenant
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d051a51-a4b3-499e-412f-08dd8eed0407
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 May 2025 11:31:18.2130 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0P223MB1213
Subject: Re: [FFmpeg-devel] [PATCH v10 06/15] fftools/textformat: Introduce
AVTextFormatOptions for avtext_context_open()
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/DM8P223MB03651604ECEFE6D53E06688DBA8AA@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
> -----Original Message-----
> From: Stefano Sabatini <stefasab@gmail.com>
> Sent: Donnerstag, 8. Mai 2025 23:39
> To: softworkz . <softworkz@hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v10 06/15] fftools/textformat: Introduce
> AVTextFormatOptions for avtext_context_open()
>
> On date Thursday 2025-05-08 00:25:28 +0000, softworkz . wrote:
> > > -----Original Message-----
> > > From: Stefano Sabatini <stefasab@gmail.com>
> > > Sent: Donnerstag, 8. Mai 2025 02:06
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Cc: softworkz <softworkz@hotmail.com>
> > > Subject: Re: [FFmpeg-devel] [PATCH v10 06/15] fftools/textformat:
> Introduce
> > > AVTextFormatOptions for avtext_context_open()
> > >
> > > On date Sunday 2025-05-04 02:57:17 +0000, softworkz wrote:
> > > > From: softworkz <softworkz@hotmail.com>
> > > >
> > > > This allows future addition of options without
> > > > changes to the signature of avtext_context_open().
> > > >
> > > > Reviewed-by: Stefano Sabatini <stefasab@gmail.com>
> > > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > > ---
> > > > fftools/ffprobe.c | 13 +++++++++----
> > > > fftools/textformat/avtextformat.c | 21 ++++++++-------------
> > > > fftools/textformat/avtextformat.h | 16 +++++++++-------
> > > > 3 files changed, 26 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > > > index f5c83925b9..1277b1e4f9 100644
> > > > --- a/fftools/ffprobe.c
> > > > +++ b/fftools/ffprobe.c
> > > > @@ -3168,10 +3168,15 @@ int main(int argc, char **argv)
> > > > if (ret < 0)
> > > > goto end;
> > > >
> > > > - if ((ret = avtext_context_open(&tctx, f, wctx, f_args,
> > > > - sections, FF_ARRAY_ELEMS(sections),
> > > show_value_unit,
> > > > - use_value_prefix,
> use_byte_value_binary_prefix,
> > > use_value_sexagesimal_format,
> > > > - show_optional_fields, show_data_hash)) >=
> 0) {
> > >
> > > > + AVTextFormatOptions tf_options = {
> > > > + .show_optional_fields = show_optional_fields,
> > > > + .show_value_unit = show_value_unit,
> > > > + .use_value_prefix = use_value_prefix,
> > > > + .use_byte_value_binary_prefix = use_byte_value_binary_prefix,
> > > > + .use_value_sexagesimal_format = use_value_sexagesimal_format,
> > > > + };
> > > > +
> > > > + if ((ret = avtext_context_open(&tctx, f, wctx, f_args, sections,
> > > FF_ARRAY_ELEMS(sections), tf_options, show_data_hash)) >= 0) {
> > > > if (f == &avtextformatter_xml)
> > > > tctx->string_validation_utf8_flags |=
> > > AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES;
> > > >
> > > > diff --git a/fftools/textformat/avtextformat.c
> > > b/fftools/textformat/avtextformat.c
> > > > index b2c3aa3fc7..91469ef576 100644
> > > > --- a/fftools/textformat/avtextformat.c
> > > > +++ b/fftools/textformat/avtextformat.c
> > > > @@ -125,13 +125,7 @@ void avtext_context_close(AVTextFormatContext
> **ptctx)
> > > >
> > > >
> > > > int avtext_context_open(AVTextFormatContext **ptctx, const
> AVTextFormatter
> > > *formatter, AVTextWriterContext *writer_context, const char *args,
> > > > - const AVTextFormatSection *sections, int
> > > nb_sections,
> > > > - int show_value_unit,
> > > > - int use_value_prefix,
> > > > - int use_byte_value_binary_prefix,
> > > > - int use_value_sexagesimal_format,
> > > > - int show_optional_fields,
> > > > - char *show_data_hash)
> > > > + const AVTextFormatSection *sections, int
> > > nb_sections, AVTextFormatOptions options, char *show_data_hash)
> > > > {
> > > > AVTextFormatContext *tctx;
> > > > int i, ret = 0;
> > > > @@ -154,11 +148,11 @@ int avtext_context_open(AVTextFormatContext
> **ptctx,
> > > const AVTextFormatter *form
> > > > goto fail;
> > > > }
> > > >
> > > > - tctx->show_value_unit = show_value_unit;
> > > > - tctx->use_value_prefix = use_value_prefix;
> > > > - tctx->use_byte_value_binary_prefix = use_byte_value_binary_prefix;
> > > > - tctx->use_value_sexagesimal_format = use_value_sexagesimal_format;
> > > > - tctx->show_optional_fields = show_optional_fields;
> > > > + tctx->show_value_unit = options.show_value_unit;
> > > > + tctx->use_value_prefix = options.use_value_prefix;
> > > > + tctx->use_byte_value_binary_prefix =
> > > options.use_byte_value_binary_prefix;
> > > > + tctx->use_value_sexagesimal_format =
> > > options.use_value_sexagesimal_format;
> > > > + tctx->show_optional_fields = options.show_optional_fields;
> > > >
> > > > if (nb_sections > SECTION_MAX_NB_SECTIONS) {
> > > > av_log(tctx, AV_LOG_ERROR, "The number of section definitions
> (%d)
> > > is larger than the maximum allowed (%d)\n", nb_sections,
> > > SECTION_MAX_NB_SECTIONS);
> > > > @@ -201,7 +195,7 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> > > const AVTextFormatter *form
> > > > av_dict_free(&opts);
> > > > }
> > > >
> > > > - if (show_data_hash)
> > > > + if (show_data_hash) {
> > > > if ((ret = av_hash_alloc(&tctx->hash, show_data_hash)) < 0) {
> > > > if (ret == AVERROR(EINVAL)) {
> > > > const char *n;
> > > > @@ -212,6 +206,7 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> > > const AVTextFormatter *form
> > > > }
> > > > goto fail;
> > > > }
> > > > + }
> > > >
> > > > /* validate replace string */
> > > > {
> > > > diff --git a/fftools/textformat/avtextformat.h
> > > b/fftools/textformat/avtextformat.h
> > > > index 8ff503401a..87f57d8c24 100644
> > > > --- a/fftools/textformat/avtextformat.h
> > > > +++ b/fftools/textformat/avtextformat.h
> > > > @@ -117,17 +117,19 @@ struct AVTextFormatContext {
> > > > unsigned int string_validation_utf8_flags;
> > > > };
> > > >
> > > > +typedef struct AVTextFormatOptions {
> > > > + int show_optional_fields;
> > > > + int show_value_unit;
> > > > + int use_value_prefix;
> > > > + int use_byte_value_binary_prefix;
> > > > + int use_value_sexagesimal_format;
> > > > +} AVTextFormatOptions;
> > >
> > > I'm not yet convinced this is needed - why not to use a flags field as
> > > in most other places?
> >
> > Believe it or not, I actually had flags initially, but then I figured
> > how ugly this code:
> >
> > > > + AVTextFormatOptions tf_options = {
> > > > + .show_optional_fields = show_optional_fields,
> > > > + .show_value_unit = show_value_unit,
> > > > + .use_value_prefix = use_value_prefix,
> > > > + .use_byte_value_binary_prefix = use_byte_value_binary_prefix,
> > > > + .use_value_sexagesimal_format = use_value_sexagesimal_format,
> > > > + };
> > > > +
> >
>
> > ..would look like when assembling flags from the settings values and then
> > I changed to this way, which looks much cleaner and nicer.
> > This also allows adding non-bool options in the future without changing the
> > signature of avtextformat_open().
>
> My concern was mostly about this not being very FFmpeg-ish as this is
> a pattern not very much applied in FFmpeg (probably the only
> instance?).
It's the only case where it is called "options", but otherwise it's an
existing pattern to have an *_open() function which takes a struct that
holds configuration data like AVInputFormat in avformat_open_input().
In other cases, we APIs where all settings are specified as function
parameters like avio_alloc_context() - that's more like we had it before
where you said that these are too many parameters for the open function.
Another frequent pattern is to have a separate *_alloc_context() function
without any parameters where the parameters are on the context object
and you do:
- *_alloc_context()
- set params on the context object
_ *_open_context(XXContext *ctx)
We could do that, but with this pattern, it's always unclear which
parts of the struct are meant to be set by the caller and which are
populated by the *_open() function - at least in cases where the
context struct is rather complex like in our case.
Once another way would be to keep AVTextFormatOptions and
avtext_context_open() as I'm proposing but make AVTextFormatOptions
part of AVTextFormatContext - like as 'options' - and the
avtext_context_open() would then set the options to the supplied
ones with a single assignment.
All accesses would need to be changed then like from
tctx->show_value_unit to tctx->options.show_value_unit
We could also call it 'config' and AVTextFormatConfig to
disambiguate from user-supplied (cli) options.
> A more FFmpeg-ish solution would be to have a context with
> options.
I don't think that this is a good idea because that would mean that
you must either expose those options as a child object in some
way to become considered as "receiver" when options are applied
or otherwise, you would need to construct a string first (before
calling avtext_context_open() and have it parsed immediately
afterwards (inside the open function) to get them applied.
> Anyway feel free to consider this an idea for a further
> improvement rather than a blocker.
Yea, I don't see a clearly better way at the moment, but we
can change it any time later once there is one.
Thanks,
sw
_______________________________________________
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".