[FFmpeg-devel] [PATCH] movtextdec.c: Add support for highlight and hilightcolor box

Philip Langdale philipl at overt.org
Sun Jul 5 18:32:43 CEST 2015


On Sun,  5 Jul 2015 15:53:32 +0530
Niklesh Lalwani <niklesh.lalwani at iitb.ac.in> wrote:

> From: Niklesh <niklesh.lalwani at iitb.ac.in>
> 
> This patch adds support for decoding of Highlight and hilightcolor
> box as defined by 3GPP standards. The code is also reorganised to
> make it easier to maintain and read. Separate functions are defined
> that process each type of box. Signed-off-by: Niklesh

Looking good. Just a couple of comments below - and make sure your
commit description doesn't go past 72 columns.

> <niklesh.lalwani at iitb.ac.in> --- libavcodec/movtextdec.c | 253
> ++++++++++++++++++++++++++++++++---------------- 1 file changed, 172
> insertions(+), 81 deletions(-)
> 
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index a3afd91..0dab0b4 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -37,31 +37,157 @@ typedef struct {
>      uint8_t style_flag;
>  } StyleBox;
>  
> +typedef struct {
> +    uint16_t hlit_start;
> +    uint16_t hlit_end;
> +} HighlightBox;
> +
> +typedef struct {
> +   uint8_t hlit_color[4];
> +} HilightcolorBox;
> +
> +typedef struct {
> +    int styl;
> +    int hlit;
> +    int hclr;
> +} Box_flags;

You can use a single uint8_t here and bit flags, like you did for
the STYLE_FLAGs.

