Message ID | 20200406175218.1299994-3-jstebbins@jetheaddev.com |
---|---|
State | Accepted |
Commit | 2949f17e992be600c379112d4e84e8455bae180a |
Headers | show |
Series | [FFmpeg-devel,01/23] lavc/movtextdec: fix ass header colors | expand |
Context | Check | Description |
---|---|---|
andriy/ffmpeg-patchwork | success | Make fate finished |
On Mon, 6 Apr 2020 11:51:57 -0600 John Stebbins <jstebbins@jetheaddev.com> wrote: > It's not necessary to walk the style record list twice per subtitle > character. style records are in order and do not overlap. > --- > libavcodec/movtextdec.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c > index 05becaf64d..47a8401119 100644 > --- a/libavcodec/movtextdec.c > +++ b/libavcodec/movtextdec.c > @@ -355,8 +355,9 @@ static int text_to_ass(AVBPrint *buf, const char > *text, const char *text_end, { > MovTextContext *m = avctx->priv_data; > int i = 0; > - int j = 0; > int text_pos = 0; > + int style_active = 0; > + int entry = 0; > > if (text < text_end && m->box_flags & TWRP_BOX) { > if (m->w.wrap_flag == 1) { > @@ -369,26 +370,27 @@ static int text_to_ass(AVBPrint *buf, const > char *text, const char *text_end, while (text < text_end) { > int len; > > - if (m->box_flags & STYL_BOX) { > - for (i = 0; i < m->style_entries; i++) { > - if (m->s[i]->style_flag && text_pos == > m->s[i]->style_end) { > - av_bprintf(buf, "{\\r}"); > + if ((m->box_flags & STYL_BOX) && entry < m->style_entries) { > + if (text_pos == m->s[entry]->style_start) { > + style_active = 1; > + if (m->s[entry]->style_flag & STYLE_FLAG_BOLD) > + av_bprintf(buf, "{\\b1}"); > + if (m->s[entry]->style_flag & STYLE_FLAG_ITALIC) > + av_bprintf(buf, "{\\i1}"); > + if (m->s[entry]->style_flag & STYLE_FLAG_UNDERLINE) > + av_bprintf(buf, "{\\u1}"); > + av_bprintf(buf, "{\\fs%d}", m->s[entry]->fontsize); > + for (i = 0; i < m->ftab_entries; i++) { > + if (m->s[entry]->style_fontID == > m->ftab[i]->fontID) > + av_bprintf(buf, "{\\fn%s}", > m->ftab[i]->font); } > } > - for (i = 0; i < m->style_entries; i++) { > - if (m->s[i]->style_flag && text_pos == > m->s[i]->style_start) { > - if (m->s[i]->style_flag & STYLE_FLAG_BOLD) > - av_bprintf(buf, "{\\b1}"); > - if (m->s[i]->style_flag & STYLE_FLAG_ITALIC) > - av_bprintf(buf, "{\\i1}"); > - if (m->s[i]->style_flag & STYLE_FLAG_UNDERLINE) > - av_bprintf(buf, "{\\u1}"); > - av_bprintf(buf, "{\\fs%d}", m->s[i]->fontsize); > - for (j = 0; j < m->ftab_entries; j++) { > - if (m->s[i]->style_fontID == > m->ftab[j]->fontID) > - av_bprintf(buf, "{\\fn%s}", > m->ftab[j]->font); > - } > + if (text_pos == m->s[entry]->style_end) { > + if (style_active) { > + av_bprintf(buf, "{\\r}"); > + style_active = 0; > } > + entry++; > } > } > if (m->box_flags & HLIT_BOX) { OK. I could not convince myself originally that styles could not overlap, but I could easily have missed that in the spec. --phil
On Mon, 2020-04-06 at 14:46 -0700, Philip Langdale wrote: > On Mon, 6 Apr 2020 11:51:57 -0600 > John Stebbins <jstebbins@jetheaddev.com> wrote: > > > It's not necessary to walk the style record list twice per subtitle > > character. style records are in order and do not overlap. > > --- > > libavcodec/movtextdec.c | 38 ++++++++++++++++++++----------------- > > - > > 1 file changed, 20 insertions(+), 18 deletions(-) > > > > diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c > > index 05becaf64d..47a8401119 100644 > > --- a/libavcodec/movtextdec.c > > +++ b/libavcodec/movtextdec.c > > @@ -355,8 +355,9 @@ static int text_to_ass(AVBPrint *buf, const > > char > > *text, const char *text_end, { > > MovTextContext *m = avctx->priv_data; > > int i = 0; > > - int j = 0; > > int text_pos = 0; > > + int style_active = 0; > > + int entry = 0; > > > > if (text < text_end && m->box_flags & TWRP_BOX) { > > if (m->w.wrap_flag == 1) { > > @@ -369,26 +370,27 @@ static int text_to_ass(AVBPrint *buf, const > > char *text, const char *text_end, while (text < text_end) { > > int len; > > > > - if (m->box_flags & STYL_BOX) { > > - for (i = 0; i < m->style_entries; i++) { > > - if (m->s[i]->style_flag && text_pos == > > m->s[i]->style_end) { > > - av_bprintf(buf, "{\\r}"); > > + if ((m->box_flags & STYL_BOX) && entry < m->style_entries) > > { > > + if (text_pos == m->s[entry]->style_start) { > > + style_active = 1; > > + if (m->s[entry]->style_flag & STYLE_FLAG_BOLD) > > + av_bprintf(buf, "{\\b1}"); > > + if (m->s[entry]->style_flag & STYLE_FLAG_ITALIC) > > + av_bprintf(buf, "{\\i1}"); > > + if (m->s[entry]->style_flag & > > STYLE_FLAG_UNDERLINE) > > + av_bprintf(buf, "{\\u1}"); > > + av_bprintf(buf, "{\\fs%d}", m->s[entry]- > > >fontsize); > > + for (i = 0; i < m->ftab_entries; i++) { > > + if (m->s[entry]->style_fontID == > > m->ftab[i]->fontID) > > + av_bprintf(buf, "{\\fn%s}", > > m->ftab[i]->font); } > > } > > - for (i = 0; i < m->style_entries; i++) { > > - if (m->s[i]->style_flag && text_pos == > > m->s[i]->style_start) { > > - if (m->s[i]->style_flag & STYLE_FLAG_BOLD) > > - av_bprintf(buf, "{\\b1}"); > > - if (m->s[i]->style_flag & STYLE_FLAG_ITALIC) > > - av_bprintf(buf, "{\\i1}"); > > - if (m->s[i]->style_flag & > > STYLE_FLAG_UNDERLINE) > > - av_bprintf(buf, "{\\u1}"); > > - av_bprintf(buf, "{\\fs%d}", m->s[i]- > > >fontsize); > > - for (j = 0; j < m->ftab_entries; j++) { > > - if (m->s[i]->style_fontID == > > m->ftab[j]->fontID) > > - av_bprintf(buf, "{\\fn%s}", > > m->ftab[j]->font); > > - } > > + if (text_pos == m->s[entry]->style_end) { > > + if (style_active) { > > + av_bprintf(buf, "{\\r}"); > > + style_active = 0; > > } > > + entry++; > > } > > } > > if (m->box_flags & HLIT_BOX) { > > OK. I could not convince myself originally that styles could not > overlap, but I could easily have missed that in the spec. > I went back and double checked this in the spec before changing the code. "The styles shall be ordered by starting character offset, and the starting offset of one style record shall be greater than or equal to the ending character offset of the preceding record; styles records shall not overlap their character ranges."
On Mon, 6 Apr 2020 22:09:22 +0000 John Stebbins <jstebbins@jetheaddev.com> wrote: > > I went back and double checked this in the spec before changing the > code. > > "The styles shall be ordered by starting character offset, and the > starting offset of one style record shall be greater than or equal to > the ending character offset of the preceding record; styles records > shall not overlap their character ranges." Alright. Great! LGTM then. --phil
diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c index 05becaf64d..47a8401119 100644 --- a/libavcodec/movtextdec.c +++ b/libavcodec/movtextdec.c @@ -355,8 +355,9 @@ static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end, { MovTextContext *m = avctx->priv_data; int i = 0; - int j = 0; int text_pos = 0; + int style_active = 0; + int entry = 0; if (text < text_end && m->box_flags & TWRP_BOX) { if (m->w.wrap_flag == 1) { @@ -369,26 +370,27 @@ static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end, while (text < text_end) { int len; - if (m->box_flags & STYL_BOX) { - for (i = 0; i < m->style_entries; i++) { - if (m->s[i]->style_flag && text_pos == m->s[i]->style_end) { - av_bprintf(buf, "{\\r}"); + if ((m->box_flags & STYL_BOX) && entry < m->style_entries) { + if (text_pos == m->s[entry]->style_start) { + style_active = 1; + if (m->s[entry]->style_flag & STYLE_FLAG_BOLD) + av_bprintf(buf, "{\\b1}"); + if (m->s[entry]->style_flag & STYLE_FLAG_ITALIC) + av_bprintf(buf, "{\\i1}"); + if (m->s[entry]->style_flag & STYLE_FLAG_UNDERLINE) + av_bprintf(buf, "{\\u1}"); + av_bprintf(buf, "{\\fs%d}", m->s[entry]->fontsize); + for (i = 0; i < m->ftab_entries; i++) { + if (m->s[entry]->style_fontID == m->ftab[i]->fontID) + av_bprintf(buf, "{\\fn%s}", m->ftab[i]->font); } } - for (i = 0; i < m->style_entries; i++) { - if (m->s[i]->style_flag && text_pos == m->s[i]->style_start) { - if (m->s[i]->style_flag & STYLE_FLAG_BOLD) - av_bprintf(buf, "{\\b1}"); - if (m->s[i]->style_flag & STYLE_FLAG_ITALIC) - av_bprintf(buf, "{\\i1}"); - if (m->s[i]->style_flag & STYLE_FLAG_UNDERLINE) - av_bprintf(buf, "{\\u1}"); - av_bprintf(buf, "{\\fs%d}", m->s[i]->fontsize); - for (j = 0; j < m->ftab_entries; j++) { - if (m->s[i]->style_fontID == m->ftab[j]->fontID) - av_bprintf(buf, "{\\fn%s}", m->ftab[j]->font); - } + if (text_pos == m->s[entry]->style_end) { + if (style_active) { + av_bprintf(buf, "{\\r}"); + style_active = 0; } + entry++; } } if (m->box_flags & HLIT_BOX) {