Changes incorporated.
Updated patch attached.
Thanks,
Niklesh
On Thu, Aug 6, 2015 at 7:51 AM, Philip Langdale <phil...@overt.org> wrote:
> On Thu, 6 Aug 2015 07:13:07 +0530
> Niklesh Lalwani <lalwani1...@gmail.com> wrote:
>
> > On Thu, Aug 6, 2015 at 2:02 AM, Philip Langdale <phil...@overt.org>
> > wrote:
> >
> > > On Thu, 6 Aug 2015 00:12:07 +0530
> > > Niklesh Lalwani <lalwani1...@gmail.com> wrote:
> > >
> > > > From 90f466bb6a5d3cd24d7ea4d9fd8a2915cc68cdb2 Mon Sep 17 00:00:00
> > > > 2001 From: Niklesh <niklesh.lalw...@iitb.ac.in>
> > > > Date: Thu, 6 Aug 2015 00:06:15 +0530
> > > > Subject: [PATCH] movtextdec.c: Add support for font names
> > > >
> > > > Signed-off-by: Niklesh <niklesh.lalw...@iitb.ac.in>
> > > > ---
> > > > libavcodec/movtextdec.c | 109
> > > > ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed,
> > > > 106 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> > > > index 869358c..451056c 100644
> > > > --- a/libavcodec/movtextdec.c
> > > > +++ b/libavcodec/movtextdec.c
> > > > @@ -36,10 +36,17 @@
> > > > #define HCLR_BOX (1<<2)
> > > >
> > > > typedef struct {
> > > > + uint16_t fontID;
> > > > + uint8_t font_name_length;
> > > > + uint8_t font[20];
> > > > +} FontRecord;
> > > > +
> > > > +typedef struct {
> > > > uint16_t style_start;
> > > > uint16_t style_end;
> > > > uint8_t style_flag;
> > > > uint8_t fontsize;
> > > > + uint16_t style_fontID;
> > > > } StyleBox;
> > > >
> > > > typedef struct {
> > > > @@ -56,11 +63,13 @@ typedef struct {
> > > > StyleBox *s_temp;
> > > > HighlightBox h;
> > > > HilightcolorBox c;
> > > > + FontRecord **ftab;
> > > > + FontRecord *ftab_temp;
> > > > uint8_t box_flags;
> > > > - uint16_t style_entries;
> > > > + uint16_t style_entries, ftab_entries;
> > > > uint64_t tracksize;
> > > > int size_var;
> > > > - int count_s;
> > > > + int count_s, count_f;
> > > > } MovTextContext;
> > > >
> > > > typedef struct {
> > > > @@ -80,6 +89,80 @@ static void mov_text_cleanup(MovTextContext *m)
> > > > }
> > > > }
> > > >
> > > > +static void mov_text_cleanup_ftab(MovTextContext *m)
> > > > +{
> > > > + int i;
> > > > + for(i = 0; i < m->count_f; i++) {
> > > > + av_freep(&m->ftab[i]);
> > > > + }
> > > > + av_freep(&m->ftab);
> > > > +}
> > > > +
> > > > +static int mov_text_t3xg(AVCodecContext *avctx, MovTextContext
> > > > *m)
> > >
> > > I'm pretty sure it's "tx3g"
> > >
> > > > +{
> > > > + char *t3xg_ptr = avctx->extradata;
> > > > + int i, tracksize;
> > > > +
> > > > + tracksize = 38; /* Size till ftab_entries */
> > >
> > > I'd say box size. It's not technically a track.
> > >
> > > And make the 38 a constant.
> > >
> > > > + if (avctx->extradata_size < tracksize)
> > > > + return -1;
> > > > +
> > > > + m->ftab_entries = 0;
> > > > + // Display Flags
> > > > + t3xg_ptr += 4;
> > > > + // Alignment
> > > > + t3xg_ptr += 2;
> > > > + // Background Color
> > > > + t3xg_ptr += 4;
> > > > + // BoxRecord
> > > > + t3xg_ptr += 8;
> > > > + // StyleRecord
> > > > + t3xg_ptr += 12;
> > > > + // FontRecord
> > > > + // FontRecord Size
> > > > + t3xg_ptr += 4;
> > > > + // ftab
> > > > + t3xg_ptr += 4;
> > > > +
> > > > + tracksize += 2;
> > > > + if (avctx->extradata_size < tracksize)
> > > > + return -1;
> > >
> > > Why not just initialize tracksize to 40? The extra += 2 isn't
> > > optional.
> > >
> > > > +
> > > > + m->ftab_entries = AV_RB16(t3xg_ptr);
> > > > + t3xg_ptr += 2;
> > > > +
> > > > + for (i = 0; i < m->ftab_entries; i++) {
> > > > +
> > > > + tracksize += 3;
> > > > + if (avctx->extradata_size < tracksize) {
> > > > + m->ftab_entries = 0;
> > > > + return -1;
> > > > + }
> > >
> > > You need to clean up any ftab entries that have already been
> > > processed.
> > >
> > > > + m->ftab_temp = av_malloc(sizeof(*m->ftab_temp));
> > > > + if (!m->ftab_temp) {
> > > > + mov_text_cleanup_ftab(m);
> > > > + return AVERROR(ENOMEM);
> > > > + }
> > > > + m->ftab_temp->fontID = AV_RB16(t3xg_ptr);
> > > > + t3xg_ptr += 2;
> > > > + m->ftab_temp->font_name_length = *t3xg_ptr++;
> > > > +
> > > > + tracksize = tracksize + m->ftab_temp->font_name_length;
> > > > + if ((avctx->extradata_size < tracksize) ||
> > > > (m->ftab_temp->font_name_length > 20)) {
> > > > + m->ftab_entries = 0;
> > > > + return -1;
> > > > + }
> > > > + memcpy(m->ftab_temp->font, t3xg_ptr,
> > > > m->ftab_temp->font_name_length);
> > > > + av_dynarray_add(&m->ftab, &m->count_f, m->ftab_temp);
> > > > + if (!m->ftab) {
> > > > + mov_text_cleanup_ftab(m);
> > > > + return AVERROR(ENOMEM);
> > > > + }
> > >
> > > I didn't think about this before, but there's no need to store the
> > > font name and length in your FontRecord in this way. Just store it
> > > as a normal null terminated string. Then you don't need the size
> > > and do can do normal string manipulation on it later (like using
> > > av_bprintf with '%s').
> > >
> >
> > Yes, I can do that and add a null character at the end after memcopy,
> > but I thought I'll define the structs as close as possible to the
> > Records defined in 3GPP spec.
> > Also, I think I should not limit the length to 20. Ideally, it is a
> > uint8_t value. In that case, I'll need to malloc the size of font
> > length.
>
> Oh my. I thought the 20 character limit was in the spec. If the spec
> has no limit, then you can't have one either - you do need to malloc
> and append a null. Thanks.
>
> >
> > Rest all updated.
> >
> > Thanks,
> > Niklesh
> >
> >
> > > > + t3xg_ptr = t3xg_ptr + 3 + m->ftab[i]->font_name_length;
> > > > + }
> > >
> > >
>
>
>
>
> --phil
>
From d0301d7fa75ee68d3234edb3be41728507093c72 Mon Sep 17 00:00:00 2001
From: Niklesh <niklesh.lalw...@iitb.ac.in>
Date: Thu, 6 Aug 2015 09:11:12 +0530
Subject: [PATCH] movtextdec.c: Add support for font names
Signed-off-by: Niklesh <niklesh.lalw...@iitb.ac.in>
---
libavcodec/movtextdec.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 108 insertions(+), 3 deletions(-)
diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
index 869358c..15dd434 100644
--- a/libavcodec/movtextdec.c
+++ b/libavcodec/movtextdec.c
@@ -31,15 +31,23 @@
#define STYLE_FLAG_ITALIC (1<<1)
#define STYLE_FLAG_UNDERLINE (1<<2)
+#define BOX_SIZE_INITIAL 40
+
#define STYL_BOX (1<<0)
#define HLIT_BOX (1<<1)
#define HCLR_BOX (1<<2)
typedef struct {
+ uint16_t fontID;
+ char *font;
+} FontRecord;
+
+typedef struct {
uint16_t style_start;
uint16_t style_end;
uint8_t style_flag;
uint8_t fontsize;
+ uint16_t style_fontID;
} StyleBox;
typedef struct {
@@ -56,11 +64,13 @@ typedef struct {
StyleBox *s_temp;
HighlightBox h;
HilightcolorBox c;
+ FontRecord **ftab;
+ FontRecord *ftab_temp;
uint8_t box_flags;
- uint16_t style_entries;
+ uint16_t style_entries, ftab_entries;
uint64_t tracksize;
int size_var;
- int count_s;
+ int count_s, count_f;
} MovTextContext;
typedef struct {
@@ -80,6 +90,85 @@ static void mov_text_cleanup(MovTextContext *m)
}
}
+static void mov_text_cleanup_ftab(MovTextContext *m)
+{
+ int i;
+ for(i = 0; i < m->count_f; i++) {
+ av_freep(&m->ftab[i]->font);
+ av_freep(&m->ftab[i]);
+ }
+ av_freep(&m->ftab);
+}
+
+static int mov_text_tx3g(AVCodecContext *avctx, MovTextContext *m)
+{
+ char *tx3g_ptr = avctx->extradata;
+ int i, box_size, font_length;
+
+ m->ftab_entries = 0;
+ box_size = BOX_SIZE_INITIAL; /* Size till ftab_entries */
+ if (avctx->extradata_size < box_size)
+ return -1;
+
+ // Display Flags
+ tx3g_ptr += 4;
+ // Alignment
+ tx3g_ptr += 2;
+ // Background Color
+ tx3g_ptr += 4;
+ // BoxRecord
+ tx3g_ptr += 8;
+ // StyleRecord
+ tx3g_ptr += 12;
+ // FontRecord
+ // FontRecord Size
+ tx3g_ptr += 4;
+ // ftab
+ tx3g_ptr += 4;
+
+ m->ftab_entries = AV_RB16(tx3g_ptr);
+ tx3g_ptr += 2;
+
+ for (i = 0; i < m->ftab_entries; i++) {
+
+ box_size += 3;
+ if (avctx->extradata_size < box_size) {
+ mov_text_cleanup_ftab(m);
+ m->ftab_entries = 0;
+ return -1;
+ }
+ m->ftab_temp = av_malloc(sizeof(*m->ftab_temp));
+ if (!m->ftab_temp) {
+ mov_text_cleanup_ftab(m);
+ return AVERROR(ENOMEM);
+ }
+ m->ftab_temp->fontID = AV_RB16(tx3g_ptr);
+ tx3g_ptr += 2;
+ font_length = *tx3g_ptr++;
+
+ box_size = box_size + font_length;
+ if (avctx->extradata_size < box_size) {
+ mov_text_cleanup_ftab(m);
+ m->ftab_entries = 0;
+ return -1;
+ }
+ m->ftab_temp->font = av_malloc(font_length + 1);
+ if (!m->ftab_temp->font) {
+ mov_text_cleanup_ftab(m);
+ return AVERROR(ENOMEM);
+ }
+ memcpy(m->ftab_temp->font, tx3g_ptr, font_length);
+ m->ftab_temp->font[font_length] = '\0';
+ av_dynarray_add(&m->ftab, &m->count_f, m->ftab_temp);
+ if (!m->ftab) {
+ mov_text_cleanup_ftab(m);
+ return AVERROR(ENOMEM);
+ }
+ tx3g_ptr = tx3g_ptr + font_length;
+ }
+ return 0;
+}
+
static int decode_hlit(const uint8_t *tsmb, MovTextContext *m, AVPacket *avpkt)
{
m->box_flags |= HLIT_BOX;
@@ -118,7 +207,7 @@ static int decode_styl(const uint8_t *tsmb, MovTextContext *m, AVPacket *avpkt)
tsmb += 2;
m->s_temp->style_end = AV_RB16(tsmb);
tsmb += 2;
- // fontID = AV_RB16(tsmb);
+ m->s_temp->style_fontID = AV_RB16(tsmb);
tsmb += 2;
m->s_temp->style_flag = AV_RB8(tsmb);
tsmb++;
@@ -147,6 +236,7 @@ static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end,
MovTextContext *m)
{
int i = 0;
+ int j = 0;
int text_pos = 0;
while (text < text_end) {
if (m->box_flags & STYL_BOX) {
@@ -164,6 +254,10 @@ static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end,
if (m->s[i]->style_flag & STYLE_FLAG_UNDERLINE)
av_bprintf(buf, "{\\u1}");
av_bprintf(buf, "{\\fs%d}", m->s[i]->fontsize);
+ for (j = 0; j < m->ftab_entries; j++) {
+ if (m->s[i]->style_fontID == m->ftab[j]->fontID)
+ av_bprintf(buf, "{\\fn%s}", m->ftab[j]->font);
+ }
}
}
}
@@ -215,6 +309,8 @@ static int mov_text_init(AVCodecContext *avctx) {
* it's very common to find files where the default style is broken
* and respecting it results in a worse experience than ignoring it.
*/
+ MovTextContext *m = avctx->priv_data;
+ mov_text_tx3g(avctx, m);
return ff_ass_subtitle_header_default(avctx);
}
@@ -265,6 +361,7 @@ static int mov_text_decode_frame(AVCodecContext *avctx,
m->style_entries = 0;
m->box_flags = 0;
m->count_s = 0;
+ m->count_f = 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) {
@@ -313,6 +410,13 @@ static int mov_text_decode_frame(AVCodecContext *avctx,
return avpkt->size;
}
+static int mov_text_decode_close(AVCodecContext *avctx)
+{
+ MovTextContext *m = avctx->priv_data;
+ mov_text_cleanup_ftab(m);
+ return 0;
+}
+
AVCodec ff_movtext_decoder = {
.name = "mov_text",
.long_name = NULL_IF_CONFIG_SMALL("3GPP Timed Text subtitle"),
@@ -321,4 +425,5 @@ AVCodec ff_movtext_decoder = {
.priv_data_size = sizeof(MovTextContext),
.init = mov_text_init,
.decode = mov_text_decode_frame,
+ .close = mov_text_decode_close,
};
--
1.9.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