> +
> +typedef struct {
> +    StyleBox **s;
> +    StyleBox *s_temp;
> +    HighlightBox h;
> +    HilightcolorBox c;
> +    Box_flags f;
> +    uint16_t style_entries;
> +    uint64_t tracksize;
> +    int size_var;
> +    int count_s;
> +} MovTextContext;
> +
> +struct Box
> +{
> +    uint32_t type;
> +    size_t base_size;
> +    int (*decode)(const uint8_t *tsmb, MovTextContext *m, AVPacket
> *avpkt); +};
> +
> +static void mov_text_cleanup(MovTextContext *m)
> +{
> +    int i;
> +    if (m->f.styl) {
> +        for(i = 0; i < m->count_s; i++) {
> +            av_freep(&m->s[i]);
> +        }
> +        av_freep(&m->s);
> +    }
> +}
> +
> +static int decode_hlit(const uint8_t *tsmb, MovTextContext *m,
> AVPacket *avpkt) +{
> +    m->f.hlit = 1;
> +    m->h.hlit_start = AV_RB16(tsmb);
> +    tsmb += 2;
> +    m->h.hlit_end = AV_RB16(tsmb);
> +    tsmb += 2;
> +    return 0;
> +}
> +
> +static int decode_hclr(const uint8_t *tsmb, MovTextContext *m,
> AVPacket *avpkt) +{
> +    m->f.hclr = 1;
> +    m->c.hlit_color[0] = *tsmb++;
> +    m->c.hlit_color[1] = *tsmb++;
> +    m->c.hlit_color[2] = *tsmb++;
> +    m->c.hlit_color[3] = *tsmb++;
> +    return 0;
> +}
> +
> +static int decode_styl(const uint8_t *tsmb, MovTextContext *m,
> AVPacket *avpkt) +{
> +    int i;
> +    m->style_entries = AV_RB16(tsmb);
> +    tsmb += 2;
> +    // A single style record is of length 12 bytes.
> +    if (m->tracksize + m->size_var + 2 + m->style_entries * 12 >
> avpkt->size)
> +        return -1;
> +
> +    m->f.styl = 1;
> +    for(i = 0; i < m->style_entries; i++) {
> +        m->s_temp = av_malloc(sizeof(StyleBox));
> +        if (!m->s_temp) {
> +            mov_text_cleanup(m);
> +            return AVERROR(ENOMEM);
> +        }
> +        m->s_temp->style_start = AV_RB16(tsmb);
> +        tsmb += 2;
> +        m->s_temp->style_end = AV_RB16(tsmb);
> +        tsmb += 2;
> +        // fontID = AV_RB16(tsmb);
> +        tsmb += 2;
> +        m->s_temp->style_flag = AV_RB8(tsmb);
> +        av_dynarray_add(&m->s, &m->count_s, m->s_temp);
> +        if(!m->s) {
> +            mov_text_cleanup(m);
> +            return AVERROR(ENOMEM);
> +        }
> +        // fontsize = AV_RB8(tsmb);
> +        tsmb += 2;
> +        // text-color-rgba
> +        tsmb += 4;
> +    }
> +    return 0;
> +}
> +
> +struct Box box_types[] = {
> +    { MKBETAG('s','t','y','l'), 2, decode_styl },
> +    { MKBETAG('h','l','i','t'), 4, decode_hlit },
> +    { MKBETAG('h','c','l','r'), 4, decode_hclr }
> +};
> +
> +const static size_t box_count = sizeof(box_types) / sizeof(struct
> Box); +
>  static int text_to_ass(AVBPrint *buf, const char *text, const char
> *text_end,
> -                        StyleBox **s, int style_entries)
> +                        MovTextContext *m)
>  {
>      int i = 0;
>      int text_pos = 0;
>      while (text < text_end) {
> -        for (i = 0; i < style_entries; i++) {
> -            if (s[i]->style_flag && text_pos == s[i]->style_end) {
> -                if (s[i]->style_flag & STYLE_FLAG_BOLD)
> -                    av_bprintf(buf, "{\\b0}");
> -                if (s[i]->style_flag & STYLE_FLAG_ITALIC)
> -                    av_bprintf(buf, "{\\i0}");
> -                if (s[i]->style_flag & STYLE_FLAG_UNDERLINE)
> -                    av_bprintf(buf, "{\\u0}");
> +        if (m->f.styl) {
> +            for (i = 0; i < m->style_entries; i++) {
> +                if (m->s[i]->style_flag && text_pos ==
> m->s[i]->style_end) {
> +                    if (m->s[i]->style_flag & STYLE_FLAG_BOLD)
> +                        av_bprintf(buf, "{\\b0}");
> +                    if (m->s[i]->style_flag & STYLE_FLAG_ITALIC)
> +                        av_bprintf(buf, "{\\i0}");
> +                    if (m->s[i]->style_flag & STYLE_FLAG_UNDERLINE)
> +                        av_bprintf(buf, "{\\u0}");
> +                }
> +            }
> +            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}");
> +                }
>              }
>          }
> -
> -        for (i = 0; i < style_entries; i++) {
> -            if (s[i]->style_flag && text_pos == s[i]->style_start) {
> -                if (s[i]->style_flag & STYLE_FLAG_BOLD)
> -                    av_bprintf(buf, "{\\b1}");
> -                if (s[i]->style_flag & STYLE_FLAG_ITALIC)
> -                    av_bprintf(buf, "{\\i1}");
> -                if (s[i]->style_flag & STYLE_FLAG_UNDERLINE)
> -                    av_bprintf(buf, "{\\u1}");
> +        if (m->f.hlit) {
> +            if (text_pos == m->h.hlit_start) {
> +                if (m->f.hclr) {
> +                    av_bprintf(buf, "{\\2c&H%02x%02x%02x&}",
> m->c.hlit_color[2], m->c.hlit_color[1], m->c.hlit_color[0]);
> +                } else {
> +                        av_bprintf(buf,
> "{\\1c&H000000&}{\\2c&HFFFFFF}");
> +                }
> +            }
> +            if (text_pos == m->h.hlit_end) {
> +                if (m->f.hclr) {
> +                    av_bprintf(buf, "{\\2c&H000000}");
> +                } else {
> +                    av_bprintf(buf, "{\\1c&HFFFFFF&}{\\2c&H000000}");
> +                }

Add some comments about the logic when there is no hclr specified.

>              }
>          }
>  
> @@ -78,7 +204,6 @@ static int text_to_ass(AVBPrint *buf, const char
> *text, const char *text_end, text++;
>          text_pos++;
>      }
> -
>      return 0;
>  }
>  
> @@ -96,17 +221,14 @@ static int mov_text_decode_frame(AVCodecContext
> *avctx, void *data, int *got_sub_ptr, AVPacket *avpkt)
>  {
>      AVSubtitle *sub = data;
> +    MovTextContext *m = avctx->priv_data;
>      int ret, ts_start, ts_end;
>      AVBPrint buf;
>      char *ptr = avpkt->data;
>      char *end;
> -    //char *ptr_temp;
> -    int text_length, tsmb_type, style_entries;
> -    uint64_t tsmb_size, tracksize;
> -    StyleBox **s = {0, };
> -    StyleBox *s_temp;
> +    int text_length, tsmb_type, ret_tsmb;
> +    uint64_t tsmb_size;
>      const uint8_t *tsmb;
> -    int count, i, size_var;
>  
>      if (!ptr || avpkt->size < 2)
>          return AVERROR_INVALIDDATA;
> @@ -138,73 +260,51 @@ static int mov_text_decode_frame(AVCodecContext
> *avctx, (AVRational){1,100});
>  
>      tsmb_size = 0;
> -    tracksize = 2 + text_length;
> +    m->tracksize = 2 + text_length;
> +    m->style_entries = 0;
> +    m->f.styl = 0;
> +    m->f.hlit = 0;
> +    m->f.hclr = 0;
> +    m->count_s = 0;
>      // Note that the spec recommends lines be no longer than 2048
> characters. av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
>      if (text_length + 2 != avpkt->size) {
> -        while (tracksize + 8 <= avpkt->size) {
> +        while (m->tracksize + 8 <= avpkt->size) {
>              // A box is a minimum of 8 bytes.
> -            tsmb = ptr + tracksize - 2;
> +            tsmb = ptr + m->tracksize - 2;
>              tsmb_size = AV_RB32(tsmb);
>              tsmb += 4;
>              tsmb_type = AV_RB32(tsmb);
>              tsmb += 4;
>  
>              if (tsmb_size == 1) {
> -                if (tracksize + 16 > avpkt->size)
> +                if (m->tracksize + 16 > avpkt->size)
>                      break;
>                  tsmb_size = AV_RB64(tsmb);
>                  tsmb += 8;
> -                size_var = 18;
> +                m->size_var = 16;
>              } else
> -                size_var = 10;
> -            //size_var is equal to 10 or 18 depending on the size of
> box
> +                m->size_var = 8;
> +            //size_var is equal to 8 or 16 depending on the size of
> box 
> -            if (tracksize + tsmb_size > avpkt->size)
> +            if (m->tracksize + tsmb_size > avpkt->size)
>                  break;
>  
> -            if (tsmb_type == MKBETAG('s','t','y','l')) {
> -                if (tracksize + size_var > avpkt->size)
> -                    break;
> -                style_entries = AV_RB16(tsmb);
> -                tsmb += 2;
> -
> -                // A single style record is of length 12 bytes.
> -                if (tracksize + size_var + style_entries * 12 >
> avpkt->size)
> -                    break;
> -                count = 0;
> -
> -                for(i = 0; i < style_entries; i++) {
> -                    s_temp = av_malloc(sizeof(*s_temp));
> -                    if (!s_temp)
> -                        goto error;
> -
> -                    s_temp->style_start = AV_RB16(tsmb);
> -                    tsmb += 2;
> -                    s_temp->style_end = AV_RB16(tsmb);
> -                    tsmb += 2;
> -                    // fontID = AV_RB16(tsmb);
> -                    tsmb += 2;
> -                    s_temp->style_flag = AV_RB8(tsmb);
> -                    av_dynarray_add(&s, &count, s_temp);
> -                    if(!s)
> -                        goto error;
> -                    //fontsize=AV_RB8(tsmb);
> -                    tsmb += 2;
> -                    // text-color-rgba
> -                    tsmb += 4;
> -                }
> -                text_to_ass(&buf, ptr, end, s, style_entries);
> -
> -                for(i = 0; i < count; i++) {
> -                    av_freep(&s[i]);
> +            for (size_t i = 0; i < box_count; i++) {
> +                if (tsmb_type == box_types[i].type) {
> +                    if (m->tracksize + m->size_var +
> box_types[i].base_size > avpkt->size)
> +                        break;
> +                    ret_tsmb = box_types[i].decode(tsmb, m, avpkt);
> +                    if (ret_tsmb == -1)
> +                        break;
>                  }
> -                av_freep(&s);
>              }
> -            tracksize = tracksize + tsmb_size;
> +            m->tracksize = m->tracksize + tsmb_size;
>          }
> +        text_to_ass(&buf, ptr, end, m);
> +        mov_text_cleanup(m);
>      } else
> -        text_to_ass(&buf, ptr, end, NULL, 0);
> +        text_to_ass(&buf, ptr, end, m);
>  
>      ret = ff_ass_add_rect_bprint(sub, &buf, ts_start, ts_end -
> ts_start); av_bprint_finalize(&buf, NULL);
> @@ -212,16 +312,6 @@ static int mov_text_decode_frame(AVCodecContext
> *avctx, return ret;
>      *got_sub_ptr = sub->num_rects > 0;
>      return avpkt->size;
> -
> -error:
> -    for(i = 0; i < count; i++) {
> -        av_freep(&s[i]);
> -    }
> -    av_freep(&s);
> -    if (s_temp)
> -        av_freep(&s_temp);
> -    av_bprint_finalize(&buf, NULL);
> -    return AVERROR(ENOMEM);
>  }
>  
>  AVCodec ff_movtext_decoder = {
> @@ -229,6 +319,7 @@ AVCodec ff_movtext_decoder = {
>      .long_name    = NULL_IF_CONFIG_SMALL("3GPP Timed Text subtitle"),
>      .type         = AVMEDIA_TYPE_SUBTITLE,
>      .id           = AV_CODEC_ID_MOV_TEXT,
> +    .priv_data_size = sizeof(MovTextContext),
>      .init         = mov_text_init,
>      .decode       = mov_text_decode_frame,
>  };




--phil


More information about the ffmpeg-devel mailing list