[FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[Connor Worley]

Signed-off-by: Connor Worley <connorbworley@gmail.com>
---
 libavutil/Makefile          |   2 +
 libavutil/hashtable.c       | 183 ++++++++++++++++++++++++++++++++++++
 libavutil/hashtable.h       |  47 +++++++++
 libavutil/tests/hashtable.c | 108 +++++++++++++++++++++
 4 files changed, 340 insertions(+)
 create mode 100644 libavutil/hashtable.c
 create mode 100644 libavutil/hashtable.h
 create mode 100644 libavutil/tests/hashtable.c

diff --git a/libavutil/Makefile b/libavutil/Makefile
index e7709b97d0..be75d464fc 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -138,6 +138,7 @@ OBJS = adler32.o                                                        \
        fixed_dsp.o                                                      \
        frame.o                                                          \
        hash.o                                                           \
+       hashtable.o                                                      \
        hdr_dynamic_metadata.o                                           \
        hdr_dynamic_vivid_metadata.o                                     \
        hmac.o                                                           \
@@ -251,6 +252,7 @@ TESTPROGS = adler32                                                     \
             file                                                        \
             fifo                                                        \
             hash                                                        \
+            hashtable                                                   \
             hmac                                                        \
             hwdevice                                                    \
             integer                                                     \
diff --git a/libavutil/hashtable.c b/libavutil/hashtable.c
new file mode 100644
index 0000000000..6fa2af8af3
--- /dev/null
+++ b/libavutil/hashtable.c
@@ -0,0 +1,183 @@
+/*
+ * Generic hashtable
+ * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdint.h>
+#include <string.h>
+
+#include "crc.h"
+#include "error.h"
+#include "mem.h"
+#include "hashtable.h"
+
+struct AVHashtableContext {
+    size_t key_size;
+    size_t val_size;
+    size_t entry_size;
+    size_t max_entries;
+    size_t utilization;
+    const AVCRC *crc;
+    uint8_t *table;
+    uint8_t *swapbuf;
+};
+
+#define ENTRY_PSL(entry) (entry)
+#define ENTRY_OCC(entry) (ENTRY_PSL(entry) + sizeof(size_t))
+#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 4)
+#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + FFALIGN(ctx->key_size, 4))
+
+#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size)
+
+int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries)
+{
+    struct AVHashtableContext *res = av_malloc(sizeof(struct AVHashtableContext));
+    if (!res)
+        return AVERROR(ENOMEM);
+    res->key_size = key_size;
+    res->val_size = val_size;
+    res->entry_size = sizeof(size_t) + 4 + FFALIGN(key_size, 4) + FFALIGN(val_size, 4);
+    res->max_entries = max_entries;
+    res->utilization = 0;
+    res->crc = av_crc_get_table(AV_CRC_32_IEEE);
+    if (!res->crc) {
+        av_hashtable_freep(&res);
+        return AVERROR_BUG;
+    }
+    res->table = av_calloc(res->max_entries, res->entry_size);
+    if (!res->table) {
+        av_hashtable_freep(&res);
+        return AVERROR(ENOMEM);
+    }
+    res->swapbuf = av_calloc(2, FFALIGN(key_size, 4) + FFALIGN(val_size, 4));
+    if (!res->swapbuf) {
+        av_hashtable_freep(&res);
+        return AVERROR(ENOMEM);
+    }
+    *ctx = res;
+    return 0;
+}
+
+static size_t hash_key(const struct AVHashtableContext* ctx, const void* key)
+{
+    return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries;
+}
+
+int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val)
+{
+    size_t hash = hash_key(ctx, key);
+
+    if (!ctx->utilization)
+        return 0;
+
+    for (size_t psl = 0; psl < ctx->max_entries; psl++) {
+        size_t wrapped_index = (hash + psl) % ctx->max_entries;
+        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
+        if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
+            return 0;
+        if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
+            memcpy(val, ENTRY_VAL(entry), ctx->val_size);
+            return 1;
+        }
+    }
+    return 0;
+}
+
+int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val)
+{
+    int swapping = 0;
+    size_t psl = 0;
+    size_t hash = hash_key(ctx, key);
+    uint8_t *set = ctx->swapbuf;
+    uint8_t *tmp = ctx->swapbuf + FFALIGN(ctx->key_size, 4) + FFALIGN(ctx->val_size, 4);
+
+    memcpy(set, key, ctx->key_size);
+    memcpy(set + FFALIGN(ctx->key_size, 4), val, ctx->val_size);
+
+    for (size_t i = 0; i < ctx->max_entries; i++) {
+        size_t wrapped_index = (hash + i) % ctx->max_entries;
+        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
+        if (!*ENTRY_OCC(entry) || (!swapping && KEYS_EQUAL(ENTRY_KEY(entry), set))) {
+            if (!*ENTRY_OCC(entry))
+                ctx->utilization++;
+            *(size_t*)ENTRY_PSL(entry) = psl;
+            *ENTRY_OCC(entry) = 1;
+            memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
+            return 1;
+        }
+        if (*(size_t*)ENTRY_PSL(entry) < psl) {
+            if (ctx->utilization == ctx->max_entries)
+                return 0;
+            swapping = 1;
+            // set needs to swap with entry
+            memcpy(tmp, ENTRY_KEY(entry), FFALIGN(ctx->key_size, 4) + ctx->val_size);
+            memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
+            FFSWAP(uint8_t*, set, tmp);
+            FFSWAP(size_t, psl, *(size_t*)ENTRY_PSL(entry));
+        }
+        psl++;
+    }
+    return 0;
+}
+
+int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key)
+{
+    uint8_t *next_entry;
+    size_t hash = hash_key(ctx, key);
+
+    if (!ctx->utilization)
+        return 0;
+
+    for (size_t psl = 0; psl < ctx->max_entries; psl++) {
+        size_t wrapped_index = (hash + psl) % ctx->max_entries;
+        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
+        if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
+            return 0;
+        if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
+            *ENTRY_OCC(entry) = 0;
+            for (psl++; psl < ctx->max_entries; psl++) {
+                wrapped_index = (hash + psl) % ctx->max_entries;
+                next_entry = ctx->table + wrapped_index * ctx->entry_size;
+                if (!*ENTRY_OCC(next_entry) || !*(size_t*)ENTRY_PSL(next_entry)) {
+                    ctx->utilization--;
+                    return 1;
+                }
+                memcpy(ENTRY_OCC(entry), ENTRY_OCC(next_entry), ctx->entry_size);
+                *(size_t*)ENTRY_PSL(entry)--;
+                *ENTRY_OCC(next_entry) = 0;
+                entry = next_entry;
+            }
+        }
+    };
+    return 0;
+}
+
+void av_hashtable_clear(struct AVHashtableContext* ctx)
+{
+    memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
+}
+
+void av_hashtable_freep(struct AVHashtableContext** ctx)
+{
+    if (*ctx) {
+        av_freep(&(*ctx)->table);
+        av_freep(&(*ctx)->swapbuf);
+    }
+    av_freep(ctx);
+}
diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h
new file mode 100644
index 0000000000..90c6cad68a
--- /dev/null
+++ b/libavutil/hashtable.h
@@ -0,0 +1,47 @@
+/*
+ * Generic hashtable
+ * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_HASHTABLE_H
+#define AVUTIL_HASHTABLE_H
+
+#include <stddef.h>
+
+/* Implements a hash table using Robin Hood open addressing.
+ * See: https://cs.uwaterloo.ca/research/tr/1986/CS-86-14.pdf
+ *
+ * Entry slots are layed out in memory as follows:
+ * - [1 byte] occupancy flag that indicates whether a slot if occupied.
+ * - [key_size bytes] entry key.
+ * - [val_size bytes] entry value.
+ * - [sizeof(size_t) bytes] PSL (probe sequence length), used by the Robin Hood
+ *   algorithm.
+ */
+
+struct AVHashtableContext;
+
+int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries);
+int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val);
+int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val);
+int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key);
+void av_hashtable_clear(struct AVHashtableContext* ctx);
+void av_hashtable_freep(struct AVHashtableContext** ctx);
+
+#endif
diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c
new file mode 100644
index 0000000000..0b38b34c5c
--- /dev/null
+++ b/libavutil/tests/hashtable.c
@@ -0,0 +1,108 @@
+/*
+ * Generic hashtable tests
+ * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/avassert.h"
+#include "libavutil/hashtable.h"
+
+int main(void)
+{
+    struct AVHashtableContext *ctx;
+    uint8_t k;
+    uint64_t v;
+
+    // impossibly large allocation should fail gracefully
+    av_assert0(av_hashtable_alloc(&ctx, -1, -1, -1) < 0);
+
+    // hashtable can store up to 3 uint8_t->uint64_t entries
+    av_assert0(!av_hashtable_alloc(&ctx, sizeof(k), sizeof(v), 3));
+
+    // unsuccessful deletes return 0
+    k = 1;
+    av_assert0(!av_hashtable_delete(ctx, &k));
+
+    // unsuccessful gets return 0
+    k = 1;
+    av_assert0(!av_hashtable_get(ctx, &k, &v));
+
+    // successful sets returns 1
+    k = 1;
+    v = 1;
+    av_assert0(av_hashtable_set(ctx, &k, &v));
+
+    // get should now contain 1
+    k = 1;
+    v = 0;
+    av_assert0(av_hashtable_get(ctx, &k, &v));
+    av_assert0(v == 1);
+
+    // updating sets should return 1
+    k = 1;
+    v = 2;
+    av_assert0(av_hashtable_set(ctx, &k, &v));
+
+    // get should now contain 2
+    k = 1;
+    v = 0;
+    av_assert0(av_hashtable_get(ctx, &k, &v));
+    av_assert0(v == 2);
+
+    // fill the table
+    k = 2;
+    v = 2;
+    av_assert0(av_hashtable_set(ctx, &k, &v));
+    k = 3;
+    v = 3;
+    av_assert0(av_hashtable_set(ctx, &k, &v));
+
+    // inserting sets on a full table should return 0
+    k = 4;
+    v = 4;
+    av_assert0(!av_hashtable_set(ctx, &k, &v));
+
+    // updating sets on a full table should return 1
+    k = 1;
+    v = 4;
+    av_assert0(av_hashtable_set(ctx, &k, &v));
+    v = 0;
+    av_assert0(av_hashtable_get(ctx, &k, &v));
+    av_assert0(v == 4);
+
+    // successful deletes should return 1
+    k = 1;
+    av_assert0(av_hashtable_delete(ctx, &k));
+
+    // get should now return 0
+    av_assert0(!av_hashtable_get(ctx, &k, &v));
+
+    // sanity check remaining keys
+    k = 2;
+    v = 0;
+    av_assert0(av_hashtable_get(ctx, &k, &v));
+    av_assert0(v == 2);
+    k = 3;
+    v = 0;
+    av_assert0(av_hashtable_get(ctx, &k, &v));
+    av_assert0(v == 3);
+
+    av_hashtable_freep(&ctx);
+
+    return 0;
+}
-- 
2.40.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH v2 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable

[Connor Worley]

Offers a modest performance gain due to the switch from naive linear
probling to robin hood.

Signed-off-by: Connor Worley <connorbworley@gmail.com>
---
 libavcodec/dxvenc.c | 121 ++++++++++++--------------------------------
 1 file changed, 33 insertions(+), 88 deletions(-)

diff --git a/libavcodec/dxvenc.c b/libavcodec/dxvenc.c
index b274175689..ebc48aace3 100644
--- a/libavcodec/dxvenc.c
+++ b/libavcodec/dxvenc.c
@@ -21,7 +21,7 @@
 
 #include <stdint.h>
 
-#include "libavutil/crc.h"
+#include "libavutil/hashtable.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
 
@@ -37,76 +37,13 @@
  * appeared in the decompressed stream. Using a simple hash table (HT)
  * significantly speeds up the lookback process while encoding.
  */
