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 24C4B4ABDF
for <ffmpegdev@gitmailbox.com>; Thu, 24 Apr 2025 11:53:01 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4E6C668B3B0;
Thu, 24 Apr 2025 14:52:57 +0300 (EEST)
Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net
[217.70.183.198])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1E6D9687DE9
for <ffmpeg-devel@ffmpeg.org>; Thu, 24 Apr 2025 14:52:50 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id 40BE043292
for <ffmpeg-devel@ffmpeg.org>; Thu, 24 Apr 2025 11:52:49 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
s=gm1; t=1745495569;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:mime-version:mime-version:content-type:content-type:
in-reply-to:in-reply-to:references:references;
bh=z/Cu5Yze7tSvdrKACtDqxdTnHw74dtdCA2ouYEUEdh4=;
b=Y3pVZGEfcvEaDxMhOMvZinYdkZzm/XsHuqdYZpuCGJXATNrXouWxUeJpTzDLs5nMmN3z3P
qh91eM2HTX03HOju5yIdeDVOFIjeA3H+2qekC/NxoovczuN9tFJpGDxjS6rWJ2qAAWZfa6
fqBno5zI2K2sj/kLmXCLmahCMC2pU11j+gpdsV5krEGmiau1Q8Hctiz6H2cTCsrDTqPMLi
+WaA8zeVZTqVLU2JkXX/HwLRFtyznJVBXOyA5AI7tloakWF/cy9LK276GMgFCzrYGoM6rc
38GPApQcqirWm6HFHszvY7JjoPe/rsE5+Aj2kE0a0ihfAc2TRvZnbZUiicUTZQ==
Date: Thu, 24 Apr 2025 13:52:48 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20250424115248.GU4991@pb2>
References: <20250420022929.724535-1-michael@niedermayer.cc>
<5c3978fd-41e4-d686-e17e-b9beb96a657b@passwd.hu>
<20250423003131.GQ4991@pb2>
<221753ed-8460-094f-8c7b-73a77d12d5ec@passwd.hu>
MIME-Version: 1.0
In-Reply-To: <221753ed-8460-094f-8c7b-73a77d12d5ec@passwd.hu>
X-GND-State: clean
X-GND-Score: -70
X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvgeelfeelucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeeigeektdejudffjefhteegjedtgeettefggedthfejgfevhfetgeekjedtvdfhveenucfkphepgedurdeiiedrieejrdduudefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieejrdduudefpdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH] avutil/map: [WIP] Introduction
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>
Content-Type: multipart/mixed; boundary="===============4091039779597709093=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250424115248.GU4991@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============4091039779597709093==
Content-Type: multipart/signed; micalg=pgp-sha512;
protocol="application/pgp-signature"; boundary="LsZelgEU9iMdmmQM"
Content-Disposition: inline
--LsZelgEU9iMdmmQM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi
On Wed, Apr 23, 2025 at 11:16:13PM +0200, Marton Balint wrote:
>=20
>=20
> On Wed, 23 Apr 2025, Michael Niedermayer wrote:
>=20
> > Hi
> >=20
> > On Mon, Apr 21, 2025 at 09:55:33PM +0200, Marton Balint wrote:
> > >=20
> > >=20
> > > On Sun, 20 Apr 2025, Michael Niedermayer wrote:
[...]
> > > Because as far as I
> > > remember AVDictionary keeps key insertion order, and we even rely on =
this
> > > behaviour sometimes, so an ordered-by-compare-function list is likely=
not
> > > going to work as an instant drop-in replacement...
> >=20
> > What do you mean by "keeps key insertion order" ?
> > this isnt documented, or is it ?
> > In fact i dont think thats supposed to be guranteed by AVDictionary
> >=20
> > I think the order coming out of av_map_iterate() will match the
> > order elements where added.
> > Is that what you meant ?
>=20
> Yes, exactly. Admittedly not documented behaviour... Another thing is that
> this does not support AV_DICT_MULTIKEY if the same value is used multiple
> times. I am just saying that we should document these fundamental
> differences to avoid any suprises...
will document this difference
>=20
> >=20
> >=20
> >=20
> > >=20
> > > > + *
> > > > + * ---------- Creating AVMaps ------------------
> > > > + *
> > > > + * AVMap *map =3D av_map_alloc(strcmp, AV_MAP_CMP_CASE_SENSITIVE +=
AV_MAP_CMP_KEY, NULL, NULL);
> > > > + *
> > > > + * This creates a case sensitve string based map using strcmp(). I=
t will not allow
> > > > + * multiple entries with the same key.
> > > > + * or
> > > > + *
> > > > + * AVMap *map =3D av_map_alloc(av_map_strcmp_keyvalue, AV_MAP_CMP_=
CASE_SENSITIVE + AV_MAP_CMP_KEYVALUE, NULL, NULL);
> > > > + *
> > > > + * This is like the previous, but it will allow multiple entries w=
ith the same key
> > > > + * the difference here is that the compare function compares the v=
alue too when
> > > > + * the key is equal.
> > > > + * All entries in a map must always be different. So by comparing =
the value
> > > > + * too we can have multiple entries with the same key
> > > > + *
> > > > + * The remaining 2 pointers in av_map_alloc() are for a function c=
opying an element
> > > > + * and one for freeing it. That is only needed for complex objects=
, not for strings.
> > > > + *
> > > > + *
> > > > + * ----------- Adding entries -----------------
> > > > + *
> > > > + * av_map_add_strings(map, "cat", "neko", 0); // add new entry or =
do nothing
> > >=20
> > > What "or do nothing" means here? That it will not overwrite a key by
> > > default? This is a different semantics than AVDictionary, where you n=
eed to
> > > explicitly set DONT_OVERWRITE flag for such.
> >=20
> > yes, we can flip the default around if people prefer
> > I really picked this solely because i felt negated flags with "DONT" in=
their
> > name are suboptimal design
> >=20
> >=20
> > >=20
> > > I think we should use function names and flags similar to what we hav=
e in
> > > AVDictionary. Like av_map_set_strings() instead of av_map_add_strings=
(), or
> > > AV_MAP_DONT_OVERWRITE. So our users won't have to use different minds=
et for
> > > similar stuff.
> >=20
> > like i said i dont like the "DONT" flag, i think we should avoid such n=
ames
> > in new designs. Maybe AV_MAP_PRESERVE would be an alternative ?
>=20
> Flag names should be easy to understand and remember,
yes
> I don't really mind if
> it is is negated. And "dont overwrite" or "no overwrite" is the most
> expressive IMHO.
>=20
> >=20
> > av_map_set_strings() implies setting a key to a value. When this in rea=
lity is
> > more flexibl, depending on flags and setup
>=20
> But if it depends on flags if "add" or "set" make more sense, then I'd
> rather keep set, considering the primary goal for this is being the
> dictionary replacement...
"set" has a double meaning, As in Lists, Maps, Sets, ...
map_set, set_map, to me fall in the group of list_map, map_list set_list, l=
ist_set, ...
so iam a bit hesitant to change it to that
Ill sleep over these things, maybe i or someone else has some more genius i=
dea
>=20
> >=20
> >=20
> > >=20
> > > > + *
> > > > + * av_map_add_strings(map, "cat", "neko", AV_MAP_REPLACE); // add =
new entry or replace existing
> > > > + *
> > > > + *
> > > > + * ----------- Removing entries -----------------
> > > > + *
> > > > + * Removing entries does by default not rebuild the map. That is, =
while access will always
> > > > + * be O(log n) when n becomes smaller, memory consumption will not=
decrease until
> > > > + * AV_SET_ALLOW_REBUILD is used. Note if you use AV_SET_ALLOW_REBU=
ILD, all previously
> > > > + * returned elements become invalid.
> > > > + *
> > > > + * av_map_del(map, "cat", 0); // remove one entry matching "the ke=
y"
> > > > + *
> > > > + * av_map_del(map, "cat", AV_SET_ALLOW_REBUILD); // remove one ent=
ry matching "the key" and rebuild the map to re
> > >=20
> > > Do you specify a key, or a concatenated key + \0 + value? Or you can =
specify
> > > both?
> >=20
> > it depends on the flags (which select the compare function)
> > In fact it can be an arbitrary struct instead of a string, if the compa=
re
> > function during setup compares the specific struct.
> >=20
> > av_map_del(map, "cat\0katze", AV_MAP_CMP_KEYVALUE);
> > av_map_del(map, "cat", AV_MAP_CMP_KEY);
> >=20
> >=20
> > >=20
> > > In general I believe the public API should not use const char * for k=
eyvalue
> > > types, that would be very fragile. A string constant is not a valid
> > > concatenated keyvalue for example, and the compiler will not catch su=
ch
> > > isses. Therefore public functions should always use separate key and
> > > separate value parameters.
> >=20
> > about "const char *", we could use a typedef or struct or something
> >=20
> > completely droping keyvalue from public API is not an option because
> > AVMapEntry always provides you with a keyvalue already and requiring
> > to copy it on every use is dumb.
>=20
> But AVMapEntry has separete key and value pointers, even sizes, so I am n=
ot
> sure where you need to copy.
current case
here you have a valid keyvalue pointer IF the user specifies that its a key=
value
and not key.
no check, passing 1 pointer, no copy
what you suggest
here you have key pointer, value pointer, keylen and valuelen
now the user needs to strlen() his strings or keep track of their
length (easy to get that wrong btw)
pass 2 pointers and 2 length into functions. (thats 4 times the stuff)
and then the functions would do
if (key + keylen !=3D value) {
copy into temporary storage
} // an additional check and potential copy
What is the gain here?
its slower, its more complex, and there are more places to mess up
Whats the point in having an efficient format in teh map when we encourage
teh user to use a incompatible one that needs to be checked and converted o=
n each use.
thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
--LsZelgEU9iMdmmQM
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaAomDAAKCRBhHseHBAsP
q80uAJ9mns/R6f+zdLzDysVqSMElnC4v/ACdFaQ2HsaBNtPYAqMxQ2G8ibg3oRk=
=etjq
-----END PGP SIGNATURE-----
--LsZelgEU9iMdmmQM--
--===============4091039779597709093==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
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".
--===============4091039779597709093==--