From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH 1/9] avcodec/Makefile: Only compile hashtable.o when needed
Date: Tue, 3 Jun 2025 23:32:30 +0200
Message-ID: <GV1P250MB0737A0698A0CBA8EC7C1807B8F6DA@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM> (raw)
[-- Attachment #1: Type: text/plain, Size: 29 bytes --]
Patches attached.
- Andreas
[-- Attachment #2: 0001-avcodec-Makefile-Only-compile-hashtable.o-when-neede.patch --]
[-- Type: text/x-patch, Size: 2769 bytes --]
From 7cc1c15bb30c47edb354512fffb1f4148e7e0a9f Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 21:38:41 +0200
Subject: [PATCH 1/9] avcodec/Makefile: Only compile hashtable.o when needed
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/Makefile | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index fb3fb7f7f7..4d0b9c2773 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -42,7 +42,6 @@ OBJS = ac3_parser.o \
dv_profile.o \
encode.o \
get_buffer.o \
- hashtable.o \
imgconvert.o \
jni.o \
lcevcdec.o \
@@ -352,7 +351,7 @@ OBJS-$(CONFIG_DVVIDEO_ENCODER) += dvenc.o dv.o dvdata.o
OBJS-$(CONFIG_DXA_DECODER) += dxa.o
OBJS-$(CONFIG_DXTORY_DECODER) += dxtory.o
OBJS-$(CONFIG_DXV_DECODER) += dxv.o
-OBJS-$(CONFIG_DXV_ENCODER) += dxvenc.o
+OBJS-$(CONFIG_DXV_ENCODER) += dxvenc.o hashtable.o
OBJS-$(CONFIG_EAC3_DECODER) += eac3_data.o
OBJS-$(CONFIG_EAC3_ENCODER) += eac3enc.o eac3_data.o
OBJS-$(CONFIG_EACMV_DECODER) += eacmv.o
@@ -1327,7 +1326,6 @@ TESTPROGS = avcodec \
bitstream_le \
celp_math \
codec_desc \
- hashtable \
htmlsubtitles \
jpeg2000dwt \
mathops \
@@ -1337,6 +1335,7 @@ TESTPROGS-$(CONFIG_AV1_VAAPI_ENCODER) += av1_levels
TESTPROGS-$(CONFIG_CABAC) += cabac
TESTPROGS-$(CONFIG_GOLOMB) += golomb
TESTPROGS-$(CONFIG_IDCTDSP) += dct
+TESTPROGS-$(CONFIG_DXV_ENCODER) += hashtable
TESTPROGS-$(CONFIG_IIRFILTER) += iirfilter
TESTPROGS-$(CONFIG_MJPEG_ENCODER) += mjpegenc_huffman
TESTPROGS-$(HAVE_MMX) += motion
--
2.45.2
[-- Attachment #3: 0002-avcodec-hashtable-Zero-initialize-hashtable.patch --]
[-- Type: text/x-patch, Size: 1075 bytes --]
From 16164aa30b3ae3dd4e4cb79895431f86bb049c06 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 21:42:24 +0200
Subject: [PATCH 2/9] avcodec/hashtable: Zero-initialize hashtable
Otherwise ff_hashtable_freep() would try to free uninitialized
pointers upon allocation error (which happens in the corresponding
test tool).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index 151476176b..4c7d3e18ba 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -58,7 +58,7 @@ struct FFHashtableContext {
int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t val_size, size_t max_entries)
{
- struct FFHashtableContext *res = av_malloc(sizeof(struct FFHashtableContext));
+ FFHashtableContext *res = av_mallocz(sizeof(*res));
if (!res)
return AVERROR(ENOMEM);
res->key_size = key_size;
--
2.45.2
[-- Attachment #4: 0003-tests-fate-libavcodec-Run-hashtable-test.patch --]
[-- Type: text/x-patch, Size: 1048 bytes --]
From aa330af12635009a05e9ff300ca4f7850ca531be Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 21:58:58 +0200
Subject: [PATCH 3/9] tests/fate/libavcodec: Run hashtable test
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
tests/fate/libavcodec.mak | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/fate/libavcodec.mak b/tests/fate/libavcodec.mak
index b69ad53f7c..361571aaee 100644
--- a/tests/fate/libavcodec.mak
+++ b/tests/fate/libavcodec.mak
@@ -43,6 +43,11 @@ fate-golomb: libavcodec/tests/golomb$(EXESUF)
fate-golomb: CMD = run libavcodec/tests/golomb$(EXESUF)
fate-golomb: CMP = null
+FATE_LIBAVCODEC-$(CONFIG_DXV_ENCODER) += fate-hashtable
+fate-hashtable: libavcodec/tests/hashtable$(EXESUF)
+fate-hashtable: CMD = run libavcodec/tests/hashtable$(EXESUF)
+fate-hashtable: CMP = null
+
FATE_LIBAVCODEC-$(CONFIG_IDCTDSP) += fate-idct8x8-0 fate-idct8x8-1 fate-idct8x8-2 fate-idct248
fate-idct8x8-0: libavcodec/tests/dct$(EXESUF)
--
2.45.2
[-- Attachment #5: 0004-avcodec-hashtable-Only-align-complete-entries.patch --]
[-- Type: text/x-patch, Size: 4297 bytes --]
From be43be1f8cdb18f54690da2bfbc050ab9af1614c Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 22:08:46 +0200
Subject: [PATCH 4/9] avcodec/hashtable: Only align complete entries
It is unnecessary to align both key and val, as they are only accessed
via memcpy()/memcmp(), which has no alignment requirements.
We only need to ensure that that the entries as a whole
are suitable aligned for the probe sequence length.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index 4c7d3e18ba..fa79330603 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -31,9 +31,7 @@
struct FFHashtableContext {
size_t key_size;
- size_t key_size_aligned;
size_t val_size;
- size_t val_size_aligned;
size_t entry_size;
size_t max_entries;
size_t nb_entries;
@@ -51,8 +49,8 @@ struct FFHashtableContext {
*/
#define ENTRY_PSL_VAL(entry) (*(size_t*)(entry))
-#define ENTRY_KEY_PTR(entry) ((entry) + FFALIGN(sizeof(size_t), ALIGN))
-#define ENTRY_VAL_PTR(entry) (ENTRY_KEY_PTR(entry) + ctx->key_size_aligned)
+#define ENTRY_KEY_PTR(entry) ((entry) + sizeof(size_t))
+#define ENTRY_VAL_PTR(entry) (ENTRY_KEY_PTR(entry) + ctx->key_size)
#define KEYS_EQUAL(k1, k2) (!memcmp((k1), (k2), ctx->key_size))
@@ -62,12 +60,8 @@ int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t
if (!res)
return AVERROR(ENOMEM);
res->key_size = key_size;
- res->key_size_aligned = FFALIGN(key_size, ALIGN);
res->val_size = val_size;
- res->val_size_aligned = FFALIGN(val_size, ALIGN);
- res->entry_size = FFALIGN(sizeof(size_t), ALIGN)
- + res->key_size_aligned
- + res->val_size_aligned;
+ res->entry_size = FFALIGN(sizeof(size_t) + key_size + val_size, ALIGN);
res->max_entries = max_entries;
res->nb_entries = 0;
res->crc = av_crc_get_table(AV_CRC_32_IEEE);
@@ -81,7 +75,7 @@ int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t
return AVERROR(ENOMEM);
}
- res->swapbuf = av_calloc(2, res->key_size_aligned + res->val_size_aligned);
+ res->swapbuf = av_calloc(2, res->key_size + res->val_size);
if (!res->swapbuf) {
ff_hashtable_freep(&res);
return AVERROR(ENOMEM);
@@ -124,10 +118,10 @@ int ff_hashtable_set(struct FFHashtableContext *ctx, const void *key, const void
size_t hash = hash_key(ctx, key);
size_t wrapped_index = hash % ctx->max_entries;
uint8_t *set = ctx->swapbuf;
- uint8_t *tmp = ctx->swapbuf + ctx->key_size_aligned + ctx->val_size_aligned;
+ uint8_t *tmp = ctx->swapbuf + ctx->key_size + ctx->val_size;
memcpy(set, key, ctx->key_size);
- memcpy(set + ctx->key_size_aligned, val, ctx->val_size);
+ memcpy(set + ctx->key_size, val, ctx->val_size);
for (size_t i = 0; i < ctx->max_entries; i++) {
if (++wrapped_index == ctx->max_entries)
@@ -137,7 +131,7 @@ int ff_hashtable_set(struct FFHashtableContext *ctx, const void *key, const void
if (!ENTRY_PSL_VAL(entry))
ctx->nb_entries++;
ENTRY_PSL_VAL(entry) = psl;
- memcpy(ENTRY_KEY_PTR(entry), set, ctx->key_size_aligned + ctx->val_size);
+ memcpy(ENTRY_KEY_PTR(entry), set, ctx->key_size + ctx->val_size);
return 1;
}
if (ENTRY_PSL_VAL(entry) < psl) {
@@ -151,8 +145,8 @@ int ff_hashtable_set(struct FFHashtableContext *ctx, const void *key, const void
// PSL of the inserted entry.
swapping = 1;
// set needs to swap with entry
- memcpy(tmp, ENTRY_KEY_PTR(entry), ctx->key_size_aligned + ctx->val_size_aligned);
- memcpy(ENTRY_KEY_PTR(entry), set, ctx->key_size_aligned + ctx->val_size_aligned);
+ memcpy(tmp, ENTRY_KEY_PTR(entry), ctx->key_size + ctx->val_size);
+ memcpy(ENTRY_KEY_PTR(entry), set, ctx->key_size + ctx->val_size);
FFSWAP(uint8_t*, set, tmp);
FFSWAP(size_t, psl, ENTRY_PSL_VAL(entry));
}
--
2.45.2
[-- Attachment #6: 0005-avcodec-hashtable-Check-for-overflow.patch --]
[-- Type: text/x-patch, Size: 1374 bytes --]
From 3407a2d3aa03360af4c7ad5037908946843ed3dc Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 22:35:03 +0200
Subject: [PATCH 5/9] avcodec/hashtable: Check for overflow
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index fa79330603..ec8eca471f 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -56,12 +56,18 @@ struct FFHashtableContext {
int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t val_size, size_t max_entries)
{
+ const size_t keyval_size = key_size + val_size;
+
+ if (keyval_size < key_size || // did (unsigned,defined) wraparound happen?
+ keyval_size > SIZE_MAX - sizeof(size_t) - (ALIGN - 1))
+ return AVERROR(ERANGE);
+
FFHashtableContext *res = av_mallocz(sizeof(*res));
if (!res)
return AVERROR(ENOMEM);
res->key_size = key_size;
res->val_size = val_size;
- res->entry_size = FFALIGN(sizeof(size_t) + key_size + val_size, ALIGN);
+ res->entry_size = FFALIGN(sizeof(size_t) + keyval_size, ALIGN);
res->max_entries = max_entries;
res->nb_entries = 0;
res->crc = av_crc_get_table(AV_CRC_32_IEEE);
--
2.45.2
[-- Attachment #7: 0006-avcodec-hashtable-Combine-allocations.patch --]
[-- Type: text/x-patch, Size: 1960 bytes --]
From 5437b460944bd27da9026b3a0edc0db69dea5275 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 22:44:41 +0200
Subject: [PATCH 6/9] avcodec/hashtable: Combine allocations
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index ec8eca471f..9b37ce3d69 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -37,7 +37,7 @@ struct FFHashtableContext {
size_t nb_entries;
const AVCRC *crc;
uint8_t *table;
- uint8_t *swapbuf;
+ uint8_t swapbuf[];
};
/*
@@ -59,10 +59,11 @@ int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t
const size_t keyval_size = key_size + val_size;
if (keyval_size < key_size || // did (unsigned,defined) wraparound happen?
- keyval_size > SIZE_MAX - sizeof(size_t) - (ALIGN - 1))
+ keyval_size > FFMIN(SIZE_MAX - sizeof(size_t) - (ALIGN - 1),
+ (SIZE_MAX - sizeof(FFHashtableContext)) / 2))
return AVERROR(ERANGE);
- FFHashtableContext *res = av_mallocz(sizeof(*res));
+ FFHashtableContext *res = av_mallocz(sizeof(*res) + 2 * keyval_size);
if (!res)
return AVERROR(ENOMEM);
res->key_size = key_size;
@@ -81,11 +82,6 @@ int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t
return AVERROR(ENOMEM);
}
- res->swapbuf = av_calloc(2, res->key_size + res->val_size);
- if (!res->swapbuf) {
- ff_hashtable_freep(&res);
- return AVERROR(ENOMEM);
- }
*ctx = res;
return 0;
}
@@ -208,7 +204,6 @@ void ff_hashtable_freep(struct FFHashtableContext **ctx)
{
if (*ctx) {
av_freep(&(*ctx)->table);
- av_freep(&(*ctx)->swapbuf);
}
av_freep(ctx);
}
--
2.45.2
[-- Attachment #8: 0007-avcodec-hashtable-Mark-alloc-free-functions-as-av_co.patch --]
[-- Type: text/x-patch, Size: 1554 bytes --]
From 1ad8db9dcd8c0e153477d87111af079fbe344f39 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 22:47:26 +0200
Subject: [PATCH 7/9] avcodec/hashtable: Mark alloc,free functions as av_cold
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index 9b37ce3d69..0e9b3d88c2 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -22,8 +22,10 @@
#include <stdint.h>
#include <string.h>
+#include "libavutil/attributes.h"
#include "libavutil/crc.h"
#include "libavutil/error.h"
+#include "libavutil/macros.h"
#include "libavutil/mem.h"
#include "hashtable.h"
@@ -54,7 +56,8 @@ struct FFHashtableContext {
#define KEYS_EQUAL(k1, k2) (!memcmp((k1), (k2), ctx->key_size))
-int ff_hashtable_alloc(struct FFHashtableContext **ctx, size_t key_size, size_t val_size, size_t max_entries)
+av_cold int ff_hashtable_alloc(FFHashtableContext **ctx, size_t key_size,
+ size_t val_size, size_t max_entries)
{
const size_t keyval_size = key_size + val_size;
@@ -200,7 +203,7 @@ void ff_hashtable_clear(struct FFHashtableContext *ctx)
memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
}
-void ff_hashtable_freep(struct FFHashtableContext **ctx)
+av_cold void ff_hashtable_freep(FFHashtableContext **ctx)
{
if (*ctx) {
av_freep(&(*ctx)->table);
--
2.45.2
[-- Attachment #9: 0008-avcodec-hashtable-Only-free-buffer-if-there-is-buffe.patch --]
[-- Type: text/x-patch, Size: 756 bytes --]
From 5aca3c9d1c7e981006205c7d74bff7a4707a81e7 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 22:50:32 +0200
Subject: [PATCH 8/9] avcodec/hashtable: Only free buffer if there is buffer to
free
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index 0e9b3d88c2..0d5c816cd7 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -207,6 +207,6 @@ av_cold void ff_hashtable_freep(FFHashtableContext **ctx)
{
if (*ctx) {
av_freep(&(*ctx)->table);
+ av_freep(ctx);
}
- av_freep(ctx);
}
--
2.45.2
[-- Attachment #10: 0009-avcodec-hashtable-Remove-null-statement.patch --]
[-- Type: text/x-patch, Size: 737 bytes --]
From ca0cfc25b57a097221334316bc09384476840a6c Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Tue, 3 Jun 2025 23:18:21 +0200
Subject: [PATCH 9/9] avcodec/hashtable: Remove null statement
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hashtable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/hashtable.c b/libavcodec/hashtable.c
index 0d5c816cd7..d18e872f4f 100644
--- a/libavcodec/hashtable.c
+++ b/libavcodec/hashtable.c
@@ -194,7 +194,7 @@ int ff_hashtable_delete(struct FFHashtableContext *ctx, const void *key)
entry = next_entry;
}
}
- };
+ }
return 0;
}
--
2.45.2
[-- Attachment #11: 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".
next reply other threads:[~2025-06-03 21:32 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-03 21:32 Andreas Rheinhardt [this message]
2025-06-04 6:10 ` Emma Worley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=GV1P250MB0737A0698A0CBA8EC7C1807B8F6DA@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM \
--to=andreas.rheinhardt@outlook.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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