[FFmpeg-devel] [PATCH 1/2] avutil/map: clearer and shorter compare flags
Michael Niedermayer
michael at niedermayer.cc
Thu Apr 24 14:54:07 EEST 2025
The new flags allow specifying the case sensitivity of key and value independently
The are shorter
0 now represents key never keyvalue reducing the risk for accidential overread
Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
---
libavutil/map.c | 60 +++++++++++++++++++++++++------------------
libavutil/map.h | 30 +++++++++++++++-------
libavutil/tests/map.c | 22 ++++++++--------
3 files changed, 67 insertions(+), 45 deletions(-)
diff --git a/libavutil/map.c b/libavutil/map.c
index af1ca61054e..795a8ba24d8 100644
--- a/libavutil/map.c
+++ b/libavutil/map.c
@@ -36,7 +36,7 @@ typedef struct{
struct AVMap{
#define CMP_MASK 31
- AVMapCompareFunc cmp[27];
+ AVMapCompareFunc cmp[36];
AVMapCopyFunc copy;
AVMapFreeFunc freef;
size_t count;
@@ -101,29 +101,34 @@ int av_map_supercmp_keyvalue(const char *a, const char *b)
int av_map_add_cmp_func(AVMap *m, AVMapCompareFunc cmp, int cmp_flags)
{
- if (cmp_flags >= 27U)
+ if (cmp_flags >= 36U)
return AVERROR(EINVAL);
- static const uint8_t sensitivity[27][3] = {
- {0,0, 0},{1,0, 0},{2,0, 0}, {0,3, 0},{1,3, 0},{2,3, 0}, {0,6, 0},{1,6, 0},{2,6, 0},
- {0,0, 9},{1,0, 9},{2,0, 9}, {0,3, 9},{1,3, 9},{2,3, 9}, {0,6, 9},{1,6, 9},{2,6, 9},
- {0,0,18},{1,0,18},{2,0,18}, {0,3,18},{1,3,18},{2,3,18}, {0,6,18},{1,6,18},{2,6,18},};
- int case_sensitive = sensitivity[cmp_flags][0];
- int keyvalue_sensitive = sensitivity[cmp_flags][1];
+ static const uint8_t sensitivity[36][3] = {
+ {0,0, 0},{1,0, 0},{2,0, 0}, {0,3, 0},{1,3, 0},{2,3, 0}, {0,6, 0},{1,6, 0},{2,6, 0}, {0,9, 0},{1,9, 0},{2,9, 0},
+ {0,0,12},{1,0,12},{2,0,12}, {0,3,12},{1,3,12},{2,3,12}, {0,6,12},{1,6,12},{2,6,12}, {0,9,12},{1,9,12},{2,9,12},
+ {0,0,24},{1,0,24},{2,0,24}, {0,3,24},{1,3,24},{2,3,24}, {0,6,24},{1,6,24},{2,6,24}, {0,9,24},{1,9,24},{2,9,24},
+ };
+
+ int key_case_sensitive = sensitivity[cmp_flags][0];
+ int value_case_sensitive = sensitivity[cmp_flags][1];
int truncated_sensitive = sensitivity[cmp_flags][2];
- if (!keyvalue_sensitive)
- return AVERROR(EINVAL);
+ av_assert1(key_case_sensitive + value_case_sensitive + truncated_sensitive == cmp_flags);
- av_assert1(case_sensitive + keyvalue_sensitive + truncated_sensitive == cmp_flags);
if (!truncated_sensitive)
truncated_sensitive = AV_MAP_CMP_NON_TRUNCATED;
- if ( case_sensitive == AV_MAP_CMP_CASE_SENSITIVE && m->cmp[keyvalue_sensitive + AV_MAP_CMP_CASE_INSENSITIVE])
+ if (key_case_sensitive == AV_MAP_CMP_KEY_S && m->cmp[value_case_sensitive + AV_MAP_CMP_KEY_I])
return AVERROR(EINVAL);
- if ( keyvalue_sensitive == AV_MAP_CMP_KEYVALUE && m->cmp[AV_MAP_CMP_KEY])
+
+ if (value_case_sensitive == AV_MAP_CMP_KEYVALUE__S && m->cmp[key_case_sensitive + AV_MAP_CMP_KEYVALUE__I])
return AVERROR(EINVAL);
- if (truncated_sensitive == AV_MAP_CMP_NON_TRUNCATED && m->cmp[keyvalue_sensitive + AV_MAP_CMP_TRUNCATED])
+
+ if (value_case_sensitive && m->cmp[key_case_sensitive])
+ return AVERROR(EINVAL);
+
+ if (truncated_sensitive == AV_MAP_CMP_NON_TRUNCATED && m->cmp[key_case_sensitive + value_case_sensitive + AV_MAP_CMP_TRUNCATED])
return AVERROR(EINVAL);
//max functions is KV NT CS -> KV NT CI -> KV T CI (CI/T is about value only) -> K NT CS -> K NT CI -> K T CI
@@ -131,8 +136,13 @@ int av_map_add_cmp_func(AVMap *m, AVMapCompareFunc cmp, int cmp_flags)
for (int i=0; i<8; i++) {
int flags = 0;
- if (i&1) flags += case_sensitive;
- if (i&2) flags += keyvalue_sensitive;
+
+ if (i&1) flags += key_case_sensitive;
+
+ if (i&2) flags += AV_MAP_CMP_KEYVALUE; // key compare is always usable as keyvalue compare
+ else if (value_case_sensitive) {
+ flags += value_case_sensitive;
+ }
if (i&4) flags += truncated_sensitive;
if (!m->cmp[flags])
@@ -143,7 +153,7 @@ int av_map_add_cmp_func(AVMap *m, AVMapCompareFunc cmp, int cmp_flags)
int av_map_is_cmp_flags_supported(AVMap *m, int cmp_flags)
{
- if (cmp_flags >= 27U)
+ if (cmp_flags >= 36U)
return AVERROR(EINVAL);
return !!m->cmp[cmp_flags];
}
@@ -184,7 +194,7 @@ const AVMapEntry *av_map_get_multiple(const AVMap *s, const AVMapEntry *prev, co
if (prev) {
void *next_node[2] = { NULL, NULL };
- void *prev_keyvalue = av_tree_find2(s->tree_root, prev->key, s->cmp[0], next_node, 2);
+ void *prev_keyvalue = av_tree_find2(s->tree_root, prev->key, s->cmp[AV_MAP_CMP_KEYVALUE], next_node, 2);
av_assert2(prev_keyvalue);
if (!next_node[1] || cmp(next_node[1], keyvalue))
return NULL;
@@ -270,15 +280,15 @@ int av_map_add(AVMap *s, const char *key, size_t keylen, const char *value, size
memcpy(entry->key , key , keylen);
memcpy(entry->value, value, valuelen);
- void *elem = av_tree_insert(&s->tree_root, entry->key, s->cmp[0], &next);
+ void *elem = av_tree_insert(&s->tree_root, entry->key, s->cmp[AV_MAP_CMP_KEYVALUE], &next);
int ret = 1;
if (elem != entry->key && elem) {
av_assert2(next);
//we assume that new entries are more common than replacements
if (flags & AV_MAP_REPLACE) {
- ret = av_map_del(s, entry->key, flags & ~CMP_MASK);
+ ret = av_map_del(s, entry->key, (flags & ~CMP_MASK) + AV_MAP_CMP_KEYVALUE);
av_assert2(ret == 1);
- elem = av_tree_insert(&s->tree_root, entry->key, s->cmp[0], &next);
+ elem = av_tree_insert(&s->tree_root, entry->key, s->cmp[AV_MAP_CMP_KEYVALUE], &next);
av_assert2(elem == entry->key || !elem);
ret = 2;
} else
@@ -303,7 +313,7 @@ int av_map_del(AVMap *s, const char *keyvalue, int flags)
AVTreeNode *next = NULL;
AVMapCompareFunc cmp = s->cmp[flags & CMP_MASK];
- if (cmp != s->cmp[0]) {
+ if (cmp != s->cmp[AV_MAP_CMP_KEYVALUE]) {
// The user asks us to remove a entry with a compare function different from the one used to build the map
// we need to do 2 calls here, first with the users compare to find the entry she wants to remove
// and then to remove it while maintaining the correct order within the map
@@ -311,10 +321,10 @@ int av_map_del(AVMap *s, const char *keyvalue, int flags)
if (!old_keyvalue)
return 0;
- av_tree_insert(&s->tree_root, old_keyvalue, s->cmp[0], &next);
+ av_tree_insert(&s->tree_root, old_keyvalue, s->cmp[AV_MAP_CMP_KEYVALUE], &next);
av_assert2(next);
} else {
- av_tree_insert(&s->tree_root, (char*)keyvalue, s->cmp[0], &next);
+ av_tree_insert(&s->tree_root, (char*)keyvalue, s->cmp[AV_MAP_CMP_KEYVALUE], &next);
if (!next)
return 0;
old_keyvalue = next->elem; //TODO add a API to av_tree() to return the elem of a AVTreeNode
@@ -327,7 +337,7 @@ int av_map_del(AVMap *s, const char *keyvalue, int flags)
s->deleted++;
if ((flags & AV_MAP_ALLOW_REBUILD) && s->deleted > s->count) {
- AVMap *news = av_map_alloc(s->cmp[0], AV_MAP_CMP_KEYVALUE + AV_MAP_CMP_NON_TRUNCATED, s->copy, s->freef);
+ AVMap *news = av_map_alloc(s->cmp[AV_MAP_CMP_KEYVALUE], AV_MAP_CMP_KEYVALUE + AV_MAP_CMP_NON_TRUNCATED, s->copy, s->freef);
if(news) {
memcpy(news->cmp, s->cmp, sizeof(news->cmp));
int ret = av_map_copy(news, s);
diff --git a/libavutil/map.h b/libavutil/map.h
index 2784bde0ae0..61c64900ace 100644
--- a/libavutil/map.h
+++ b/libavutil/map.h
@@ -37,13 +37,13 @@
*
* ---------- Creating AVMaps ------------------
*
- * AVMap *map = av_map_alloc(strcmp, AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEY, NULL, NULL);
+ * AVMap *map = av_map_alloc(strcmp, AV_MAP_CMP_KEY_S, 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);
+ * AVMap *map = av_map_alloc(av_map_strcmp_keyvalue, AV_MAP_CMP_KEYVALUE_SS, 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
@@ -78,7 +78,7 @@
*
* AVMapEntry *e = av_map_get(map, "cat", AV_MAP_CMP_KEY); //Find an entry with the key = "cat"
*
- * AVMapEntry *e = av_map_get(map, "cat", AV_MAP_CMP_KEY+AV_MAP_CMP_CASE_INSENSITIVE); //Find an entry with the key = "cat", "Cat", "cAt", ...
+ * AVMapEntry *e = av_map_get(map, "cat", AV_MAP_CMP_KEY_I); //Find an entry with the key = "cat", "Cat", "cAt", ...
* // this will only work if one of the set compare functions is case insensitive
*
*
@@ -133,12 +133,24 @@
enum {
//use + not | to combine these flags
- AV_MAP_CMP_CASE_SENSITIVE = 1,
- AV_MAP_CMP_CASE_INSENSITIVE = 2,
- AV_MAP_CMP_KEY = 3,
- AV_MAP_CMP_KEYVALUE = 6,
- AV_MAP_CMP_TRUNCATED = 9,
- AV_MAP_CMP_NON_TRUNCATED = 18,
+ AV_MAP_CMP_KEY = 0, ///< key compare
+ AV_MAP_CMP_KEY_I = 1, ///< case insensitive key compare
+ AV_MAP_CMP_KEY_S = 2, ///< case sensitive key compare
+
+ AV_MAP_CMP_KEYVALUE = 3, ///< key and value compare
+ AV_MAP_CMP_KEYVALUE_I_ = 3+1, ///< case insensitive key and value compare
+ AV_MAP_CMP_KEYVALUE_S_ = 3+2, ///< case sensitive key and value compare
+
+ AV_MAP_CMP_KEYVALUE__I = 6, ///< key and case insensitive value compare
+ AV_MAP_CMP_KEYVALUE_II = 6+1, ///< case insensitive key and case insensitive value compare
+ AV_MAP_CMP_KEYVALUE_SI = 6+2, ///< case sensitive key and case insensitive value compare
+
+ AV_MAP_CMP_KEYVALUE__S = 9, ///< key and case sensitive value compare
+ AV_MAP_CMP_KEYVALUE_IS = 9+1, ///< case insensitive key and case sensitive value compare
+ AV_MAP_CMP_KEYVALUE_SS = 9+2, ///< case sensitive key and case sensitive value compare
+
+ AV_MAP_CMP_TRUNCATED = 12,
+ AV_MAP_CMP_NON_TRUNCATED = 24,
AV_MAP_ALLOW_REBUILD = 256, ///< when removing entries rebuild the map to reduce memory consumption, note, this invalidates previously retrieved elements and iterate state.
AV_MAP_REPLACE = 512, ///< replace keyvalue if already in the map
diff --git a/libavutil/tests/map.c b/libavutil/tests/map.c
index e43f0f05db4..e1412e0dfc8 100644
--- a/libavutil/tests/map.c
+++ b/libavutil/tests/map.c
@@ -48,11 +48,11 @@ int main(void)
av_map_supercmp_keyvalue,
};
int our_flags[] = {
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEYVALUE,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_INSENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEYVALUE,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEYVALUE,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_S,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEYVALUE_SS,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_I,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEYVALUE_SS,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEYVALUE_SS,
};
void *our_subcmp[] = {
strcmp,
@@ -62,11 +62,11 @@ int main(void)
av_strcasecmp,
};
int our_subflags[] = {
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_INSENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_SENSITIVE + AV_MAP_CMP_KEY,
- AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_CASE_INSENSITIVE + AV_MAP_CMP_KEY,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_S,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_S,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_I,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_S,
+ AV_MAP_CMP_NON_TRUNCATED + AV_MAP_CMP_KEY_I,
};
for (int settype=0; settype<3; settype++) {
@@ -224,7 +224,7 @@ int main(void)
#define P 4
fprintf(stderr, "%d entries variable bytes at location %d-%d\n", N_ENTRIES, P, P+1);
for (int runs = 0; runs < 1000; runs++) {
- AVMap *map = av_map_alloc(av_strcasecmp, AV_MAP_CMP_CASE_INSENSITIVE+AV_MAP_CMP_KEYVALUE+AV_MAP_CMP_NON_TRUNCATED, NULL, NULL);
+ AVMap *map = av_map_alloc(av_strcasecmp, AV_MAP_CMP_KEY_I+AV_MAP_CMP_NON_TRUNCATED, NULL, NULL);
for(int pass = 0; pass < 2; pass++) {
START_TIMER
unsigned r = 5;
--
2.49.0
More information about the ffmpeg-devel
mailing list