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 424E04D3CC
for <ffmpegdev@gitmailbox.com>; Thu, 17 Apr 2025 20:12:52 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D705F687A43;
Thu, 17 Apr 2025 23:12:46 +0300 (EEST)
Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net
[217.70.183.194])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4FEA6687A43
for <ffmpeg-devel@ffmpeg.org>; Thu, 17 Apr 2025 23:12:40 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id 67C3F43153
for <ffmpeg-devel@ffmpeg.org>; Thu, 17 Apr 2025 20:12:39 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
s=gm1; t=1744920759;
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=v8Rl9BPMdOh24Sw8csoZW39NZ7ga7i4XzlSYLfNzNAw=;
b=VLftfQSQQwHmjuhSxVGs3X+wVUrPvCuKPbklPEjC9+3HQ/fMiycezlyxomVOt63gtXhzGp
cQ5/Nxx3wL0HZva2yRyZiTddVC6t50AVQzXvnSt/A8A+K43DUdAs0Eu9BnGchTtyopiW7A
ZEAbMY1v2zLZ/xwnRZhREsQ94hojOSdVkqRX7NugXtcI42mErGGJxClYs5VziklLpMBXiC
8wo8soMyDiolDsX9Q1Z00n3b72zJvCTKqmxKoP+NgneWGVKel/gm9AkG7axYDqHzy4fiYd
JVpJxxBiYJ9+afylJU+2t7A6z2foA2PR6clyiTrlWMKgUt1iKOUZgpVD6Ypn5w==
Date: Thu, 17 Apr 2025 22:12:38 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20250417201238.GU4991@pb2>
References: <20250417165241.3266643-1-michael@niedermayer.cc>
<aAFBGsbuU5SG2ma4@phare.normalesup.org>
MIME-Version: 1.0
In-Reply-To: <aAFBGsbuU5SG2ma4@phare.normalesup.org>
X-GND-State: clean
X-GND-Score: -70
X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvfedtudelucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeeigeektdejudffjefhteegjedtgeettefggedthfejgfevhfetgeekjedtvdfhveenucfkphepgedurdeiiedrieejrdduudefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieejrdduudefpdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH v3] libavutil: Add AVMap
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="===============4111055113297966815=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250417201238.GU4991@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============4111055113297966815==
Content-Type: multipart/signed; micalg=pgp-sha512;
protocol="application/pgp-signature"; boundary="meQYVMD4fd/+dJ9P"
Content-Disposition: inline
--meQYVMD4fd/+dJ9P
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi
On Thu, Apr 17, 2025 at 07:57:46PM +0200, Nicolas George wrote:
> Michael Niedermayer (HE12025-04-17):
> > +AVMap *av_map_new(AVMapCompareFunc cmp_keyvalue, int cmp_flags, AVMapC=
opyFunc copy, AVMapFreeFunc freef)
> > +{
> > + AVMap *s =3D av_mallocz(sizeof(*s));
> > + if (!s)
> > + return NULL;
> > +
> > + s->copy =3D copy;
> > + s->freef =3D freef;
> > +
> > + av_map_add_cmp_func(s, cmp_keyvalue, cmp_flags);
> > +
> > + return s;
> > +}
>=20
> If you do not want to do the version where dynamic allocation can be
> avoided, would you at least consider using _alloc() for this? It is only
> to letters more, it is consistent with the rest of the API and it would
> leave _new() available if somebody adds the lighter version.
ok, i will change teh name
>=20
> I really do not understand why you do not want to do this: it is a very
> useful low-hanging fruit, and it is quite elegant and fun to implement.
You can implement an av_map_new() variant thats on the stack
if thats what you want to have / to use / to do
once the code is in git
or maybe i will do it but there are many more interresting things
> Alas, you did not reply to my last round of arguments.
yes, mails take alot of time to read and reply to.
>=20
> > + * @param keyvalue_cmp compare function, will be passed the key + valu=
e concatenated.
>=20
> Please no. Pass the key and the value separately to the compare
> function.
thats neither efficient nor does it fit the existing APIs.
The value is most of the time not used and when used it is known exactly
where it is. after a string compare of the key thats equal the value locati=
on is known
or with arbitrary binary data the compare function knows the size of the da=
ta.
Passing redudant pointers just wastes cpu cycles
also existing APIs from av_tree strcmp() av_strcasecmp() and so on none of
which take 4 pointers from which they would ignore 2.
>=20
> > +/**
> > + * Add a compatible compare function to the map.
> > + * The function will later be selected by using AV_MAP_CMP_* flags
> > + *
> > + * Functions must be added from most specific to least specific, that =
is if a AVMap is build
> > + * with a case insensitive compare no case sensitive compare functions=
can be added. This is
> > + * to avoid building non functional AVMaps.
> > + *
> > + *
> > + * @see av_map_new
> > + *
> > + * @param cmp compare function to be added.
> > + * cmp(a,b) must return 0 or be equal to the previously add=
ed compare function for (a,b), if it returns 0 it also must do so for all
> > + * elements between a and b
> > + *
> > + * @param cmp_flags a combination of AV_MAP_CMP_*, note key/keyvalue a=
nd truncated vs non truncated
> > + * are mandatory to be specified
> > + *
> > + * @return a negative error code if the cmp_flags are illegal or unsup=
ported for this AVMap
> > + * If you know your flags are valid, then you dont need to che=
ck the return
> > + */
> > +int av_map_add_cmp_func(AVMap *m, AVMapCompareFunc cmp, int cmp_flags);
>=20
> This whole thing with multiple compare functions makes the code quite
> hard to understand. And not just the code: it makes the API itself hard
> to understand.
>=20
> I think the practical goal it server could be achieved in a simpler way.
>=20
> If I understand correctly, it servers to allow the equivalent of
> AV_DICT_IGNORE_SUFFIX and such. It would be simpler to have a flag to
> av_map_find() to let it return the first key >=3D to the searched key.
There is
AV_MAP_CMP_CASE_SENSITIVE
AV_MAP_CMP_CASE_INSENSITIVE
AV_MAP_CMP_KEY
AV_MAP_CMP_KEYVALUE
AV_MAP_CMP_TRUNCATED
AV_MAP_CMP_NON_TRUNCATED
and these are passed to av_map_get() no compare function.
the compare functions are needed purely for setup. And there
they are needed because of arbitrary data support. av_map cannot
know how to compare your data. (if its not strings)
but something like a av_map_find() that returns the closest
previous and next matching and mismatching entries is a good idea too.
But the existing code is much simpler:
AVMapEntry e =3D NULL;
while (e =3D av_map_get_multiple(set, e, "author", AV_MAP_CMP_CASE_INSE=
NSITIVE | AV_MAP_CMP_KEY) {
printf("Tag key:%s value:%s\n", e->key, e->value);
}
vs:
/**
*
* @param surroundings these are the 2 closest non matching and 2 farthe=
st matching entries
*/
const AVMapEntry *av_map_get_find(const AVMap *s, AVMapEntry *surroundi=
ngs[4], const char *keyvalue, int flags);
you would probably need something like this:
(which is alot uglier and probably contains bugs)
AVMapEntry *e4[4] =3D {NULL};
AVMapEntry *e =3D av_map_find(set, e4, "author", AV_MAP_CMP_KEY); //We =
hope that "author" is the first in the ordering of case insensitive strings
if (e4[2]) // we compare just the key so we can have multiple matches a=
nd need to take the first not any
e =3D e4[2];
if (!e)
e =3D e4[1]; //if we have no exact match we start with the next non=
match which may be matching with case insensitive compare
while(e) {
if (!av_strcasecmp(e->key, "author")) {
printf("Tag key:%s value:%s\n", e->key, e->value);
} else {
break;
}
memset(e4, 0, sizeof(e4));
av_map_find(set, e4, e->key, 0);
e =3D e4[1];
}
Also the code above will just give wrong results with no warning if the com=
pare
function the map is setup with is incompatible with
thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
--meQYVMD4fd/+dJ9P
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaAFgswAKCRBhHseHBAsP
q7/jAKCLc2ZObsdIfjiiMhUS1frBJgEOAwCfbsJMBny2oyzUDn/ZEia7c6jCSOU=
=5awg
-----END PGP SIGNATURE-----
--meQYVMD4fd/+dJ9P--
--===============4111055113297966815==
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".
--===============4111055113297966815==--