[FFmpeg-devel] [PATCH 02/23] lavc/movtextdec: simplify style record walk

[John Stebbins]

On Mon, 2020-04-06 at 14:46 -0700, Philip Langdale wrote:
> On Mon, 6 Apr 2020 11:51:57 -0600
> John Stebbins <jstebbins at 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."