mirror of
https://git.videolan.org/git/ffmpeg.git
synced 2024-09-07 16:40:10 +02:00
avcodec/movtextenc: Fix memleak on (re)allocation error
Up until now, the mov_text encoder used the dynamic array API for its list of style attributes; it used the (horrible) av_dynarray_add() which works with an array of pointers; on error it frees its array but not the buffers referenced by the pointers said array contains. It also returns no error code, encouraging not to check for errors. These properties imply that this function may only be used if the buffers referenced by the list either need not be freed at all or if they are freed by other means (i.e. if the list contains non-ownership pointers). In this case, the style attributes are owned by the pointers of the dynamic list. Ergo the old style attributes leak on a subsequent reallocation failure. But given that the (re)allocation isn't checked for success, the style attribute intended to be added to the list also leaks because the only pointer to it gets overwritten in the belief that it is now owned by the list. This commit fixes this by switching to av_fast_realloc() and an array containing the styles directly instead of pointers to individually allocated style attributes. The current style attributes are now no longer individually allocated, instead they are part of the context. Furthermore, av_fast_realloc() allows to easily distinguish between valid and allocated elements, thereby allowing to reuse the array (which up until now has always been freed after processing an AVSubtitleRect). Reviewed-by: Philip Langdale <philipl@overt.org> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit is contained in:
parent
0dd7b8232d
commit
9a731e9fec
@ -73,12 +73,13 @@ typedef struct {
|
||||
|
||||
ASSSplitContext *ass_ctx;
|
||||
ASSStyle *ass_dialog_style;
|
||||
StyleBox *style_attributes;
|
||||
unsigned count;
|
||||
unsigned style_attributes_bytes_allocated;
|
||||
StyleBox style_attributes_temp;
|
||||
AVBPrint buffer;
|
||||
StyleBox **style_attributes;
|
||||
StyleBox *style_attributes_temp;
|
||||
HighlightBox hlit;
|
||||
HilightcolorBox hclr;
|
||||
int count;
|
||||
uint8_t box_flags;
|
||||
StyleBox d;
|
||||
uint16_t text_pos;
|
||||
@ -96,22 +97,12 @@ typedef struct {
|
||||
|
||||
static void mov_text_cleanup(MovTextContext *s)
|
||||
{
|
||||
int j;
|
||||
if (s->box_flags & STYL_BOX) {
|
||||
for (j = 0; j < s->count; j++) {
|
||||
av_freep(&s->style_attributes[j]);
|
||||
}
|
||||
av_freep(&s->style_attributes);
|
||||
s->count = 0;
|
||||
}
|
||||
if (s->style_attributes_temp) {
|
||||
*s->style_attributes_temp = s->d;
|
||||
}
|
||||
s->count = 0;
|
||||
s->style_attributes_temp = s->d;
|
||||
}
|
||||
|
||||
static void encode_styl(MovTextContext *s, uint32_t tsmb_type)
|
||||
{
|
||||
int j;
|
||||
uint32_t tsmb_size;
|
||||
uint16_t style_entries;
|
||||
if ((s->box_flags & STYL_BOX) && s->count) {
|
||||
@ -124,20 +115,20 @@ static void encode_styl(MovTextContext *s, uint32_t tsmb_type)
|
||||
av_bprint_append_any(&s->buffer, &tsmb_size, 4);
|
||||
av_bprint_append_any(&s->buffer, &tsmb_type, 4);
|
||||
av_bprint_append_any(&s->buffer, &style_entries, 2);
|
||||
for (j = 0; j < s->count; j++) {
|
||||
for (unsigned j = 0; j < s->count; j++) {
|
||||
uint16_t style_start, style_end, style_fontID;
|
||||
uint32_t style_color;
|
||||
|
||||
style_start = AV_RB16(&s->style_attributes[j]->style_start);
|
||||
style_end = AV_RB16(&s->style_attributes[j]->style_end);
|
||||
style_color = AV_RB32(&s->style_attributes[j]->style_color);
|
||||
style_fontID = AV_RB16(&s->style_attributes[j]->style_fontID);
|
||||
style_start = AV_RB16(&s->style_attributes[j].style_start);
|
||||
style_end = AV_RB16(&s->style_attributes[j].style_end);
|
||||
style_color = AV_RB32(&s->style_attributes[j].style_color);
|
||||
style_fontID = AV_RB16(&s->style_attributes[j].style_fontID);
|
||||
|
||||
av_bprint_append_any(&s->buffer, &style_start, 2);
|
||||
av_bprint_append_any(&s->buffer, &style_end, 2);
|
||||
av_bprint_append_any(&s->buffer, &style_fontID, 2);
|
||||
av_bprint_append_any(&s->buffer, &s->style_attributes[j]->style_flag, 1);
|
||||
av_bprint_append_any(&s->buffer, &s->style_attributes[j]->style_fontsize, 1);
|
||||
av_bprint_append_any(&s->buffer, &s->style_attributes[j].style_flag, 1);
|
||||
av_bprint_append_any(&s->buffer, &s->style_attributes[j].style_fontsize, 1);
|
||||
av_bprint_append_any(&s->buffer, &style_color, 4);
|
||||
}
|
||||
}
|
||||
@ -186,17 +177,10 @@ const static size_t box_count = FF_ARRAY_ELEMS(box_types);
|
||||
static int mov_text_encode_close(AVCodecContext *avctx)
|
||||
{
|
||||
MovTextContext *s = avctx->priv_data;
|
||||
int i;
|
||||
|
||||
ff_ass_split_free(s->ass_ctx);
|
||||
if (s->style_attributes) {
|
||||
for (i = 0; i < s->count; i++) {
|
||||
av_freep(&s->style_attributes[i]);
|
||||
}
|
||||
av_freep(&s->style_attributes);
|
||||
}
|
||||
av_freep(&s->style_attributes);
|
||||
av_freep(&s->fonts);
|
||||
av_freep(&s->style_attributes_temp);
|
||||
av_bprint_finalize(&s->buffer, NULL);
|
||||
return 0;
|
||||
}
|
||||
@ -368,12 +352,6 @@ static av_cold int mov_text_encode_init(AVCodecContext *avctx)
|
||||
|
||||
av_bprint_init(&s->buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
|
||||
|
||||
s->style_attributes_temp = av_mallocz(sizeof(*s->style_attributes_temp));
|
||||
if (!s->style_attributes_temp) {
|
||||
ret = AVERROR(ENOMEM);
|
||||
goto fail;
|
||||
}
|
||||
|
||||
s->ass_ctx = ff_ass_split(avctx->subtitle_header);
|
||||
if (!s->ass_ctx) {
|
||||
ret = AVERROR_INVALIDDATA;
|
||||
@ -394,30 +372,34 @@ fail:
|
||||
static int mov_text_style_start(MovTextContext *s)
|
||||
{
|
||||
// there's an existing style entry
|
||||
if (s->style_attributes_temp->style_start == s->text_pos)
|
||||
if (s->style_attributes_temp.style_start == s->text_pos)
|
||||
// Still at same text pos, use same entry
|
||||
return 1;
|
||||
if (s->style_attributes_temp->style_flag != s->d.style_flag ||
|
||||
s->style_attributes_temp->style_color != s->d.style_color ||
|
||||
s->style_attributes_temp->style_fontID != s->d.style_fontID ||
|
||||
s->style_attributes_temp->style_fontsize != s->d.style_fontsize) {
|
||||
if (s->style_attributes_temp.style_flag != s->d.style_flag ||
|
||||
s->style_attributes_temp.style_color != s->d.style_color ||
|
||||
s->style_attributes_temp.style_fontID != s->d.style_fontID ||
|
||||
s->style_attributes_temp.style_fontsize != s->d.style_fontsize) {
|
||||
StyleBox *tmp;
|
||||
|
||||
// last style != defaults, end the style entry and start a new one
|
||||
s->box_flags |= STYL_BOX;
|
||||
s->style_attributes_temp->style_end = s->text_pos;
|
||||
av_dynarray_add(&s->style_attributes, &s->count, s->style_attributes_temp);
|
||||
s->style_attributes_temp = av_malloc(sizeof(*s->style_attributes_temp));
|
||||
if (!s->style_attributes_temp) {
|
||||
if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
|
||||
!(tmp = av_fast_realloc(s->style_attributes,
|
||||
&s->style_attributes_bytes_allocated,
|
||||
(s->count + 1) * sizeof(*s->style_attributes)))) {
|
||||
mov_text_cleanup(s);
|
||||
av_bprint_clear(&s->buffer);
|
||||
s->box_flags &= ~STYL_BOX;
|
||||
return 0;
|
||||
}
|
||||
|
||||
*s->style_attributes_temp = s->d;
|
||||
s->style_attributes_temp->style_start = s->text_pos;
|
||||
s->style_attributes = tmp;
|
||||
s->style_attributes_temp.style_end = s->text_pos;
|
||||
s->style_attributes[s->count++] = s->style_attributes_temp;
|
||||
s->box_flags |= STYL_BOX;
|
||||
s->style_attributes_temp = s->d;
|
||||
s->style_attributes_temp.style_start = s->text_pos;
|
||||
} else { // style entry matches defaults, drop entry
|
||||
*s->style_attributes_temp = s->d;
|
||||
s->style_attributes_temp->style_start = s->text_pos;
|
||||
s->style_attributes_temp = s->d;
|
||||
s->style_attributes_temp.style_start = s->text_pos;
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
@ -442,13 +424,12 @@ static uint8_t mov_text_style_to_flag(const char style)
|
||||
|
||||
static void mov_text_style_set(MovTextContext *s, uint8_t style_flags)
|
||||
{
|
||||
if (!s->style_attributes_temp ||
|
||||
!((s->style_attributes_temp->style_flag & style_flags) ^ style_flags)) {
|
||||
if (!((s->style_attributes_temp.style_flag & style_flags) ^ style_flags)) {
|
||||
// setting flags that that are already set
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s))
|
||||
s->style_attributes_temp->style_flag |= style_flags;
|
||||
s->style_attributes_temp.style_flag |= style_flags;
|
||||
}
|
||||
|
||||
static void mov_text_style_cb(void *priv, const char style, int close)
|
||||
@ -456,29 +437,27 @@ static void mov_text_style_cb(void *priv, const char style, int close)
|
||||
MovTextContext *s = priv;
|
||||
uint8_t style_flag = mov_text_style_to_flag(style);
|
||||
|
||||
if (!s->style_attributes_temp ||
|
||||
!!(s->style_attributes_temp->style_flag & style_flag) != close) {
|
||||
if (!!(s->style_attributes_temp.style_flag & style_flag) != close) {
|
||||
// setting flag that is already set
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s)) {
|
||||
if (!close)
|
||||
s->style_attributes_temp->style_flag |= style_flag;
|
||||
s->style_attributes_temp.style_flag |= style_flag;
|
||||
else
|
||||
s->style_attributes_temp->style_flag &= ~style_flag;
|
||||
s->style_attributes_temp.style_flag &= ~style_flag;
|
||||
}
|
||||
}
|
||||
|
||||
static void mov_text_color_set(MovTextContext *s, uint32_t color)
|
||||
{
|
||||
if (!s->style_attributes_temp ||
|
||||
(s->style_attributes_temp->style_color & 0xffffff00) == color) {
|
||||
if ((s->style_attributes_temp.style_color & 0xffffff00) == color) {
|
||||
// color hasn't changed
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s))
|
||||
s->style_attributes_temp->style_color = (color & 0xffffff00) |
|
||||
(s->style_attributes_temp->style_color & 0xff);
|
||||
s->style_attributes_temp.style_color = (color & 0xffffff00) |
|
||||
(s->style_attributes_temp.style_color & 0xff);
|
||||
}
|
||||
|
||||
static void mov_text_color_cb(void *priv, unsigned int color, unsigned int color_id)
|
||||
@ -491,7 +470,7 @@ static void mov_text_color_cb(void *priv, unsigned int color, unsigned int color
|
||||
} else if (color_id == 2) { //secondary color changes
|
||||
if (!(s->box_flags & HCLR_BOX))
|
||||
// Highlight alpha not set yet, use current primary alpha
|
||||
s->hclr.color = s->style_attributes_temp->style_color;
|
||||
s->hclr.color = s->style_attributes_temp.style_color;
|
||||
if (!(s->box_flags & HLIT_BOX) || s->hlit.start == s->text_pos) {
|
||||
s->box_flags |= HCLR_BOX;
|
||||
s->box_flags |= HLIT_BOX;
|
||||
@ -510,14 +489,13 @@ static void mov_text_color_cb(void *priv, unsigned int color, unsigned int color
|
||||
|
||||
static void mov_text_alpha_set(MovTextContext *s, uint8_t alpha)
|
||||
{
|
||||
if (!s->style_attributes_temp ||
|
||||
(s->style_attributes_temp->style_color & 0xff) == alpha) {
|
||||
if ((s->style_attributes_temp.style_color & 0xff) == alpha) {
|
||||
// color hasn't changed
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s))
|
||||
s->style_attributes_temp->style_color =
|
||||
(s->style_attributes_temp->style_color & 0xffffff00) | alpha;
|
||||
s->style_attributes_temp.style_color =
|
||||
(s->style_attributes_temp.style_color & 0xffffff00) | alpha;
|
||||
}
|
||||
|
||||
static void mov_text_alpha_cb(void *priv, int alpha, int alpha_id)
|
||||
@ -530,7 +508,7 @@ static void mov_text_alpha_cb(void *priv, int alpha, int alpha_id)
|
||||
else if (alpha_id == 2) { //secondary alpha changes
|
||||
if (!(s->box_flags & HCLR_BOX))
|
||||
// Highlight color not set yet, use current primary color
|
||||
s->hclr.color = s->style_attributes_temp->style_color;
|
||||
s->hclr.color = s->style_attributes_temp.style_color;
|
||||
if (!(s->box_flags & HLIT_BOX) || s->hlit.start == s->text_pos) {
|
||||
s->box_flags |= HCLR_BOX;
|
||||
s->box_flags |= HLIT_BOX;
|
||||
@ -556,13 +534,12 @@ static uint16_t find_font_id(MovTextContext *s, const char *name)
|
||||
static void mov_text_font_name_set(MovTextContext *s, const char *name)
|
||||
{
|
||||
int fontID = find_font_id(s, name);
|
||||
if (!s->style_attributes_temp ||
|
||||
s->style_attributes_temp->style_fontID == fontID) {
|
||||
if (s->style_attributes_temp.style_fontID == fontID) {
|
||||
// color hasn't changed
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s))
|
||||
s->style_attributes_temp->style_fontID = fontID;
|
||||
s->style_attributes_temp.style_fontID = fontID;
|
||||
}
|
||||
|
||||
static void mov_text_font_name_cb(void *priv, const char *name)
|
||||
@ -573,13 +550,12 @@ static void mov_text_font_name_cb(void *priv, const char *name)
|
||||
static void mov_text_font_size_set(MovTextContext *s, int size)
|
||||
{
|
||||
size = FONTSIZE_SCALE(s, size);
|
||||
if (!s->style_attributes_temp ||
|
||||
s->style_attributes_temp->style_fontsize == size) {
|
||||
if (s->style_attributes_temp.style_fontsize == size) {
|
||||
// color hasn't changed
|
||||
return;
|
||||
}
|
||||
if (mov_text_style_start(s))
|
||||
s->style_attributes_temp->style_fontsize = size;
|
||||
s->style_attributes_temp.style_fontsize = size;
|
||||
}
|
||||
|
||||
static void mov_text_font_size_cb(void *priv, int size)
|
||||
|
Loading…
Reference in New Issue
Block a user