-#define LOOKBACK_HT_ELEMS 0x40000
+#define LOOKBACK_HT_ELEMS 0x20202
 #define LOOKBACK_WORDS    0x20202
 
 enum DXVTextureFormat {
     DXV_FMT_DXT1 = MKBETAG('D', 'X', 'T', '1'),
 };
 
-typedef struct HTEntry {
-    uint32_t key;
-    uint32_t pos;
-} HTEntry;
-
-static void ht_init(HTEntry *ht)
-{
-    for (size_t i = 0; i < LOOKBACK_HT_ELEMS; i++) {
-        ht[i].pos = -1;
-    }
-}
-
-static uint32_t ht_lookup_and_upsert(HTEntry *ht, const AVCRC *hash_ctx,
-                                    uint32_t key, uint32_t pos)
-{
-    uint32_t ret = -1;
-    size_t hash = av_crc(hash_ctx, 0, (uint8_t*)&key, 4) % LOOKBACK_HT_ELEMS;
-    for (size_t i = hash; i < hash + LOOKBACK_HT_ELEMS; i++) {
-        size_t wrapped_index = i % LOOKBACK_HT_ELEMS;
-        HTEntry *entry = &ht[wrapped_index];
-        if (entry->key == key || entry->pos == -1) {
-            ret = entry->pos;
-            entry->key = key;
-            entry->pos = pos;
-            break;
-        }
-    }
-    return ret;
-}
-
-static void ht_delete(HTEntry *ht, const AVCRC *hash_ctx,
-                      uint32_t key, uint32_t pos)
-{
-    HTEntry *removed_entry = NULL;
-    size_t removed_hash;
-    size_t hash = av_crc(hash_ctx, 0, (uint8_t*)&key, 4) % LOOKBACK_HT_ELEMS;
-
-    for (size_t i = hash; i < hash + LOOKBACK_HT_ELEMS; i++) {
-        size_t wrapped_index = i % LOOKBACK_HT_ELEMS;
-        HTEntry *entry = &ht[wrapped_index];
-        if (entry->pos == -1)
-            return;
-        if (removed_entry) {
-            size_t candidate_hash = av_crc(hash_ctx, 0, (uint8_t*)&entry->key, 4) % LOOKBACK_HT_ELEMS;
-            if ((wrapped_index > removed_hash && (candidate_hash <= removed_hash || candidate_hash > wrapped_index)) ||
-                (wrapped_index < removed_hash && (candidate_hash <= removed_hash && candidate_hash > wrapped_index))) {
-                *removed_entry = *entry;
-                entry->pos = -1;
-                removed_entry = entry;
-                removed_hash = wrapped_index;
-            }
-        } else if (entry->key == key) {
-            if (entry->pos <= pos) {
-                entry->pos = -1;
-                removed_entry = entry;
-                removed_hash = wrapped_index;
-            } else {
-                return;
-            }
-        }
-    }
-}
-
 typedef struct DXVEncContext {
     AVClass *class;
 
@@ -123,10 +60,8 @@ typedef struct DXVEncContext {
     enum DXVTextureFormat tex_fmt;
     int (*compress_tex)(AVCodecContext *avctx);
 
-    const AVCRC *crc_ctx;
-
-    HTEntry color_lookback_ht[LOOKBACK_HT_ELEMS];
-    HTEntry lut_lookback_ht[LOOKBACK_HT_ELEMS];
+    struct AVHashtableContext *color_ht;
+    struct AVHashtableContext *lut_ht;
 } DXVEncContext;
 
 /* Converts an index offset value to a 2-bit opcode and pushes it to a stream.
@@ -161,27 +96,32 @@ static int dxv_compress_dxt1(AVCodecContext *avctx)
     DXVEncContext *ctx = avctx->priv_data;
     PutByteContext *pbc = &ctx->pbc;
     uint32_t *value;
-    uint32_t color, lut, idx, color_idx, lut_idx, prev_pos, state = 16, pos = 2, op = 0;
+    uint32_t color, lut, idx, color_idx, lut_idx, prev_pos, state = 16, pos = 0, op = 0;
 
-    ht_init(ctx->color_lookback_ht);
-    ht_init(ctx->lut_lookback_ht);
+    av_hashtable_clear(ctx->color_ht);
+    av_hashtable_clear(ctx->lut_ht);
 
     bytestream2_put_le32(pbc, AV_RL32(ctx->tex_data));
+    av_hashtable_set(ctx->color_ht, ctx->tex_data, &pos);
+    pos++;
     bytestream2_put_le32(pbc, AV_RL32(ctx->tex_data + 4));
-
-    ht_lookup_and_upsert(ctx->color_lookback_ht, ctx->crc_ctx, AV_RL32(ctx->tex_data), 0);
-    ht_lookup_and_upsert(ctx->lut_lookback_ht, ctx->crc_ctx, AV_RL32(ctx->tex_data + 4), 1);
+    av_hashtable_set(ctx->lut_ht, ctx->tex_data + 4, &pos);
+    pos++;
 
     while (pos + 2 <= ctx->tex_size / 4) {
         idx = 0;
+        color_idx = 0;
+        lut_idx = 0;
 
         color = AV_RL32(ctx->tex_data + pos * 4);
-        prev_pos = ht_lookup_and_upsert(ctx->color_lookback_ht, ctx->crc_ctx, color, pos);
-        color_idx = prev_pos != -1 ? pos - prev_pos : 0;
+        if (av_hashtable_get(ctx->color_ht, &color, &prev_pos))
+            color_idx = pos - prev_pos;
+        av_hashtable_set(ctx->color_ht, &color, &pos);
+
         if (pos >= LOOKBACK_WORDS) {
             uint32_t old_pos = pos - LOOKBACK_WORDS;
-            uint32_t old_color = AV_RL32(ctx->tex_data + old_pos * 4);
-            ht_delete(ctx->color_lookback_ht, ctx->crc_ctx, old_color, old_pos);
+            if (av_hashtable_get(ctx->color_ht, ctx->tex_data + old_pos * 4, &prev_pos) && prev_pos <= old_pos)
+                av_hashtable_delete(ctx->color_ht, ctx->tex_data + old_pos * 4);
         }
         pos++;
 
@@ -190,13 +130,14 @@ static int dxv_compress_dxt1(AVCodecContext *avctx)
             idx = color_idx;
         } else {
             idx = 0;
-            prev_pos = ht_lookup_and_upsert(ctx->lut_lookback_ht, ctx->crc_ctx, lut, pos);
-            lut_idx = prev_pos != -1 ? pos - prev_pos : 0;
+            if (av_hashtable_get(ctx->lut_ht, &lut, &prev_pos))
+                lut_idx = pos - prev_pos;
+            av_hashtable_set(ctx->lut_ht, &lut, &pos);
         }
         if (pos >= LOOKBACK_WORDS) {
             uint32_t old_pos = pos - LOOKBACK_WORDS;
-            uint32_t old_lut = AV_RL32(ctx->tex_data + old_pos * 4);
-            ht_delete(ctx->lut_lookback_ht, ctx->crc_ctx, old_lut, old_pos);
+            if (av_hashtable_get(ctx->lut_ht, ctx->tex_data + old_pos * 4, &prev_pos) && prev_pos <= old_pos)
+                av_hashtable_delete(ctx->lut_ht, ctx->tex_data + old_pos * 4);
         }
         pos++;
 
@@ -298,11 +239,12 @@ static av_cold int dxv_init(AVCodecContext *avctx)
         return AVERROR(ENOMEM);
     }
 
-    ctx->crc_ctx = av_crc_get_table(AV_CRC_32_IEEE);
-    if (!ctx->crc_ctx) {
-        av_log(avctx, AV_LOG_ERROR, "Could not initialize CRC table.\n");
-        return AVERROR_BUG;
-    }
+    ret = av_hashtable_alloc(&ctx->color_ht, 4, 4, LOOKBACK_HT_ELEMS);
+    if (ret < 0)
+        return ret;
+    ret = av_hashtable_alloc(&ctx->lut_ht, 4, 4, LOOKBACK_HT_ELEMS);
+    if (ret < 0)
+        return ret;
 
     return 0;
 }
@@ -313,6 +255,9 @@ static av_cold int dxv_close(AVCodecContext *avctx)
 
     av_freep(&ctx->tex_data);
 
+    av_hashtable_freep(&ctx->color_ht);
+    av_hashtable_freep(&ctx->lut_ht);
+
     return 0;
 }
 
-- 
2.40.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[Andreas Rheinhardt]

Connor Worley:
> Signed-off-by: Connor Worley <connorbworley@gmail.com>
> ---
>  libavutil/Makefile          |   2 +
>  libavutil/hashtable.c       | 183 ++++++++++++++++++++++++++++++++++++
>  libavutil/hashtable.h       |  47 +++++++++
>  libavutil/tests/hashtable.c | 108 +++++++++++++++++++++
>  4 files changed, 340 insertions(+)
>  create mode 100644 libavutil/hashtable.c
>  create mode 100644 libavutil/hashtable.h
>  create mode 100644 libavutil/tests/hashtable.c
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index e7709b97d0..be75d464fc 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -138,6 +138,7 @@ OBJS = adler32.o                                                        \
>         fixed_dsp.o                                                      \
>         frame.o                                                          \
>         hash.o                                                           \
> +       hashtable.o                                                      \
>         hdr_dynamic_metadata.o                                           \
>         hdr_dynamic_vivid_metadata.o                                     \
>         hmac.o                                                           \
> @@ -251,6 +252,7 @@ TESTPROGS = adler32                                                     \
>              file                                                        \
>              fifo                                                        \
>              hash                                                        \
> +            hashtable                                                   \
>              hmac                                                        \
>              hwdevice                                                    \
>              integer                                                     \
> diff --git a/libavutil/hashtable.c b/libavutil/hashtable.c
> new file mode 100644
> index 0000000000..6fa2af8af3
> --- /dev/null
> +++ b/libavutil/hashtable.c
> @@ -0,0 +1,183 @@
> +/*
> + * Generic hashtable
> + * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "crc.h"
> +#include "error.h"
> +#include "mem.h"
> +#include "hashtable.h"
> +
> +struct AVHashtableContext {
> +    size_t key_size;
> +    size_t val_size;
> +    size_t entry_size;
> +    size_t max_entries;
> +    size_t utilization;
> +    const AVCRC *crc;
> +    uint8_t *table;
> +    uint8_t *swapbuf;
> +};
> +
> +#define ENTRY_PSL(entry) (entry)
> +#define ENTRY_OCC(entry) (ENTRY_PSL(entry) + sizeof(size_t))
> +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 4)
> +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + FFALIGN(ctx->key_size, 4))
> +

It seems you misunderstood what I said about alignment in my last
review. You should align entry_size and not align the size of key and
val fields. The way you are doing it now does not prevent unaligned
accesses: Imagine the common case of size_t having an alignment
requirement of 8 and key_size and val_size being four. Then entry_size
is not a multiple of 8 and therefore of two consecutive entries, only
one can be properly aligned (e.g. the second entry
ctx->table+ctx->entry_size is misaligned).
(For the same reason, the alignment of a struct is a multiple of the
alignment of every member of said struct.)
The way you are doing it also adds FFALIGN to every access to val.

> +#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size)
> +
> +int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries)
> +{
> +    struct AVHashtableContext *res = av_malloc(sizeof(struct AVHashtableContext));
> +    if (!res)
> +        return AVERROR(ENOMEM);
> +    res->key_size = key_size;
> +    res->val_size = val_size;

You mostly only need key_size and key_size+val_size; val_size is not
used that often, so if you only store two sizes, it should not be stored.

> +    res->entry_size = sizeof(size_t) + 4 + FFALIGN(key_size, 4) + FFALIGN(val_size, 4);
> +    res->max_entries = max_entries;
> +    res->utilization = 0;
> +    res->crc = av_crc_get_table(AV_CRC_32_IEEE);
> +    if (!res->crc) {
> +        av_hashtable_freep(&res);
> +        return AVERROR_BUG;
> +    }
> +    res->table = av_calloc(res->max_entries, res->entry_size);
> +    if (!res->table) {
> +        av_hashtable_freep(&res);
> +        return AVERROR(ENOMEM);
> +    }
> +    res->swapbuf = av_calloc(2, FFALIGN(key_size, 4) + FFALIGN(val_size, 4));
> +    if (!res->swapbuf) {
> +        av_hashtable_freep(&res);
> +        return AVERROR(ENOMEM);
> +    }
> +    *ctx = res;
> +    return 0;
> +}
> +
> +static size_t hash_key(const struct AVHashtableContext* ctx, const void* key)
> +{
> +    return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries;
> +}
> +
> +int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val)
> +{
> +    size_t hash = hash_key(ctx, key);
> +
> +    if (!ctx->utilization)
> +        return 0;
> +
> +    for (size_t psl = 0; psl < ctx->max_entries; psl++) {
> +        size_t wrapped_index = (hash + psl) % ctx->max_entries;
> +        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> +        if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
> +            return 0;
> +        if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
> +            memcpy(val, ENTRY_VAL(entry), ctx->val_size);
> +            return 1;
> +        }
> +    }
> +    return 0;
> +}
> +
> +int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val)
> +{
> +    int swapping = 0;
> +    size_t psl = 0;
> +    size_t hash = hash_key(ctx, key);
> +    uint8_t *set = ctx->swapbuf;
> +    uint8_t *tmp = ctx->swapbuf + FFALIGN(ctx->key_size, 4) + FFALIGN(ctx->val_size, 4);
> +
> +    memcpy(set, key, ctx->key_size);
> +    memcpy(set + FFALIGN(ctx->key_size, 4), val, ctx->val_size);
> +
> +    for (size_t i = 0; i < ctx->max_entries; i++) {
> +        size_t wrapped_index = (hash + i) % ctx->max_entries;
> +        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> +        if (!*ENTRY_OCC(entry) || (!swapping && KEYS_EQUAL(ENTRY_KEY(entry), set))) {
> +            if (!*ENTRY_OCC(entry))
> +                ctx->utilization++;
> +            *(size_t*)ENTRY_PSL(entry) = psl;
> +            *ENTRY_OCC(entry) = 1;
> +            memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
> +            return 1;
> +        }
> +        if (*(size_t*)ENTRY_PSL(entry) < psl) {
> +            if (ctx->utilization == ctx->max_entries)
> +                return 0;
> +            swapping = 1;
> +            // set needs to swap with entry
> +            memcpy(tmp, ENTRY_KEY(entry), FFALIGN(ctx->key_size, 4) + ctx->val_size);
> +            memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
> +            FFSWAP(uint8_t*, set, tmp);
> +            FFSWAP(size_t, psl, *(size_t*)ENTRY_PSL(entry));
> +        }
> +        psl++;
> +    }
> +    return 0;
> +}
> +
> +int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key)
> +{
> +    uint8_t *next_entry;
> +    size_t hash = hash_key(ctx, key);
> +
> +    if (!ctx->utilization)
> +        return 0;
> +
> +    for (size_t psl = 0; psl < ctx->max_entries; psl++) {
> +        size_t wrapped_index = (hash + psl) % ctx->max_entries;
> +        uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> +        if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
> +            return 0;
> +        if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
> +            *ENTRY_OCC(entry) = 0;
> +            for (psl++; psl < ctx->max_entries; psl++) {
> +                wrapped_index = (hash + psl) % ctx->max_entries;
> +                next_entry = ctx->table + wrapped_index * ctx->entry_size;
> +                if (!*ENTRY_OCC(next_entry) || !*(size_t*)ENTRY_PSL(next_entry)) {
> +                    ctx->utilization--;
> +                    return 1;
> +                }
> +                memcpy(ENTRY_OCC(entry), ENTRY_OCC(next_entry), ctx->entry_size);
> +                *(size_t*)ENTRY_PSL(entry)--;
> +                *ENTRY_OCC(next_entry) = 0;
> +                entry = next_entry;
> +            }
> +        }
> +    };

This ';' is a null statement.

> +    return 0;
> +}
> +
> +void av_hashtable_clear(struct AVHashtableContext* ctx)
> +{
> +    memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
> +}
> +
> +void av_hashtable_freep(struct AVHashtableContext** ctx)
> +{
> +    if (*ctx) {
> +        av_freep(&(*ctx)->table);
> +        av_freep(&(*ctx)->swapbuf);
> +    }
> +    av_freep(ctx);
> +}
> diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h
> new file mode 100644
> index 0000000000..90c6cad68a
> --- /dev/null
> +++ b/libavutil/hashtable.h
> @@ -0,0 +1,47 @@
> +/*
> + * Generic hashtable
> + * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_HASHTABLE_H
> +#define AVUTIL_HASHTABLE_H
> +
> +#include <stddef.h>
> +
> +/* Implements a hash table using Robin Hood open addressing.
> + * See: https://cs.uwaterloo.ca/research/tr/1986/CS-86-14.pdf
> + *
> + * Entry slots are layed out in memory as follows:
> + * - [1 byte] occupancy flag that indicates whether a slot if occupied.
> + * - [key_size bytes] entry key.
> + * - [val_size bytes] entry value.
> + * - [sizeof(size_t) bytes] PSL (probe sequence length), used by the Robin Hood
> + *   algorithm.

This is no longer true and it definitely does not belong in a header.

> + */
> +
> +struct AVHashtableContext;
> +
> +int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries);
> +int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val);
> +int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val);
> +int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key);
> +void av_hashtable_clear(struct AVHashtableContext* ctx);
> +void av_hashtable_freep(struct AVHashtableContext** ctx);
> +
> +#endif
> diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c
> new file mode 100644
> index 0000000000..0b38b34c5c
> --- /dev/null
> +++ b/libavutil/tests/hashtable.c
> @@ -0,0 +1,108 @@
> +/*
> + * Generic hashtable tests
> + * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/hashtable.h"
> +
> +int main(void)
> +{
> +    struct AVHashtableContext *ctx;
> +    uint8_t k;
> +    uint64_t v;
> +
> +    // impossibly large allocation should fail gracefully

Actually, av_hashtable_alloc is missing overflow checks.

> +    av_assert0(av_hashtable_alloc(&ctx, -1, -1, -1) < 0);
> +
> +    // hashtable can store up to 3 uint8_t->uint64_t entries
> +    av_assert0(!av_hashtable_alloc(&ctx, sizeof(k), sizeof(v), 3));
> +
> +    // unsuccessful deletes return 0
> +    k = 1;
> +    av_assert0(!av_hashtable_delete(ctx, &k));
> +
> +    // unsuccessful gets return 0
> +    k = 1;
> +    av_assert0(!av_hashtable_get(ctx, &k, &v));
> +
> +    // successful sets returns 1
> +    k = 1;
> +    v = 1;
> +    av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> +    // get should now contain 1
> +    k = 1;
> +    v = 0;
> +    av_assert0(av_hashtable_get(ctx, &k, &v));
> +    av_assert0(v == 1);
> +
> +    // updating sets should return 1
> +    k = 1;
> +    v = 2;
> +    av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> +    // get should now contain 2
> +    k = 1;
> +    v = 0;
> +    av_assert0(av_hashtable_get(ctx, &k, &v));
> +    av_assert0(v == 2);
> +
> +    // fill the table
> +    k = 2;
> +    v = 2;
> +    av_assert0(av_hashtable_set(ctx, &k, &v));
> +    k = 3;
> +    v = 3;
> +    av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> +    // inserting sets on a full table should return 0
> +    k = 4;
> +    v = 4;
> +    av_assert0(!av_hashtable_set(ctx, &k, &v));
> +
> +    // updating sets on a full table should return 1
> +    k = 1;
> +    v = 4;
> +    av_assert0(av_hashtable_set(ctx, &k, &v));
> +    v = 0;
> +    av_assert0(av_hashtable_get(ctx, &k, &v));
> +    av_assert0(v == 4);
> +
> +    // successful deletes should return 1
> +    k = 1;
> +    av_assert0(av_hashtable_delete(ctx, &k));
> +
> +    // get should now return 0
> +    av_assert0(!av_hashtable_get(ctx, &k, &v));
> +
> +    // sanity check remaining keys
> +    k = 2;
> +    v = 0;
> +    av_assert0(av_hashtable_get(ctx, &k, &v));
> +    av_assert0(v == 2);
> +    k = 3;
> +    v = 0;
> +    av_assert0(av_hashtable_get(ctx, &k, &v));
> +    av_assert0(v == 3);
> +
> +    av_hashtable_freep(&ctx);
> +
> +    return 0;
> +}

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[Connor Worley]

On Wed, Feb 7, 2024 at 2:16 AM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Connor Worley:
> > +#define ENTRY_PSL(entry) (entry)
> > +#define ENTRY_OCC(entry) (ENTRY_PSL(entry) + sizeof(size_t))
> > +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 4)
> > +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + FFALIGN(ctx->key_size, 4))
> > +
>
> It seems you misunderstood what I said about alignment in my last
> review. You should align entry_size and not align the size of key and
> val fields. The way you are doing it now does not prevent unaligned
> accesses: Imagine the common case of size_t having an alignment
> requirement of 8 and key_size and val_size being four. Then entry_size
> is not a multiple of 8 and therefore of two consecutive entries, only
> one can be properly aligned (e.g. the second entry
> ctx->table+ctx->entry_size is misaligned).
> (For the same reason, the alignment of a struct is a multiple of the
> alignment of every member of said struct.)
> The way you are doing it also adds FFALIGN to every access to val.
>

Apologies as I don't usually write C. Is it not necessary to ensure val is
aligned?
Would aligning every field to sizeof(size_t) be sufficient for ensuring
consecutive entries are aligned?

I believe I would need to store a possibly rounded-up key_size to avoid the
FFALIGN in val access.

-- 
Connor Worley
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1569 bytes --]

On Tue, Feb 06, 2024 at 02:57:53AM -0800, Connor Worley wrote:
[...]
> new file mode 100644
> index 0000000000..0b38b34c5c
> --- /dev/null
> +++ b/libavutil/tests/hashtable.c
> @@ -0,0 +1,108 @@
> +/*
> + * Generic hashtable tests
> + * Copyright (C) 2024 Connor Worley <connorbworley@gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/hashtable.h"

missing stdint.h or inttypes.h


> +
> +int main(void)
> +{
> +    struct AVHashtableContext *ctx;
> +    uint8_t k;
> +    uint64_t v;

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".