summaryrefslogtreecommitdiffstats
path: root/libass
diff options
context:
space:
mode:
authorUoti Urpala <uau@glyph.nonexistent.invalid>2008-12-21 23:32:07 +0200
committerUoti Urpala <uau@glyph.nonexistent.invalid>2008-12-22 00:46:52 +0200
commit312d9e4b104741b834aa5d71b02228d0cd988a4e (patch)
treec287118d1de9612fca06239205d7d745b0147c2d /libass
parentfccb0a7e45b61128262a396f2426bd7168fd198c (diff)
downloadmpv-312d9e4b104741b834aa5d71b02228d0cd988a4e.tar.bz2
mpv-312d9e4b104741b834aa5d71b02228d0cd988a4e.tar.xz
libass: Fix cache lookup problem causing memory bloat
The cache code did hash lookups by storing key values in struct fields and then hashing and comparing the struct as a single memory block. In at least one case such a struct contained uninitialized padding bytes which prevented the complete memory area of the struct from matching even though the fields did. As a result the code failed to find existing objects in the cache and stored new versions of them, causing gigabytes of memory use in some circumstances. Initializing the struct memory to zero before writing the fields avoided such memory use in tests but is not guaranteed to work if I interpret the C standard correctly (the compiler is allowed to write garbage over padding bytes when changing struct member values). Change the code to use struct-specific hashing and comparison functions that work field by field to guarantee correct behavior. Create these by replacing the struct definition with a template that lists the fields and can be used the generate each of struct definition, hash function and compare function with some preprocessor magic (otherwise every field would need to be listed separately in all three).
Diffstat (limited to 'libass')
-rw-r--r--libass/ass_cache.c12
-rw-r--r--libass/ass_cache.h32
-rw-r--r--libass/ass_cache_template.c87
3 files changed, 100 insertions, 31 deletions
diff --git a/libass/ass_cache.c b/libass/ass_cache.c
index 41645c4a48..8c4c91e739 100644
--- a/libass/ass_cache.c
+++ b/libass/ass_cache.c
@@ -228,6 +228,13 @@ void ass_font_cache_done(void)
hashmap_done(font_cache);
}
+
+// Create hash/compare functions for bitmap and glyph
+#define CREATE_HASH_FUNCTIONS
+#include "ass_cache_template.c"
+#define CREATE_COMPARISON_FUNCTIONS
+#include "ass_cache_template.c"
+
//---------------------------------
// bitmap cache
@@ -263,7 +270,8 @@ void ass_bitmap_cache_init(void)
bitmap_cache = hashmap_init(sizeof(bitmap_hash_key_t),
sizeof(bitmap_hash_val_t),
0xFFFF + 13,
- bitmap_hash_dtor, NULL, NULL);
+ bitmap_hash_dtor, bitmap_compare,
+ bitmap_hash);
}
void ass_bitmap_cache_done(void)
@@ -311,7 +319,7 @@ void ass_glyph_cache_init(void)
glyph_cache = hashmap_init(sizeof(glyph_hash_key_t),
sizeof(glyph_hash_val_t),
0xFFFF + 13,
- glyph_hash_dtor, NULL, NULL);
+ glyph_hash_dtor, glyph_compare, glyph_hash);
}
void ass_glyph_cache_done(void)
diff --git a/libass/ass_cache.h b/libass/ass_cache.h
index 5f6a2ddd94..a76d935992 100644
--- a/libass/ass_cache.h
+++ b/libass/ass_cache.h
@@ -33,24 +33,9 @@ void* ass_font_cache_add(ass_font_t* font);
void ass_font_cache_done(void);
-// describes a bitmap; bitmaps with equivalents structs are considered identical
-typedef struct bitmap_hash_key_s {
- char bitmap; // bool : true = bitmap, false = outline
- ass_font_t* font;
- double size; // font size
- uint32_t ch; // character code
- unsigned outline; // border width, 16.16 fixed point value
- int bold, italic;
- char be; // blur edges
-
- unsigned scale_x, scale_y; // 16.16
- int frx, fry, frz; // signed 16.16
- int shift_x, shift_y; // shift vector that was added to glyph before applying rotation
- // = 0, if frx = fry = frx = 0
- // = (glyph base point) - (rotation origin), otherwise
-
- FT_Vector advance; // subpixel shift vector
-} bitmap_hash_key_t;
+// Create definitions for bitmap_hash_key and glyph_hash_key
+#define CREATE_STRUCT_DEFINITIONS
+#include "ass_cache_template.c"
typedef struct bitmap_hash_val_s {
bitmap_t* bm; // the actual bitmaps
@@ -64,17 +49,6 @@ bitmap_hash_val_t* cache_find_bitmap(bitmap_hash_key_t* key);
void ass_bitmap_cache_reset(void);
void ass_bitmap_cache_done(void);
-// describes an outline glyph
-typedef struct glyph_hash_key_s {
- ass_font_t* font;
- double size; // font size
- uint32_t ch; // character code
- int bold, italic;
- unsigned scale_x, scale_y; // 16.16
- FT_Vector advance; // subpixel shift vector
- unsigned outline; // border width, 16.16
-} glyph_hash_key_t;
-
typedef struct glyph_hash_val_s {
FT_Glyph glyph;
FT_Glyph outline_glyph;
diff --git a/libass/ass_cache_template.c b/libass/ass_cache_template.c
new file mode 100644
index 0000000000..5f77190fa1
--- /dev/null
+++ b/libass/ass_cache_template.c
@@ -0,0 +1,87 @@
+#ifdef CREATE_STRUCT_DEFINITIONS
+#undef CREATE_STRUCT_DEFINITIONS
+#define START(funcname, structname) \
+ typedef struct structname {
+#define GENERIC(type, member) \
+ type member;
+#define FTVECTOR(member) \
+ FT_Vector member;
+#define END(typedefnamename) \
+ } typedefnamename;
+
+#elif defined(CREATE_COMPARISON_FUNCTIONS)
+#undef CREATE_COMPARISON_FUNCTIONS
+#define START(funcname, structname) \
+ static int funcname##_compare(void *key1, void *key2, size_t key_size) \
+ { \
+ struct structname *a = key1; \
+ struct structname *b = key2; \
+ return // conditions follow
+#define GENERIC(type, member) \
+ a->member == b->member &&
+#define FTVECTOR(member) \
+ a->member.x == b->member.x && a->member.y == b->member.y &&
+#define END(typedefname) \
+ 1; \
+ }
+
+#elif defined(CREATE_HASH_FUNCTIONS)
+#undef CREATE_HASH_FUNCTIONS
+#define START(funcname, structname) \
+ static unsigned funcname##_hash(void *buf, size_t len) \
+ { \
+ struct structname *p = buf; \
+ unsigned hval = FNV1_32A_INIT;
+#define GENERIC(type, member) \
+ hval = fnv_32a_buf(&p->member, sizeof(p->member), hval);
+#define FTVECTOR(member) GENERIC(, member.x); GENERIC(, member.y);
+#define END(typedefname) \
+ return hval; \
+ }
+
+#else
+#error missing defines
+#endif
+
+
+
+// describes a bitmap; bitmaps with equivalents structs are considered identical
+START(bitmap, bipmap_hash_key_s)
+ GENERIC(char, bitmap) // bool : true = bitmap, false = outline
+ GENERIC(ass_font_t *, font)
+ GENERIC(double, size) // font size
+ GENERIC(uint32_t, ch) // character code
+ GENERIC(unsigned, outline) // border width, 16.16 fixed point value
+ GENERIC(int, bold)
+ GENERIC(int, italic)
+ GENERIC(char, be) // blur edges
+ GENERIC(unsigned, scale_x) // 16.16
+ GENERIC(unsigned, scale_y) // 16.16
+ GENERIC(int, frx) // signed 16.16
+ GENERIC(int, fry) // signed 16.16
+ GENERIC(int, frz) // signed 16.16
+ // shift vector that was added to glyph before applying rotation
+ // = 0, if frx = fry = frx = 0
+ // = (glyph base point) - (rotation origin), otherwise
+ GENERIC(int, shift_x)
+ GENERIC(int, shift_y)
+ FTVECTOR(advance) // subpixel shift vector
+END(bitmap_hash_key_t)
+
+// describes an outline glyph
+START(glyph, glyph_hash_key_s)
+ GENERIC(ass_font_t *, font)
+ GENERIC(double, size) // font size
+ GENERIC(uint32_t, ch) // character code
+ GENERIC(int, bold)
+ GENERIC(int, italic)
+ GENERIC(unsigned, scale_x) // 16.16
+ GENERIC(unsigned, scale_y) // 16.16
+ FTVECTOR(advance) // subpixel shift vector
+ GENERIC(unsigned, outline) // border width, 16.16
+END(glyph_hash_key_t)
+
+#undef START
+#undef GENERIC
+#undef FTVECTOR
+#undef END