On Sun, 5 Jul 2015 15:53:32 +0530
Niklesh Lalwani <niklesh.lalw...@iitb.ac.in> wrote:
> From: Niklesh <niklesh.lalw...@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.lalw...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
