[FFmpeg-devel] [PATCH 2/2] Using dynamic arrays for multiple style records

[Philip Langdale]

On 2015-04-13 07:57, Niklesh Lalwani wrote:
> From: Niklesh <niklesh.lalwani at iitb.ac.in>
> 
> This patch attempts to use dynamic arrays to support multiple style
> records. However, I am unable to get proper output with using
> av_dynamic_array(). It seems I am not using this function properly.
> Can anyone explain?
> Signed-off-by: Niklesh <niklesh.lalwani at iitb.ac.in>
> ---
>  libavcodec/movtextdec.c | 65 
> +++++++++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index 4e463ed..2220cf9 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -25,24 +25,28 @@
>  #include "libavutil/common.h"
>  #include "libavutil/bprint.h"
>  #include "libavutil/intreadwrite.h"
> + #include "libavutil/mem.h"
> 
>  #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 char **style_start, const char 
> **style_end,
> +                        const int **style_flags, const int 
> style_entries)
>  {
>          while (text < text_end) {
> -        if (style_flags && text == style_start)
> +        for (int i=0; i<style_entries; i++)
>          {
> -            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}");
> +            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) {
> @@ -56,14 +60,17 @@ static int text_to_ass(AVBPrint *buf, const char
> *text, const char *text_end,
>              break;
>          }
> 
> -        if (style_flags && text == style_end)
> +        for (int i=0; i<style_entries; i++)
>          {
> -            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}");
> +            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++;
>      }
> @@ -89,9 +96,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, style_flags, tsmb_size;
> -    const char *style_start, *style_end;
> +    int text_length, tsmb_type, style_entries, tsmb_size;
> +    char **style_start={0,};
> +    char **style_end={0,};
> +    int **style_flags={0,};

1) You need to free these at the end, otherwise you're leaking them.

2) The style_flags is an integer array, not a pointer-to-integer array.

3) I think it would be clearer to store style_start and style_end as 
indices, rather
than computing pointers. You already pass the start/end of the string to 
text_to_ass,
so just pass the indicies as well.

>      const uint8_t *tsmb;
> +    int index, flag=0;;
> +    char *ptr_temp;
> 
>      if (!ptr || avpkt->size < 2)
>          return AVERROR_INVALIDDATA;
> @@ -143,26 +154,32 @@ static int mov_text_decode_frame(AVCodecContext 
> *avctx,
>              tsmb += 2;
> 
>              for(int i = 0; i < style_entries;i++)
> -            {
> -                style_start = ptr + AV_RB16(tsmb);
> +            {
> +                ptr_temp= ptr + AV_RB16(tsmb);
> +                index=i;
> +                av_dynarray_add(&style_start, &index, ptr_temp);
>                  tsmb += 2;
> -                style_end = ptr + AV_RB16(tsmb);
> +                ptr_temp= ptr+ AV_RB16(tsmb);
> +                index=i;
> +                av_dynarray_add(&style_end, &index, ptr_temp);
>                  tsmb += 2;
>                  // fontID = AV_RB16(tsmb);
>                  tsmb += 2;
> -                style_flags = AV_RB8(tsmb);
> +                flag=AV_RB16(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);
>              }
> +            text_to_ass(&buf, ptr, end, style_start, style_end,
> style_flags,style_entries);
>          }
> 
>      }
>      }
>      else
> -        text_to_ass(&buf, ptr, end, NULL, NULL, 0);
> +        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);

-- 
--phil