| Message ID | 20200406175218.1299994-15-jstebbins@jetheaddev.com |
|---|---|
| State | Accepted |
| Commit | 1bf0df4c4f52df044b57ff477418cae9c24820f8 |
| Headers | show |
| Series | [FFmpeg-devel,01/23] lavc/movtextdec: fix ass header colors | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | warning | Failed to apply patch |
On Mon, 6 Apr 2020 11:52:09 -0600 John Stebbins <jstebbins@jetheaddev.com> wrote: > Makes style update code easier to extend for style types not yet > handled --- > libavcodec/movtextenc.c | 131 > +++++++++++++++++++--------------------- 1 file changed, 62 > insertions(+), 69 deletions(-) > > diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c > index 5e5b786f44..05532cd544 100644 > --- a/libavcodec/movtextenc.c > +++ b/libavcodec/movtextenc.c > @@ -89,6 +89,10 @@ static void mov_text_cleanup(MovTextContext *s) > } > av_freep(&s->style_attributes); > } > + if (s->style_attributes_temp) { > + s->style_attributes_temp->style_flag = 0; > + s->style_attributes_temp->style_start = 0; > + } > } > > static void encode_styl(MovTextContext *s, uint32_t tsmb_type) > @@ -96,7 +100,7 @@ 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) { > + if ((s->box_flags & STYL_BOX) && s->count) { > tsmb_size = s->count * STYLE_RECORD_SIZE + SIZE_ADD; > tsmb_size = AV_RB32(&tsmb_size); > style_entries = AV_RB16(&s->count); > @@ -120,8 +124,8 @@ static void encode_styl(MovTextContext *s, > uint32_t tsmb_type) av_bprint_append_any(&s->buffer, > &s->style_fontsize, 1); av_bprint_append_any(&s->buffer, > &s->style_color, 4); } > - mov_text_cleanup(s); > } > + mov_text_cleanup(s); > } > > static void encode_hlit(MovTextContext *s, uint32_t tsmb_type) > @@ -201,6 +205,11 @@ static av_cold int > mov_text_encode_init(AVCodecContext *avctx) MovTextContext *s = > avctx->priv_data; s->avctx = avctx; > > + s->style_attributes_temp = > av_mallocz(sizeof(*s->style_attributes_temp)); > + if (!s->style_attributes_temp) { > + return AVERROR(ENOMEM); > + } > + > avctx->extradata_size = sizeof text_sample_entry; > avctx->extradata = av_mallocz(avctx->extradata_size + > AV_INPUT_BUFFER_PADDING_SIZE); if (!avctx->extradata) > @@ -214,85 +223,69 @@ static av_cold int > mov_text_encode_init(AVCodecContext *avctx) return s->ass_ctx ? 0 : > AVERROR_INVALIDDATA; } > > -static void mov_text_style_cb(void *priv, const char style, int > close) +// Start a new style box if needed > +static int mov_text_style_start(MovTextContext *s) > { > - MovTextContext *s = priv; > - if (!close) { > - if (!(s->box_flags & STYL_BOX)) { //first style entry > - > - s->style_attributes_temp = > av_malloc(sizeof(*s->style_attributes_temp)); - > - if (!s->style_attributes_temp) { > - av_bprint_clear(&s->buffer); > - s->box_flags &= ~STYL_BOX; > - return; > - } > - > - s->style_attributes_temp->style_flag = 0; > - s->style_attributes_temp->style_start = s->text_pos; > - } else { > - if (s->style_attributes_temp->style_flag) { //break the > style record here and start a new one > - 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) { > - mov_text_cleanup(s); > - av_bprint_clear(&s->buffer); > - s->box_flags &= ~STYL_BOX; > - return; > - } > - > - s->style_attributes_temp->style_flag = > s->style_attributes[s->count - 1]->style_flag; > - s->style_attributes_temp->style_start = s->text_pos; > - } else { > - s->style_attributes_temp->style_flag = 0; > - s->style_attributes_temp->style_start = s->text_pos; > - } > - } > - switch (style){ > - case 'b': > - s->style_attributes_temp->style_flag |= STYLE_FLAG_BOLD; > - break; > - case 'i': > - s->style_attributes_temp->style_flag |= > STYLE_FLAG_ITALIC; > - break; > - case 'u': > - s->style_attributes_temp->style_flag |= > STYLE_FLAG_UNDERLINE; > - break; > - } > - } else if (!s->style_attributes_temp) { > - av_log(s->avctx, AV_LOG_WARNING, "Ignoring unmatched close > tag\n"); > - return; > - } else { > + // there's an existing style entry > + 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) { > + // 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) { > mov_text_cleanup(s); > av_bprint_clear(&s->buffer); > s->box_flags &= ~STYL_BOX; > - return; > + return 0; > } > > s->style_attributes_temp->style_flag = > s->style_attributes[s->count - 1]->style_flag; > - switch (style){ > - case 'b': > - s->style_attributes_temp->style_flag &= ~STYLE_FLAG_BOLD; > - break; > - case 'i': > - s->style_attributes_temp->style_flag &= > ~STYLE_FLAG_ITALIC; > - break; > - case 'u': > - s->style_attributes_temp->style_flag &= > ~STYLE_FLAG_UNDERLINE; > - break; > - } > - if (s->style_attributes_temp->style_flag) { //start of new > style record > - s->style_attributes_temp->style_start = s->text_pos; > - } > + s->style_attributes_temp->style_start = s->text_pos; > + } else { // style entry matches defaults, drop entry > + s->style_attributes_temp->style_flag = 0; > + s->style_attributes_temp->style_start = s->text_pos; > + } > + return 1; > +} > + > +static uint8_t mov_text_style_to_flag(const char style) > +{ > + uint8_t style_flag = 0; > + > + switch (style){ > + case 'b': > + style_flag = STYLE_FLAG_BOLD; > + break; > + case 'i': > + style_flag = STYLE_FLAG_ITALIC; > + break; > + case 'u': > + style_flag = STYLE_FLAG_UNDERLINE; > + break; > + } > + return style_flag; > +} > + > +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) { > + // setting flag that is already set > + return; > + } > + if (mov_text_style_start(s)) { > + if (!close) > + s->style_attributes_temp->style_flag |= style_flag; > + else > + s->style_attributes_temp->style_flag &= ~style_flag; > } > - s->box_flags |= STYL_BOX; > } > > static void mov_text_color_cb(void *priv, unsigned int color, > unsigned int color_id) LGTM. --phil
diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index 5e5b786f44..05532cd544 100644 --- a/libavcodec/movtextenc.c +++ b/libavcodec/movtextenc.c @@ -89,6 +89,10 @@ static void mov_text_cleanup(MovTextContext *s) } av_freep(&s->style_attributes); } + if (s->style_attributes_temp) { + s->style_attributes_temp->style_flag = 0; + s->style_attributes_temp->style_start = 0; + } } static void encode_styl(MovTextContext *s, uint32_t tsmb_type) @@ -96,7 +100,7 @@ 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) { + if ((s->box_flags & STYL_BOX) && s->count) { tsmb_size = s->count * STYLE_RECORD_SIZE + SIZE_ADD; tsmb_size = AV_RB32(&tsmb_size); style_entries = AV_RB16(&s->count); @@ -120,8 +124,8 @@ static void encode_styl(MovTextContext *s, uint32_t tsmb_type) av_bprint_append_any(&s->buffer, &s->style_fontsize, 1); av_bprint_append_any(&s->buffer, &s->style_color, 4); } - mov_text_cleanup(s); } + mov_text_cleanup(s); } static void encode_hlit(MovTextContext *s, uint32_t tsmb_type) @@ -201,6 +205,11 @@ static av_cold int mov_text_encode_init(AVCodecContext *avctx) MovTextContext *s = avctx->priv_data; s->avctx = avctx; + s->style_attributes_temp = av_mallocz(sizeof(*s->style_attributes_temp)); + if (!s->style_attributes_temp) { + return AVERROR(ENOMEM); + } + avctx->extradata_size = sizeof text_sample_entry; avctx->extradata = av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE); if (!avctx->extradata) @@ -214,85 +223,69 @@ static av_cold int mov_text_encode_init(AVCodecContext *avctx) return s->ass_ctx ? 0 : AVERROR_INVALIDDATA; } -static void mov_text_style_cb(void *priv, const char style, int close) +// Start a new style box if needed +static int mov_text_style_start(MovTextContext *s) { - MovTextContext *s = priv; - if (!close) { - if (!(s->box_flags & STYL_BOX)) { //first style entry - - s->style_attributes_temp = av_malloc(sizeof(*s->style_attributes_temp)); - - if (!s->style_attributes_temp) { - av_bprint_clear(&s->buffer); - s->box_flags &= ~STYL_BOX; - return; - } - - s->style_attributes_temp->style_flag = 0; - s->style_attributes_temp->style_start = s->text_pos; - } else { - if (s->style_attributes_temp->style_flag) { //break the style record here and start a new one - 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) { - mov_text_cleanup(s); - av_bprint_clear(&s->buffer); - s->box_flags &= ~STYL_BOX; - return; - } - - s->style_attributes_temp->style_flag = s->style_attributes[s->count - 1]->style_flag; - s->style_attributes_temp->style_start = s->text_pos; - } else { - s->style_attributes_temp->style_flag = 0; - s->style_attributes_temp->style_start = s->text_pos; - } - } - switch (style){ - case 'b': - s->style_attributes_temp->style_flag |= STYLE_FLAG_BOLD; - break; - case 'i': - s->style_attributes_temp->style_flag |= STYLE_FLAG_ITALIC; - break; - case 'u': - s->style_attributes_temp->style_flag |= STYLE_FLAG_UNDERLINE; - break; - } - } else if (!s->style_attributes_temp) { - av_log(s->avctx, AV_LOG_WARNING, "Ignoring unmatched close tag\n"); - return; - } else { + // there's an existing style entry + 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) { + // 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) { mov_text_cleanup(s); av_bprint_clear(&s->buffer); s->box_flags &= ~STYL_BOX; - return; + return 0; } s->style_attributes_temp->style_flag = s->style_attributes[s->count - 1]->style_flag; - switch (style){ - case 'b': - s->style_attributes_temp->style_flag &= ~STYLE_FLAG_BOLD; - break; - case 'i': - s->style_attributes_temp->style_flag &= ~STYLE_FLAG_ITALIC; - break; - case 'u': - s->style_attributes_temp->style_flag &= ~STYLE_FLAG_UNDERLINE; - break; - } - if (s->style_attributes_temp->style_flag) { //start of new style record - s->style_attributes_temp->style_start = s->text_pos; - } + s->style_attributes_temp->style_start = s->text_pos; + } else { // style entry matches defaults, drop entry + s->style_attributes_temp->style_flag = 0; + s->style_attributes_temp->style_start = s->text_pos; + } + return 1; +} + +static uint8_t mov_text_style_to_flag(const char style) +{ + uint8_t style_flag = 0; + + switch (style){ + case 'b': + style_flag = STYLE_FLAG_BOLD; + break; + case 'i': + style_flag = STYLE_FLAG_ITALIC; + break; + case 'u': + style_flag = STYLE_FLAG_UNDERLINE; + break; + } + return style_flag; +} + +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) { + // setting flag that is already set + return; + } + if (mov_text_style_start(s)) { + if (!close) + s->style_attributes_temp->style_flag |= style_flag; + else + s->style_attributes_temp->style_flag &= ~style_flag; } - s->box_flags |= STYL_BOX; } static void mov_text_color_cb(void *priv, unsigned int color, unsigned int color_id)