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).

Originally committed to SVN as r2895.
This commit is contained in:
Amar Takhar 2009-05-04 06:48:21 +00:00
parent b55460ede5
commit c2267bedcc
4 changed files with 109 additions and 37 deletions

View file

@ -230,6 +230,13 @@ void ass_font_cache_done(void)
hashmap_done(font_cache); 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 // bitmap cache
@ -265,7 +272,8 @@ void ass_bitmap_cache_init(void)
bitmap_cache = hashmap_init(sizeof(bitmap_hash_key_t), bitmap_cache = hashmap_init(sizeof(bitmap_hash_key_t),
sizeof(bitmap_hash_val_t), sizeof(bitmap_hash_val_t),
0xFFFF + 13, 0xFFFF + 13,
bitmap_hash_dtor, NULL, NULL); bitmap_hash_dtor, bitmap_compare,
bitmap_hash);
} }
void ass_bitmap_cache_done(void) void ass_bitmap_cache_done(void)
@ -313,7 +321,7 @@ void ass_glyph_cache_init(void)
glyph_cache = hashmap_init(sizeof(glyph_hash_key_t), glyph_cache = hashmap_init(sizeof(glyph_hash_key_t),
sizeof(glyph_hash_val_t), sizeof(glyph_hash_val_t),
0xFFFF + 13, 0xFFFF + 13,
glyph_hash_dtor, NULL, NULL); glyph_hash_dtor, glyph_compare, glyph_hash);
} }
void ass_glyph_cache_done(void) void ass_glyph_cache_done(void)

View file

@ -33,25 +33,9 @@ void* ass_font_cache_add(ass_font_t* font);
void ass_font_cache_done(void); void ass_font_cache_done(void);
// describes a bitmap; bitmaps with equivalents structs are considered identical // Create definitions for bitmap_hash_key and glyph_hash_key
typedef struct bitmap_hash_key_s { #define CREATE_STRUCT_DEFINITIONS
char bitmap; // bool : true = bitmap, false = outline #include "ass_cache_template.c"
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
double blur; // gaussian blur
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;
typedef struct bitmap_hash_val_s { typedef struct bitmap_hash_val_s {
bitmap_t* bm; // the actual bitmaps bitmap_t* bm; // the actual bitmaps
@ -86,17 +70,6 @@ void ass_composite_cache_reset(void);
void ass_composite_cache_done(void); void ass_composite_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 { typedef struct glyph_hash_val_s {
FT_Glyph glyph; FT_Glyph glyph;
FT_Glyph outline_glyph; FT_Glyph outline_glyph;

View file

@ -0,0 +1,88 @@
#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(double, blur) // gaussian blur
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

View file

@ -1018,7 +1018,10 @@ static char* parse_tag(char* p, double pwr) {
mystrtod(&p, &v2); mystrtod(&p, &v2);
skip(')'); skip(')');
mp_msg(MSGT_ASS, MSGL_DBG2, "pos(%f, %f)\n", v1, v2); mp_msg(MSGT_ASS, MSGL_DBG2, "pos(%f, %f)\n", v1, v2);
if (render_context.evt_type != EVENT_POSITIONED) { if (render_context.evt_type == EVENT_POSITIONED) {
mp_msg(MSGT_ASS, MSGL_V, "Subtitle has a new \\pos "
"after \\move or \\pos, ignoring\n");
} else {
render_context.evt_type = EVENT_POSITIONED; render_context.evt_type = EVENT_POSITIONED;
render_context.detect_collisions = 0; render_context.detect_collisions = 0;
render_context.pos_x = v1; render_context.pos_x = v1;
@ -1603,7 +1606,7 @@ static void wrap_lines_smart(int max_text_width)
mp_msg(MSGT_ASS, MSGL_DBG2, "forced line break at %d\n", break_at); mp_msg(MSGT_ASS, MSGL_DBG2, "forced line break at %d\n", break_at);
} }
if ((len >= max_text_width) && (frame_context.track->WrapStyle != 2)) { if (len >= max_text_width) {
break_type = 1; break_type = 1;
break_at = last_space; break_at = last_space;
if (break_at == -1) if (break_at == -1)
@ -2186,7 +2189,7 @@ static int ass_render_event(ass_event_t* event, event_images_t* event_images)
* \brief deallocate image list * \brief deallocate image list
* \param img list pointer * \param img list pointer
*/ */
void ass_free_images(ass_image_t* img) static void ass_free_images(ass_image_t* img)
{ {
while (img) { while (img) {
ass_image_t* next = img->next; ass_image_t* next = img->next;
@ -2495,7 +2498,7 @@ static void fix_collisions(event_images_t* imgs, int cnt)
* \param i2 second image * \param i2 second image
* \return 0 if identical, 1 if different positions, 2 if different content * \return 0 if identical, 1 if different positions, 2 if different content
*/ */
int ass_image_compare(ass_image_t *i1, ass_image_t *i2) static int ass_image_compare(ass_image_t *i1, ass_image_t *i2)
{ {
if (i1->w != i2->w) return 2; if (i1->w != i2->w) return 2;
if (i1->h != i2->h) return 2; if (i1->h != i2->h) return 2;
@ -2513,7 +2516,7 @@ int ass_image_compare(ass_image_t *i1, ass_image_t *i2)
* \param priv library handle * \param priv library handle
* \return 0 if identical, 1 if different positions, 2 if different content * \return 0 if identical, 1 if different positions, 2 if different content
*/ */
int ass_detect_change(ass_renderer_t *priv) static int ass_detect_change(ass_renderer_t *priv)
{ {
ass_image_t* img, *img2; ass_image_t* img, *img2;
int diff; int diff;