* [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table
@ 2024-02-05 5:27 Connor Worley
2024-02-05 5:27 ` [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable Connor Worley
2024-02-05 12:00 ` [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Andreas Rheinhardt
0 siblings, 2 replies; 9+ messages in thread
From: Connor Worley @ 2024-02-05 5:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Connor Worley
Signed-off-by: Connor Worley <connorbworley@gmail.com>
---
libavutil/Makefile | 2 +
libavutil/hashtable.c | 172 ++++++++++++++++++++++++++++++++++++
libavutil/hashtable.h | 62 +++++++++++++
libavutil/tests/hashtable.c | 104 ++++++++++++++++++++++
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..fb0f0aae99
--- /dev/null
+++ b/libavutil/hashtable.c
@@ -0,0 +1,172 @@
+/*
+ * 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 "error.h"
+#include "mem.h"
+#include "hashtable.h"
+#include "string.h"
+
+#define ENTRY_OCC(entry) (entry)
+#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 1)
+#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + ctx->key_size)
+#define ENTRY_PSL(entry) (size_t*) (ENTRY_VAL(entry) + ctx->val_size)
+
+#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size)
+
+int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries)
+{
+ ctx->key_size = key_size;
+ ctx->val_size = val_size;
+ ctx->entry_size = 1 + key_size + val_size + sizeof(size_t);
+ ctx->max_entries = max_entries;
+ ctx->utilization = 0;
+ ctx->crc = av_crc_get_table(AV_CRC_32_IEEE);
+ if (!ctx->crc)
+ return AVERROR_BUG;
+ ctx->table = av_mallocz(ctx->entry_size * max_entries);
+ if (!ctx->table)
+ return AVERROR(ENOMEM);
+ ctx->set_key = av_malloc(key_size);
+ if (!ctx->set_key)
+ return AVERROR(ENOMEM);
+ ctx->set_val = av_malloc(key_size);
+ if (!ctx->set_val)
+ return AVERROR(ENOMEM);
+ ctx->tmp_key = av_malloc(key_size);
+ if (!ctx->tmp_key)
+ return AVERROR(ENOMEM);
+ ctx->tmp_val = av_malloc(val_size);
+ if (!ctx->tmp_val)
+ return AVERROR(ENOMEM);
+ return 0;
+}
+
+static size_t hash_key(const AVHashtableContext* ctx, const void* key)
+{
+ return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries;
+}
+
+int av_hashtable_get(const 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) || *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(AVHashtableContext* ctx, const void* key, const void* val)
+{
+ int swapping = 0;
+ size_t psl = 0;
+ size_t hash = hash_key(ctx, key);
+
+ memcpy(ctx->set_key, key, ctx->key_size);
+ memcpy(ctx->set_val, 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), ctx->set_key))) {
+ if (!*ENTRY_OCC(entry))
+ ctx->utilization++;
+ *ENTRY_OCC(entry) = 1;
+ memcpy(ENTRY_KEY(entry), ctx->set_key, ctx->key_size);
+ memcpy(ENTRY_VAL(entry), ctx->set_val, ctx->val_size);
+ *ENTRY_PSL(entry) = psl;
+ return 1;
+ }
+ if (*ENTRY_PSL(entry) < psl) {
+ if (ctx->utilization == ctx->max_entries)
+ return 0;
+ swapping = 1;
+ memcpy(ctx->tmp_key, ctx->set_key, ctx->key_size);
+ memcpy(ctx->tmp_val, ctx->set_val, ctx->val_size);
+ memcpy(ctx->set_key, ENTRY_KEY(entry), ctx->key_size);
+ memcpy(ctx->set_val, ENTRY_VAL(entry), ctx->val_size);
+ memcpy(ENTRY_KEY(entry), ctx->tmp_key, ctx->key_size);
+ memcpy(ENTRY_VAL(entry), ctx->tmp_val, ctx->val_size);
+ FFSWAP(size_t, psl, *ENTRY_PSL(entry));
+ }
+ psl++;
+ }
+ return 0;
+}
+
+int av_hashtable_delete(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) || *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) || !*ENTRY_PSL(next_entry)) {
+ ctx->utilization--;
+ return 1;
+ }
+ FFSWAP(uint8_t, *ENTRY_OCC(entry), *ENTRY_OCC(next_entry));
+ memcpy(ENTRY_KEY(entry), ENTRY_KEY(next_entry), ctx->key_size);
+ memcpy(ENTRY_VAL(entry), ENTRY_VAL(next_entry), ctx->val_size);
+ *ENTRY_PSL(entry) = *ENTRY_PSL(next_entry) - 1;
+ entry = next_entry;
+ }
+ }
+ };
+ return 0;
+}
+
+void av_hashtable_clear(AVHashtableContext* ctx)
+{
+ memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
+}
+
+void av_hashtable_destroy(AVHashtableContext* ctx)
+{
+ av_freep(&ctx->table);
+ av_freep(&ctx->set_key);
+ av_freep(&ctx->set_val);
+ av_freep(&ctx->tmp_key);
+ av_freep(&ctx->tmp_val);
+}
\ No newline at end of file
diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h
new file mode 100644
index 0000000000..aac31ee922
--- /dev/null
+++ b/libavutil/hashtable.h
@@ -0,0 +1,62 @@
+/*
+ * 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>
+
+#include "crc.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.
+ */
+
+typedef 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;
+ /* used for swaps during insertion */
+ uint8_t *set_key;
+ uint8_t *set_val;
+ uint8_t *tmp_key;
+ uint8_t *tmp_val;
+} AVHashtableContext;
+
+int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries);
+int av_hashtable_get(const AVHashtableContext* ctx, const void* key, void* val);
+int av_hashtable_set(AVHashtableContext* ctx, const void* key, const void* val);
+int av_hashtable_delete(AVHashtableContext* ctx, const void* key);
+void av_hashtable_clear(AVHashtableContext* ctx);
+void av_hashtable_destroy(AVHashtableContext* ctx);
+
+#endif
\ No newline at end of file
diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c
new file mode 100644
index 0000000000..1cdbd7afd3
--- /dev/null
+++ b/libavutil/tests/hashtable.c
@@ -0,0 +1,104 @@
+/*
+ * 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)
+{
+ AVHashtableContext ctx;
+ int k, v;
+
+ // hashtable can store up to 3 int->int entries
+ av_assert0(!av_hashtable_init(&ctx, sizeof(int), sizeof(int), 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_destroy(&ctx);
+
+ return 0;
+}
\ No newline at end of file
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable
2024-02-05 5:27 [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Connor Worley
@ 2024-02-05 5:27 ` Connor Worley
2024-02-05 12:04 ` Andreas Rheinhardt
2024-02-05 12:00 ` [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Andreas Rheinhardt
1 sibling, 1 reply; 9+ messages in thread
From: Connor Worley @ 2024-02-05 5:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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, 35 insertions(+), 86 deletions(-)
diff --git a/libavcodec/dxvenc.c b/libavcodec/dxvenc.c
index b274175689..e17b3b2c36 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"
@@ -44,69 +44,6 @@ 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];
+ AVHashtableContext color_ht;
+ 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,10 +239,15 @@ 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_init(&ctx->color_ht, sizeof(uint32_t), sizeof(uint32_t), LOOKBACK_HT_ELEMS);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "Could not initialize color lookback table.\n");
+ return ret;
+ }
+ ret = av_hashtable_init(&ctx->lut_ht, sizeof(uint32_t), sizeof(uint32_t), LOOKBACK_HT_ELEMS);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "Could not initialize LUT lookback table.\n");
+ return ret;
}
return 0;
@@ -313,6 +259,9 @@ static av_cold int dxv_close(AVCodecContext *avctx)
av_freep(&ctx->tex_data);
+ av_hashtable_destroy(&ctx->color_ht);
+ av_hashtable_destroy(&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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table
2024-02-05 5:27 [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Connor Worley
2024-02-05 5:27 ` [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable Connor Worley
@ 2024-02-05 12:00 ` Andreas Rheinhardt
2024-02-06 5:00 ` Connor Worley
1 sibling, 1 reply; 9+ messages in thread
From: Andreas Rheinhardt @ 2024-02-05 12:00 UTC (permalink / raw)
To: ffmpeg-devel
Connor Worley:
> Signed-off-by: Connor Worley <connorbworley@gmail.com>
> ---
> libavutil/Makefile | 2 +
> libavutil/hashtable.c | 172 ++++++++++++++++++++++++++++++++++++
> libavutil/hashtable.h | 62 +++++++++++++
> libavutil/tests/hashtable.c | 104 ++++++++++++++++++++++
> 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..fb0f0aae99
> --- /dev/null
> +++ b/libavutil/hashtable.c
> @@ -0,0 +1,172 @@
> +/*
> + * 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 "error.h"
> +#include "mem.h"
> +#include "hashtable.h"
> +#include "string.h"
We don't have a string.h header; the string.h from the C stdlib should
of course be included via <>.
> +
> +#define ENTRY_OCC(entry) (entry)
> +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 1)
> +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + ctx->key_size)
> +#define ENTRY_PSL(entry) (size_t*) (ENTRY_VAL(entry) + ctx->val_size)
Unaligned access. You need to align entry_size to the required alignment
of size_t and put this at the start of it the entry to avoid this.
Apart from that: The layout of your entries implies that only two
members are at a fixed offset from the start, so that accessing val and
psl requires actual additions. If you make it psl, occ, key, val, then
only val has a variable offset. This should be beneficial given that it
is the less accessed of key and val (in particular if you combine
several memcpy calls as I outline below).
(This is of course a good reason not to make the layout of the entries
public.)
> +
> +#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size)
> +
> +int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries)
> +{
> + ctx->key_size = key_size;
> + ctx->val_size = val_size;
> + ctx->entry_size = 1 + key_size + val_size + sizeof(size_t);
> + ctx->max_entries = max_entries;
> + ctx->utilization = 0;
> + ctx->crc = av_crc_get_table(AV_CRC_32_IEEE);
> + if (!ctx->crc)
> + return AVERROR_BUG;
> + ctx->table = av_mallocz(ctx->entry_size * max_entries);
av_calloc()
> + if (!ctx->table)
> + return AVERROR(ENOMEM);
> + ctx->set_key = av_malloc(key_size);
> + if (!ctx->set_key)
> + return AVERROR(ENOMEM);
> + ctx->set_val = av_malloc(key_size);
Should be val_size. Your test should test scenarios with key_size !=
val_size to uncover this.
> + if (!ctx->set_val)
> + return AVERROR(ENOMEM);
> + ctx->tmp_key = av_malloc(key_size);
> + if (!ctx->tmp_key)
> + return AVERROR(ENOMEM);
> + ctx->tmp_val = av_malloc(val_size);
> + if (!ctx->tmp_val)
> + return AVERROR(ENOMEM);
> + return 0;
> +}
(There are btw issues with what happens on error here, but we can ignore
this in the first round.)
> +
> +static size_t hash_key(const AVHashtableContext* ctx, const void* key)
> +{
> + return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries;
> +}
> +
> +int av_hashtable_get(const 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) || *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(AVHashtableContext* ctx, const void* key, const void* val)
> +{
> + int swapping = 0;
> + size_t psl = 0;
> + size_t hash = hash_key(ctx, key);
> +
> + memcpy(ctx->set_key, key, ctx->key_size);
> + memcpy(ctx->set_val, 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), ctx->set_key))) {
> + if (!*ENTRY_OCC(entry))
> + ctx->utilization++;
> + *ENTRY_OCC(entry) = 1;
> + memcpy(ENTRY_KEY(entry), ctx->set_key, ctx->key_size);
> + memcpy(ENTRY_VAL(entry), ctx->set_val, ctx->val_size);
> + *ENTRY_PSL(entry) = psl;
> + return 1;
> + }
> + if (*ENTRY_PSL(entry) < psl) {
> + if (ctx->utilization == ctx->max_entries)
> + return 0;
> + swapping = 1;
> + memcpy(ctx->tmp_key, ctx->set_key, ctx->key_size);
> + memcpy(ctx->tmp_val, ctx->set_val, ctx->val_size);
Given that set_key/val are overwritten below, these two can be done via
pointerswaps.
> + memcpy(ctx->set_key, ENTRY_KEY(entry), ctx->key_size);
> + memcpy(ctx->set_val, ENTRY_VAL(entry), ctx->val_size);
> + memcpy(ENTRY_KEY(entry), ctx->tmp_key, ctx->key_size);
> + memcpy(ENTRY_VAL(entry), ctx->tmp_val, ctx->val_size);
> + FFSWAP(size_t, psl, *ENTRY_PSL(entry));
If you allocated foo_key and foo_val jointly, you can combine these
memcpy's. In fact, you can allocate all four them jointly with a
key-val-key-val layout and only keep one pointer in the context. You
would then use an uint8_t *set, *val pointers on the stack of this
function that are initialized at the beginning of this function.
> + }
> + psl++;
> + }
> + return 0;
> +}
> +
> +int av_hashtable_delete(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) || *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) || !*ENTRY_PSL(next_entry)) {
> + ctx->utilization--;
> + return 1;
> + }
> + FFSWAP(uint8_t, *ENTRY_OCC(entry), *ENTRY_OCC(next_entry));
> + memcpy(ENTRY_KEY(entry), ENTRY_KEY(next_entry), ctx->key_size);
> + memcpy(ENTRY_VAL(entry), ENTRY_VAL(next_entry), ctx->val_size);
These two memcpy can be combined (if one lays out key and val
sequentially in entry. Can even be combined with occ if you change the
entry layout as I outlined above.
> + *ENTRY_PSL(entry) = *ENTRY_PSL(next_entry) - 1;
> + entry = next_entry;
> + }
> + }
> + };
> + return 0;
> +}
> +
> +void av_hashtable_clear(AVHashtableContext* ctx)
> +{
> + memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
> +}
> +
> +void av_hashtable_destroy(AVHashtableContext* ctx)
> +{
> + av_freep(&ctx->table);
> + av_freep(&ctx->set_key);
> + av_freep(&ctx->set_val);
> + av_freep(&ctx->tmp_key);
> + av_freep(&ctx->tmp_val);
> +}
> \ No newline at end of file
> diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h
> new file mode 100644
> index 0000000000..aac31ee922
> --- /dev/null
> +++ b/libavutil/hashtable.h
> @@ -0,0 +1,62 @@
> +/*
> + * 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>
> +
> +#include "crc.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.
Such things should not be made public. Even if
sizeof(AVHashtableContext) were part of the ABI.
> + */
> +
> +typedef struct AVHashtableContext {
> + size_t key_size;
> + size_t val_size;
> + size_t entry_size;
> + size_t max_entries;
> + size_t utilization;
> + const AVCRC *crc;
You don't need the complete type here, so by using struct AVCRC *crc you
can avoid the crc.h inclusion.
> + uint8_t *table;
> + /* used for swaps during insertion */
> + uint8_t *set_key;
> + uint8_t *set_val;
> + uint8_t *tmp_key;
> + uint8_t *tmp_val;
> +} AVHashtableContext;
In patch 2/2 you are using this in a way that makes
sizeof(AVHashtableContext) part of the ABI. Is this intended?
Even if it is, the user should be forbidden from accessing anything in it.
> +
> +int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries);
We generally use
> +int av_hashtable_get(const AVHashtableContext* ctx, const void* key, void* val);
> +int av_hashtable_set(AVHashtableContext* ctx, const void* key, const void* val);
> +int av_hashtable_delete(AVHashtableContext* ctx, const void* key);
> +void av_hashtable_clear(AVHashtableContext* ctx);
> +void av_hashtable_destroy(AVHashtableContext* ctx);
> +
> +#endif
> \ No newline at end of file
> diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c
> new file mode 100644
> index 0000000000..1cdbd7afd3
> --- /dev/null
> +++ b/libavutil/tests/hashtable.c
> @@ -0,0 +1,104 @@
> +/*
> + * 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)
> +{
> + AVHashtableContext ctx;
> + int k, v;
> +
> + // hashtable can store up to 3 int->int entries
> + av_assert0(!av_hashtable_init(&ctx, sizeof(int), sizeof(int), 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_destroy(&ctx);
> +
> + return 0;
> +}
> \ No newline at end of file
You should add these newlines, they are AFAIK required by the spec.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable
2024-02-05 5:27 ` [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable Connor Worley
@ 2024-02-05 12:04 ` Andreas Rheinhardt
2024-02-05 19:57 ` Connor Worley
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Rheinhardt @ 2024-02-05 12:04 UTC (permalink / raw)
To: ffmpeg-devel
Connor Worley:
> Offers a modest performance gain due to the switch from naive linear
> probling to robin hood.
How much would one gain if the hash function knew that key_size and
val_size are four?
>
> Signed-off-by: Connor Worley <connorbworley@gmail.com>
> ---
> libavcodec/dxvenc.c | 121 +++++++++++++-------------------------------
> 1 file changed, 35 insertions(+), 86 deletions(-)
>
> diff --git a/libavcodec/dxvenc.c b/libavcodec/dxvenc.c
> index b274175689..e17b3b2c36 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"
>
> @@ -44,69 +44,6 @@ 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];
> + AVHashtableContext color_ht;
> + 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,10 +239,15 @@ 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_init(&ctx->color_ht, sizeof(uint32_t), sizeof(uint32_t), LOOKBACK_HT_ELEMS);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Could not initialize color lookback table.\n");
> + return ret;
> + }
> + ret = av_hashtable_init(&ctx->lut_ht, sizeof(uint32_t), sizeof(uint32_t), LOOKBACK_HT_ELEMS);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Could not initialize LUT lookback table.\n");
These av_logs are pointless.
> + return ret;
> }
>
> return 0;
> @@ -313,6 +259,9 @@ static av_cold int dxv_close(AVCodecContext *avctx)
>
> av_freep(&ctx->tex_data);
>
> + av_hashtable_destroy(&ctx->color_ht);
> + av_hashtable_destroy(&ctx->lut_ht);
> +
> 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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable
2024-02-05 12:04 ` Andreas Rheinhardt
@ 2024-02-05 19:57 ` Connor Worley
2024-02-05 20:08 ` Andreas Rheinhardt
0 siblings, 1 reply; 9+ messages in thread
From: Connor Worley @ 2024-02-05 19:57 UTC (permalink / raw)
To: FFmpeg development discussions and patches
>
> How much would one gain if the hash function knew that key_size and
> val_size are four?
>
That yields a nice 10-20% speedup on my machine. Are you thinking of macros
to parametrize key/val size, or possibly optimized versions for common
sizes?
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable
2024-02-05 19:57 ` Connor Worley
@ 2024-02-05 20:08 ` Andreas Rheinhardt
2024-02-05 20:46 ` Connor Worley
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Rheinhardt @ 2024-02-05 20:08 UTC (permalink / raw)
To: ffmpeg-devel
Connor Worley:
>>
>> How much would one gain if the hash function knew that key_size and
>> val_size are four?
>>
>
> That yields a nice 10-20% speedup on my machine. Are you thinking of macros
> to parametrize key/val size, or possibly optimized versions for common
> sizes?
I am thinking about it not being public at all; as long as we have only
one user...
- Andreas
PS: Is the speedup only for the hashing part or for overall decoding? Is
the comparison the current version of your patch or something that
already has my comments incorporated?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable
2024-02-05 20:08 ` Andreas Rheinhardt
@ 2024-02-05 20:46 ` Connor Worley
0 siblings, 0 replies; 9+ messages in thread
From: Connor Worley @ 2024-02-05 20:46 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, Feb 5, 2024 at 12:06 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:
> Connor Worley:
> >>
> >> How much would one gain if the hash function knew that key_size and
> >> val_size are four?
> >>
> >
> > That yields a nice 10-20% speedup on my machine. Are you thinking of
> macros
> > to parametrize key/val size, or possibly optimized versions for common
> > sizes?
>
> I am thinking about it not being public at all; as long as we have only
> one user...
>
That's fair. I will need some variants for future DXV work, but they can
also be private if this doesn't seem generally useful.
PS: Is the speedup only for the hashing part or for overall decoding? Is
> the comparison the current version of your patch or something that
> already has my comments incorporated?
>
Overall encoding -- current version of my patch with ctx->key/val_size vs
hardcoded sizes.
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table
2024-02-05 12:00 ` [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Andreas Rheinhardt
@ 2024-02-06 5:00 ` Connor Worley
2024-02-06 10:13 ` Connor Worley
0 siblings, 1 reply; 9+ messages in thread
From: Connor Worley @ 2024-02-06 5:00 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, Feb 5, 2024 at 3:58 AM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:
> Connor Worley:
> > + memcpy(ctx->tmp_key, ctx->set_key, ctx->key_size);
> > + memcpy(ctx->tmp_val, ctx->set_val, ctx->val_size);
>
> Given that set_key/val are overwritten below, these two can be done via
> pointerswaps.
I don't quite follow, can you elaborate on this part?
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table
2024-02-06 5:00 ` Connor Worley
@ 2024-02-06 10:13 ` Connor Worley
0 siblings, 0 replies; 9+ messages in thread
From: Connor Worley @ 2024-02-06 10:13 UTC (permalink / raw)
To: FFmpeg development discussions and patches
NVM, I see what you mean by pointerswaps.
On Mon, Feb 5, 2024 at 9:00 PM Connor Worley <connorbworley@gmail.com>
wrote:
> On Mon, Feb 5, 2024 at 3:58 AM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
>
>> Connor Worley:
>> > + memcpy(ctx->tmp_key, ctx->set_key, ctx->key_size);
>> > + memcpy(ctx->tmp_val, ctx->set_val, ctx->val_size);
>>
>> Given that set_key/val are overwritten below, these two can be done via
>> pointerswaps.
>
>
> I don't quite follow, can you elaborate on this part?
> --
> Connor Worley
>
--
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-06 10:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-05 5:27 [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Connor Worley
2024-02-05 5:27 ` [FFmpeg-devel] [PATCH 2/2] lavc/dxvenc: migrate DXT1 encoder to lavu hashtable Connor Worley
2024-02-05 12:04 ` Andreas Rheinhardt
2024-02-05 19:57 ` Connor Worley
2024-02-05 20:08 ` Andreas Rheinhardt
2024-02-05 20:46 ` Connor Worley
2024-02-05 12:00 ` [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table Andreas Rheinhardt
2024-02-06 5:00 ` Connor Worley
2024-02-06 10:13 ` Connor Worley
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git