Thank you for your suggestions.
I'll try to do as much as I can before the deadline.
-Niklesh
On Wed, Apr 22, 2015 at 10:57 PM, Philip Langdale <phil...@overt.org> wrote:
> On 2015-04-22 03:10, Niklesh Lalwani wrote:
>
>> From: Niklesh <niklesh.lalw...@iitb.ac.in>
>>
>> This patch supports decoding of Bold, Italic, Underlined styles for
>> 3gpp timed text. While the code can be improved upon to make it more
>> clean and well structured, this works for now, even for multiple style
>> records. Suggestions awaited.
>> Signed-off-by: Niklesh <niklesh.lalw...@iitb.ac.in>
>> ---
>> libavcodec/movtextdec.c | 86
>> ++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 82 insertions(+), 4 deletions(-)
>>
>
> Thank you Niklesh.
>
> I will repeat my three primary items from the previous review as they all
> still
> apply:
>
> * You need to handle large (> 32bit) boxes.
> * You should store indices in style_start and style_end
> * You should define a struct TextSampleModifierBox and use that for
> de-serialization.
>
> I'm personally ok with you doing these as follow-up items, and not having
> them in to
> initial committed patch, but the others need to agree with that.
>
> Please address the syntax and warning issues that have already been
> pointed out.
>
>
>
>> diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c
>> index 1c7ffea..a4aa7cb 100644
>> --- a/libavcodec/movtextdec.c
>> +++ b/libavcodec/movtextdec.c
>> @@ -25,10 +25,28 @@
>> #include "libavutil/common.h"
>> #include "libavutil/bprint.h"
>> #include "libavutil/intreadwrite.h"
>> +#include "libavutil/mem.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, const int style_entries)
>> {
>> while (text < text_end) {
>> + for (int i=0; i<style_entries; i++) {
>> + 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) {
>> case '\r':
>> break;
>> @@ -39,6 +57,17 @@ static int text_to_ass(AVBPrint *buf, const char
>> *text, const char *text_end)
>> av_bprint_chars(buf, *text, 1);
>> break;
>> }
>> +
>> + for (int i=0; i<style_entries; i++) {
>> + 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++;
>> }
>>
>> @@ -63,6 +92,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, tsmb_size;
>> + char **style_start={0,};
>> + char **style_end={0,};
>> + int **style_flags={0,};
>> + const uint8_t *tsmb;
>> + int index,i, flag=0;;
>> + char *ptr_temp;
>>
>> if (!ptr || avpkt->size < 2)
>> return AVERROR_INVALIDDATA;
>> @@ -82,7 +118,8 @@ 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);
>> + text_length = AV_RB16(ptr);
>> + end = ptr + FFMIN(2 + text_length, avpkt->size);
>> ptr += 2;
>>
>> ts_start = av_rescale_q(avpkt->pts,
>> @@ -92,10 +129,51 @@ static int mov_text_decode_frame(AVCodecContext
>> *avctx,
>> 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;
>> +
>> + if (tsmb_type == MKBETAG('s','t','y','l')) {
>> + style_entries = AV_RB16(tsmb);
>> + tsmb += 2;
>> +
>> + for(i = 0; i < style_entries;i++) {
>> + ptr_temp= ptr + AV_RB16(tsmb);
>> + index=i;
>> + av_dynarray_add(&style_start, &index, ptr_temp);
>> + tsmb += 2;
>> + ptr_temp= ptr+ AV_RB16(tsmb);
>> + index=i;
>> + av_dynarray_add(&style_end, &index, ptr_temp);
>> + tsmb += 2;
>> + // fontID = AV_RB16(tsmb);
>> + tsmb += 2;
>> + flag=AV_RB8(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,style_entries);
>> + av_freep(&style_start);
>> + av_freep(&style_end);
>> + av_freep(&style_flags);
>> + }
>> + }
>> + }
>> + else
>> + 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);
>> if (ret < 0)
>> return ret;
>>
>
> --phil
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
--
Niklesh Lalwani
Third Year Undergraduate
Electrical Engineering
IIT Bombay
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
