[FFmpeg-devel] [PATCH 1/2] Decoding of Bold Italic and Underlined styles for 3GPP timed text subtitles

[Philip Langdale]

On 2015-04-13 07:56, Niklesh Lalwani wrote:
> From: Niklesh <niklesh.lalwani at iitb.ac.in>
> 
> This patch is a part of my qualification task to implement support for
> Bold, Italic, and Underlined style records for 3GPP timed text
> subtitles. I am continuing Wesley's work. This patch supports decoding
> of no more than one style record. Patch[2/2] attempts to use dynamic
> arrays to support multiple style records.
> Signed-off-by: Niklesh <niklesh.lalwani at iitb.ac.in>
> ---
>  libavcodec/movtextdec.c | 81 
> +++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 75 insertions(+), 6 deletions(-)

Thank you for posting this.

> 
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index 1c7ffea..4e463ed 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -26,9 +26,25 @@
>  #include "libavutil/bprint.h"
>  #include "libavutil/intreadwrite.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)
>  {
> -    while (text < text_end) {
> +        while (text < text_end) {
> +        if (style_flags && text == style_start)
> +        {
> +            if (style_flags & STYLE_FLAG_BOLD)
> +                av_bprintf(buf, "{\\b1}");
> +            if (style_flags & STYLE_FLAG_ITALIC)
> +                av_bprintf(buf, "{\\i1}");
> +            if (style_flags & STYLE_FLAG_UNDERLINE)
> +                av_bprintf(buf, "{\\u1}");
> +        }
> +
>          switch (*text) {
>          case '\r':
>              break;
> @@ -39,6 +55,16 @@ static int text_to_ass(AVBPrint *buf, const char
> *text, const char *text_end)
>              av_bprint_chars(buf, *text, 1);
>              break;
>          }
> +
> +        if (style_flags && text == style_end)
> +        {
> +            if (style_flags & STYLE_FLAG_BOLD)
> +                av_bprintf(buf, "{\\b0}");
> +            if (style_flags & STYLE_FLAG_ITALIC)
> +                av_bprintf(buf, "{\\i0}");
> +            if (style_flags & STYLE_FLAG_UNDERLINE)
> +                av_bprintf(buf, "{\\u0}");
> +        }
>          text++;
>      }
> 
> @@ -63,6 +89,9 @@ 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, style_flags, tsmb_size;
> +    const char *style_start, *style_end;
> +    const uint8_t *tsmb;
> 
>      if (!ptr || avpkt->size < 2)
>          return AVERROR_INVALIDDATA;
> @@ -74,6 +103,7 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>       * already. If the value is non-zero, then it's technically a
>       * bad packet.
>       */
> +
>      if (avpkt->size == 2)
>          return AV_RB16(ptr) == 0 ? 0 : AVERROR_INVALIDDATA;
> 
> @@ -82,7 +112,10 @@ 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);
> +    //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,
> @@ -91,11 +124,47 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>      ts_end   = av_rescale_q(avpkt->pts + avpkt->duration,
>                              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;

Remember that you need to account for large (> 32bit) boxes here. I 
guess it's
almost impossible to get a large box, but it's better to be correct than 
to blow
up in the future.

> +
> +        if (tsmb_type == MKBETAG('s','t','y','l'))
> +        {
> +            style_entries = AV_RB16(tsmb);
> +            tsmb += 2;
> +
> +            for(int i = 0; i < style_entries;i++)
> +            {
> +                style_start = ptr + AV_RB16(tsmb);
> +                tsmb += 2;
> +                style_end = ptr + AV_RB16(tsmb);
> +                tsmb += 2;
> +                // fontID = AV_RB16(tsmb);
> +                tsmb += 2;
> +                style_flags = AV_RB8(tsmb);
> +                //fontsize=AV_RB8(tsmb);
> +                //tsmb += 2;
> +                // text-color-rgba
> +                //tsmb += 4;
> +                text_to_ass(&buf, ptr, end, style_start, style_end,
> style_flags);
> +            }

As we will be seeing TextModifierStyleBoxes used in multiple places, and 
because it's
easier to read, please define and use a struct for it, so you don't have 
to do all these
individual reads to access the information. Remember to use a packed 
struct or the layout
will not match.

> +        }
> +
> +    }
> +    }
> +    else
> +        text_to_ass(&buf, ptr, end, NULL, NULL, 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