Hi On Mon, Apr 21, 2025 at 09:55:33PM +0200, Marton Balint wrote: > > > On Sun, 20 Apr 2025, Michael Niedermayer wrote: > > > Note, help is welcome. > > Time i spend on this, i cannot spend on other things > > > > Note2: i intend to push AVMap after the release unless the release > > ends up delayed alot for other reasons, theres no real reason > > to hurry here except that i seem to keep workig on it when > > people ask for some non trivial changes/improvments :) > > so dont ask, send patch yourself if its not a trivial change :)) > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > --- > > libavutil/map.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 86 insertions(+) > > > > diff --git a/libavutil/map.h b/libavutil/map.h > > index 8211a05ec8d..0d3f7eab9ac 100644 > > --- a/libavutil/map.h > > +++ b/libavutil/map.h > > @@ -31,6 +31,92 @@ > > #include "tree.h" > > > > /** > > + * @file > > + * > > + * AVMap is a simple and fast key -> value map. > > Is this intended to be an AVDictionary replacement?
Yes. In how far a "automatic" replacement is possible iam not sure > 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... 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 I think the order coming out of av_map_iterate() will match the order elements where added. Is that what you meant ? > > > + * > > + * ---------- Creating AVMaps ------------------ > > + * > > + * AVMap *map = 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(). It will > > not allow > > + * multiple entries with the same key. > > + * or > > + * > > + * AVMap *map = 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 with the > > same key > > + * the difference here is that the compare function compares the value 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 copying > > 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 > > What "or do nothing" means here? That it will not overwrite a key by > default? This is a different semantics than AVDictionary, where you need to > explicitly set DONT_OVERWRITE flag for such. 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 > > I think we should use function names and flags similar to what we have 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 mindset for > similar stuff. like i said i dont like the "DONT" flag, i think we should avoid such names in new designs. Maybe AV_MAP_PRESERVE would be an alternative ? av_map_set_strings() implies setting a key to a value. When this in reality is more flexibl, depending on flags and setup > > > + * > > + * 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_REBUILD, all > > previously > > + * returned elements become invalid. > > + * > > + * av_map_del(map, "cat", 0); // remove one entry matching "the key" > > + * > > + * av_map_del(map, "cat", AV_SET_ALLOW_REBUILD); // remove one entry > > matching "the key" and rebuild the map to re > > Do you specify a key, or a concatenated key + \0 + value? Or you can specify > both? it depends on the flags (which select the compare function) In fact it can be an arbitrary struct instead of a string, if the compare function during setup compares the specific struct. av_map_del(map, "cat\0katze", AV_MAP_CMP_KEYVALUE); av_map_del(map, "cat", AV_MAP_CMP_KEY); > > In general I believe the public API should not use const char * for keyvalue > types, that would be very fragile. A string constant is not a valid > concatenated keyvalue for example, and the compiler will not catch such > isses. Therefore public functions should always use separate key and > separate value parameters. about "const char *", we could use a typedef or struct or something 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. so we need to find a smarter solution here thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
