Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, 26 Apr 2015 14:32:31 +0530 Niklesh Lalwani wrote: > On 26-Apr-2015 10:46 AM, "Philip Langdale" wrote: > > No problem. I had never used packed structs before so it was good to > learn about them. So should I just post the patch for implementing > support for large boxes for now? Yes please. And, as we discussed, change the dynarrays from int** to int*. --phil ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On 26-Apr-2015 4:00 PM, "wm4" wrote: > > > We usually write: text_style[i].face_style_flags > > It's the same. Got it. Thank you. :) -Niklesh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, 26 Apr 2015 08:33:34 +0530 Niklesh Lalwani wrote: > > > > > > > > On Sun, Apr 26, 2015 at 7:05 AM, Carl Eugen Hoyos > > wrote: > > > > Niklesh Lalwani iitb.ac.in> writes: > > > > > >> > +if ((text_style + i)->face_style_flags > > > > > >> This looks very strange imo... > > > > > What's wrong? I point my structure to the start of the style records, so, > (text_style + i)->face_style_flags will give me the value of the attribute > face_style_flags of (i+1)th style record. We usually write: text_style[i].face_style_flags It's the same. > > >> > +long tsmb_size; > > > > > >> int64_t ? > > > > > >> > My bad. Yes, I intended to use int64_t / double. I will correct it. > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On 26-Apr-2015 10:46 AM, "Philip Langdale" wrote:
>
> On Sun, 26 Apr 2015 05:04:07 +0200
> Michael Niedermayer wrote:
>
> > On Sun, Apr 26, 2015 at 08:22:56AM +0530, Niklesh Lalwani wrote:
> > > On Sun, Apr 26, 2015 at 6:56 AM, Michael Niedermayer
> > >
> > > > wrote:
> > > >
> > > >>
> > > >> > +struct __attribute__((__packed__)) StyleRecord {
> > > >
> > > >
> > > >> you cant do this
> > > >
> > > > attribute packed is not ANSI/ISO C
> > > >
> > > >
> > > >> How do I implement packed structures then here? ( since I don't
> > > >> want
> > > padding for proper alignment with the text sample)
> >
> > you cant create packed structs, theres no portable way to do so
> > also there likely would be issues on either big or little endian
> > with such structs if they where used for reading data
>
> You're right. This was my suggestion. Niklesh - you can still use the
> struct, but you'd have to read the members of the struct out one by
> one. It's not clear whether that provides any meaningful benefit. You
> still end up losing the benefit of being able to directly address the
> styles. I think we have to abandon this approach. Sorry.
>
> --phil
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
No problem. I had never used packed structs before so it was good to learn
about them. So should I just post the patch for implementing support for
large boxes for now?
-Niklesh
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, 26 Apr 2015 05:04:07 +0200
Michael Niedermayer wrote:
> On Sun, Apr 26, 2015 at 08:22:56AM +0530, Niklesh Lalwani wrote:
> > On Sun, Apr 26, 2015 at 6:56 AM, Michael Niedermayer
> >
> > > wrote:
> > >
> > >>
> > >> > +struct __attribute__((__packed__)) StyleRecord {
> > >
> > >
> > >> you cant do this
> > >
> > > attribute packed is not ANSI/ISO C
> > >
> > >
> > >> How do I implement packed structures then here? ( since I don't
> > >> want
> > padding for proper alignment with the text sample)
>
> you cant create packed structs, theres no portable way to do so
> also there likely would be issues on either big or little endian
> with such structs if they where used for reading data
You're right. This was my suggestion. Niklesh - you can still use the
struct, but you'd have to read the members of the struct out one by
one. It's not clear whether that provides any meaningful benefit. You
still end up losing the benefit of being able to directly address the
styles. I think we have to abandon this approach. Sorry.
--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, Apr 26, 2015 at 08:22:56AM +0530, Niklesh Lalwani wrote:
> On Sun, Apr 26, 2015 at 6:56 AM, Michael Niedermayer
> > wrote:
> >
> >>
> >> > +struct __attribute__((__packed__)) StyleRecord {
> >
> >
> >> you cant do this
> >
> > attribute packed is not ANSI/ISO C
> >
> >
> >> How do I implement packed structures then here? ( since I don't want
> padding for proper alignment with the text sample)
you cant create packed structs, theres no portable way to do so
also there likely would be issues on either big or little endian
with such structs if they where used for reading data
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
> > > > On Sun, Apr 26, 2015 at 7:05 AM, Carl Eugen Hoyos > wrote: > > Niklesh Lalwani iitb.ac.in> writes: > > >> > +if ((text_style + i)->face_style_flags > > >> This looks very strange imo... > > What's wrong? I point my structure to the start of the style records, so, (text_style + i)->face_style_flags will give me the value of the attribute face_style_flags of (i+1)th style record. >> > +long tsmb_size; > > >> int64_t ? > > >> My bad. Yes, I intended to use int64_t / double. I will correct it. -- Niklesh Lalwani Third Year Undergraduate Electrical Engineering IIT Bombay ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, Apr 26, 2015 at 6:56 AM, Michael Niedermayer
> wrote:
>
>>
>> > +struct __attribute__((__packed__)) StyleRecord {
>
>
>> you cant do this
>
> attribute packed is not ANSI/ISO C
>
>
>> How do I implement packed structures then here? ( since I don't want
padding for proper alignment with the text sample)
>
>> also the patch doesnt apply
>
>
>> Applying: Using struct for StyleRecords and implementing support for
>> large boxes(>32 bit) for 3gpp timed text
>
> fatal: sha1 information is lacking or useless (libavcodec/movtextdec.c).
>
> Repository lacks necessary blobs to fall back on 3-way merge.
>
> Cannot fall back to three-way merge.
>
> Patch failed at 0001 Using struct for StyleRecords and implementing
>> support for large boxes(>32 bit) for 3gpp timed text
>
> When you have resolved this problem run "git am --resolved".
>
> If you would prefer to skip this patch, instead run "git am --skip".
>
> To restore the original branch and stop patching run "git am --abort".
>
>
What does this mean? Probably creating patch again would solve this. I will
post one after working on improvements over this patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
Niklesh Lalwani iitb.ac.in> writes: > +if ((text_style + i)->face_style_flags This looks very strange imo... > +long tsmb_size; int64_t ? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
On Sun, Apr 26, 2015 at 05:31:21AM +0530, Niklesh Lalwani wrote:
> From: Niklesh
>
> This patch uses struct for the Style Records. The pointer to this struct is
> updated to point to the start of style records in the TextStyleModifierBox.
> However, in doing this, I get a warning for incompatible pointer type
> assignment. How to get over this? The patch gives proper outputs though.
>
> Also, I have added support for large boxes(>32 bit) as per the ISO base media
> file format. Please let me know if I am doing it wrong.
>
> Signed-off-by: Niklesh
> ---
> libavcodec/movtextdec.c | 83
> -
> 1 file changed, 33 insertions(+), 50 deletions(-)
>
> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
> index ab4efdb..7eb41b8 100644
> --- a/libavcodec/movtextdec.c
> +++ b/libavcodec/movtextdec.c
> @@ -31,20 +31,28 @@
> #define STYLE_FLAG_ITALIC 2
> #define STYLE_FLAG_UNDERLINE4
>
> +struct __attribute__((__packed__)) StyleRecord {
you cant do this
attribute packed is not ANSI/ISO C
also the patch doesnt apply
Applying: Using struct for StyleRecords and implementing support for large
boxes(>32 bit) for 3gpp timed text
fatal: sha1 information is lacking or useless (libavcodec/movtextdec.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Using struct for StyleRecords and implementing support for
large boxes(>32 bit) for 3gpp timed text
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Using struct for StyleRecords and implementing support for large boxes(>32 bit) for 3gpp timed text
From: Niklesh
This patch uses struct for the Style Records. The pointer to this struct is
updated to point to the start of style records in the TextStyleModifierBox.
However, in doing this, I get a warning for incompatible pointer type
assignment. How to get over this? The patch gives proper outputs though.
Also, I have added support for large boxes(>32 bit) as per the ISO base media
file format. Please let me know if I am doing it wrong.
Signed-off-by: Niklesh
---
libavcodec/movtextdec.c | 83 -
1 file changed, 33 insertions(+), 50 deletions(-)
diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
index ab4efdb..7eb41b8 100644
--- a/libavcodec/movtextdec.c
+++ b/libavcodec/movtextdec.c
@@ -31,20 +31,28 @@
#define STYLE_FLAG_ITALIC 2
#define STYLE_FLAG_UNDERLINE4
+struct __attribute__((__packed__)) StyleRecord {
+uint16_t startChar;
+uint16_t endChar;
+uint16_t font_ID;
+uint8_t face_style_flags;
+uint8_t font_size;
+uint8_t text_color_rgba[4];
+};
+
static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end,
-int **style_start, int **style_end,
-int **style_flags, int style_entries)
+struct StyleRecord *text_style, int entry_count)
{
int i = 0;
int style_pos = 0;
while (text < text_end) {
-for (i = 0; i < style_entries; i++) {
-if (*style_flags[i] && style_pos == *style_start[i]) {
-if (*style_flags[i] & STYLE_FLAG_BOLD)
+for (i = 0; i < entry_count; i++) {
+if ((text_style + i)->face_style_flags && style_pos ==
AV_RB16(&((text_style + i)->startChar))) {
+if ((text_style + i)->face_style_flags & STYLE_FLAG_BOLD)
av_bprintf(buf, "{\\b1}");
-if (*style_flags[i] & STYLE_FLAG_ITALIC)
+if ((text_style + i)->face_style_flags & STYLE_FLAG_ITALIC)
av_bprintf(buf, "{\\i1}");
-if (*style_flags[i] & STYLE_FLAG_UNDERLINE)
+if ((text_style + i)->face_style_flags & STYLE_FLAG_UNDERLINE)
av_bprintf(buf, "{\\u1}");
}
}
@@ -60,13 +68,13 @@ static int text_to_ass(AVBPrint *buf, const char *text,
const char *text_end,
break;
}
-for (i = 0; i < style_entries; i++) {
-if (*style_flags[i] && style_pos == *style_end[i]) {
-if (*style_flags[i] & STYLE_FLAG_BOLD)
+for (i = 0; i < entry_count; i++) {
+if ((text_style + i)->face_style_flags && style_pos ==
AV_RB16(&((text_style + i)->endChar))) {
+if ((text_style + i)->face_style_flags & STYLE_FLAG_BOLD)
av_bprintf(buf, "{\\b0}");
-if (*style_flags[i] & STYLE_FLAG_ITALIC)
+if ((text_style + i)->face_style_flags & STYLE_FLAG_ITALIC)
av_bprintf(buf, "{\\i0}");
-if (*style_flags[i] & STYLE_FLAG_UNDERLINE)
+if ((text_style + i)->face_style_flags & STYLE_FLAG_UNDERLINE)
av_bprintf(buf, "{\\u0}");
}
}
@@ -95,15 +103,10 @@ static int mov_text_decode_frame(AVCodecContext *avctx,
AVBPrint buf;
char *ptr = avpkt->data;
char *end;
-//char *ptr_temp;
-int text_length, tsmb_type, style_entries, tsmb_size;
-int **style_start = {0,};
-int **style_end = {0,};
-int **style_flags = {0,};
-const uint8_t *tsmb;
-int index, i;
-int *flag;
-int *style_pos;
+int text_length, tsmb_type, entry_count;
+long tsmb_size;
+uint8_t *tsmb;
+struct StyleRecord *text_style;
if (!ptr || avpkt->size < 2)
return AVERROR_INVALIDDATA;
@@ -145,40 +148,20 @@ static int mov_text_decode_frame(AVCodecContext *avctx,
tsmb_type = AV_RB32(tsmb);
tsmb += 4;
+if (tsmb_size == 1) {
+tsmb_size = AV_RB64(tsmb);
+tsmb += 8;
+}
+
if (tsmb_type == MKBETAG('s','t','y','l')) {
-style_entries = AV_RB16(tsmb);
+entry_count = AV_RB16(tsmb);
tsmb += 2;
-
-for(i = 0; i < style_entries; i++) {
-style_pos = av_malloc(4);
-*style_pos = AV_RB16(tsmb);
-index = i;
-av_dynarray_add(&style_start, &index, style_pos);
-tsmb += 2;
-style_pos = av_malloc(4);
-*style_pos = AV_RB16(tsmb);
-index = i;
-av_dynarray_add(&style_end, &index, style_pos);
-tsmb += 2;
-// fontID = AV_RB16(tsmb);
-tsmb += 2;
-flag = av_malloc(4);
-*flag = AV_RB8(tsmb);
-index =
