[FFmpeg-devel] [PATCH] Decoding of Bold-Italic-Underlined styles for 3gpp timed text

[Philip Langdale]

On 2015-04-22 03:10, Niklesh Lalwani wrote:
> From: Niklesh <niklesh.lalwani at iitb.ac.in>
> 
> This patch supports decoding of Bold, Italic, Underlined styles for
> 3gpp timed text. While the code can be improved upon to make it more
> clean and well structured, this works for now, even for multiple style
> records. Suggestions awaited.
> Signed-off-by: Niklesh <niklesh.lalwani at iitb.ac.in>
> ---
>  libavcodec/movtextdec.c | 86 
> ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 82 insertions(+), 4 deletions(-)

Thank you Niklesh.

I will repeat my three primary items from the previous review as they 
all still
apply:

* You need to handle large (> 32bit) boxes.
* You should store indices in style_start and style_end
* You should define a struct TextSampleModifierBox and use that for 
de-serialization.

I'm personally ok with you doing these as follow-up items, and not 
having them in to
initial committed patch, but the others need to agree with that.

Please address the syntax and warning issues that have already been 
pointed out.

> 
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index 1c7ffea..a4aa7cb 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -25,10 +25,28 @@
>  #include "libavutil/common.h"
>  #include "libavutil/bprint.h"
>  #include "libavutil/intreadwrite.h"
> +#include "libavutil/mem.h"
> 
> -static int text_to_ass(AVBPrint *buf, const char *text, const char 
> *text_end)
> +#define STYLE_FLAG_BOLD         1
> +#define STYLE_FLAG_ITALIC       2
> +#define STYLE_FLAG_UNDERLINE    4
> +
> +static int text_to_ass(AVBPrint *buf, const char *text, const char 
> *text_end,
> +                        const char **style_start, const char 
> **style_end,
> +                        const int **style_flags, const int 
> style_entries)
>  {
>      while (text < text_end) {
> +        for (int i=0; i<style_entries; i++) {
> +            if (*style_flags[i] && text == style_start[i]) {
> +                if (*style_flags[i] & STYLE_FLAG_BOLD)
> +                    av_bprintf(buf, "{\\b1}");
> +                if (*style_flags[i] & STYLE_FLAG_ITALIC)
> +                    av_bprintf(buf, "{\\i1}");
> +                if (*style_flags[i] & STYLE_FLAG_UNDERLINE)
> +                    av_bprintf(buf, "{\\u1}");
> +            }
> +        }
> +
>          switch (*text) {
>          case '\r':
>              break;
> @@ -39,6 +57,17 @@ static int text_to_ass(AVBPrint *buf, const char
> *text, const char *text_end)
>              av_bprint_chars(buf, *text, 1);
>              break;
>          }
> +
> +        for (int i=0; i<style_entries; i++) {
> +            if (*style_flags[i] && text == style_end[i]) {
> +                if (*style_flags[i] & STYLE_FLAG_BOLD)
> +                    av_bprintf(buf, "{\\b0}");
> +                if (*style_flags[i] & STYLE_FLAG_ITALIC)
> +                    av_bprintf(buf, "{\\i0}");
> +                if (*style_flags[i] & STYLE_FLAG_UNDERLINE)
> +                    av_bprintf(buf, "{\\u0}");
> +            }
> +        }
>          text++;
>      }
> 
> @@ -63,6 +92,13 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>      AVBPrint buf;
>      const char *ptr = avpkt->data;
>      const char *end;
> +    int text_length, tsmb_type, style_entries, tsmb_size;
> +    char **style_start={0,};
> +    char **style_end={0,};
> +    int **style_flags={0,};
> +    const uint8_t *tsmb;
> +    int index,i, flag=0;;
> +    char *ptr_temp;
> 
>      if (!ptr || avpkt->size < 2)
>          return AVERROR_INVALIDDATA;
> @@ -82,7 +118,8 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>       * In complex cases, there are style descriptors appended to the 
> string
>       * so we can't just assume the packet size is the string size.
>       */
> -    end = ptr + FFMIN(2 + AV_RB16(ptr), avpkt->size);
> +    text_length = AV_RB16(ptr);
> +    end = ptr + FFMIN(2 + text_length, avpkt->size);
>      ptr += 2;
> 
>      ts_start = av_rescale_q(avpkt->pts,
> @@ -92,10 +129,51 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>                              avctx->time_base,
>                              (AVRational){1,100});
> 
> +    tsmb_size=0;
>      // Note that the spec recommends lines be no longer than 2048 
> characters.
>      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> -    text_to_ass(&buf, ptr, end);
> -    ret = ff_ass_add_rect_bprint(sub, &buf, ts_start, 
> ts_end-ts_start);
> +    if (text_length + 2 != avpkt->size) {
> +        while (text_length + 2 + tsmb_size < avpkt->size)  {
> +            tsmb = ptr + text_length+tsmb_size;
> +            tsmb_size = AV_RB32(tsmb);
> +            tsmb += 4;
> +            tsmb_type = AV_RB32(tsmb);
> +            tsmb += 4;
> +
> +            if (tsmb_type == MKBETAG('s','t','y','l')) {
> +                style_entries = AV_RB16(tsmb);
> +                tsmb += 2;
> +
> +                for(i = 0; i < style_entries;i++) {
> +                    ptr_temp= ptr + AV_RB16(tsmb);
> +                    index=i;
> +                    av_dynarray_add(&style_start, &index, ptr_temp);
> +                    tsmb += 2;
> +                    ptr_temp= ptr+ AV_RB16(tsmb);
> +                    index=i;
> +                    av_dynarray_add(&style_end, &index, ptr_temp);
> +                    tsmb += 2;
> +                    // fontID = AV_RB16(tsmb);
> +                    tsmb += 2;
> +                    flag=AV_RB8(tsmb);
> +                    index=i;
> +                    av_dynarray_add(&style_flags, &index, &flag);
> +                    //fontsize=AV_RB8(tsmb);
> +                    tsmb += 2;
> +                    // text-color-rgba
> +                    tsmb += 4;
> +                }
> +                text_to_ass(&buf, ptr, end, style_start, style_end,
> style_flags,style_entries);
> +                av_freep(&style_start);
> +                av_freep(&style_end);
> +                av_freep(&style_flags);
> +            }
> +        }
> +    }
> +    else
> +        text_to_ass(&buf, ptr, end, NULL, NULL, 0, 0);
> +
> +    ret = ff_ass_add_rect_bprint(sub, &buf, ts_start, ts_end - 
> ts_start);
>      av_bprint_finalize(&buf, NULL);
>      if (ret < 0)
>          return ret;

--phil