On 05/04/2015 08:50 PM, Michael Niedermayer wrote:
On Mon, May 04, 2015 at 05:30:05PM +0530, Anshul wrote:

On 05/04/2015 05:27 PM, Anshul wrote:

On 05/04/2015 03:09 AM, Michael Niedermayer wrote:
On Sun, May 03, 2015 at 08:53:14PM +0200, Clément Bœsch wrote:
On Sun, May 03, 2015 at 06:46:15PM +0530, Anshul wrote:
On 05/02/2015 08:24 PM, Clément Bœsch wrote:
On Tue, Apr 28, 2015 at 07:50:15PM +0530, Anshul wrote:
On 04/28/2015 02:14 PM, Clément Bœsch wrote:
Then FATE test patch should be applied after the CC patch.

New patch attached. I have used other ass api.
+fate-sub-cc: CMD = fmtstdout ass -f lavfi -i 
"movie=$(TARGET_SAMPLES)/sub/Closedcaption_atsc_rollup.ts[out0+subcc]"

BTW, do we really need to go through libavfilter to extract the cc?
Yes, there is no other way then libavfilter that I know.
Closed caption was
possible only after nicolas patch.
if you want more details, Why libavfilter was chosen,
you might need to read
last year's mail chain.
there has been long debate on it. so I don't want to
indulge myself again in
it.
+
  FATE_SUBTITLES_ASS-$(call DEMDEC, JACOSUB,
JACOSUB) += fate-sub-jacosub
  fate-sub-jacosub: CMD = fmtstdout ass -i
$(TARGET_SAMPLES)/sub/JACOsub_capability_tester.jss
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
new file mode 100644
index 0000000..035a51a
--- /dev/null
+++ b/tests/ref/fate/sub-cc
@@ -0,0 +1,38 @@
+[Script Info]
+; Script generated by FFmpeg/Lavc
+ScriptType: v4.00+
+PlayResX: 384
+PlayResY: 288
+
+[V4+ Styles]
+Format: Name, Fontname, Fontsize, PrimaryColour,
SecondaryColour, OutlineColour, BackColour, Bold,
Italic, Underline, StrikeOut, ScaleX, ScaleY,
Spacing, Angle, BorderStyle, Outline, Shadow,
Alignment, MarginL, MarginR, MarginV, Encoding
+Style: 
Default,Arial,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,0
+
+[Events]
+Format: Layer, Start, End, Style, Name, MarginL,
MarginR, MarginV, Effect, Text
+Dialogue: 0,0:00:36.87,0:00:38.37,Default,,0,0,0,,( explosion )
+Dialogue: 0,0:00:38.37,0:00:40.51,Default,,0,0,0,,(
explosion )\N( inaudible radio chatter )
+Dialogue: 0,0:00:40.51,0:00:41.88,Default,,0,0,0,,(
inaudible radio chatter )\N>> Safety remains our
number one
+Dialogue:
0,0:00:41.88,0:00:43.34,Default,,0,0,0,,>> Safety
remains our number one\Npriority.
+Dialogue:
0,0:00:43.34,0:00:44.51,Default,,0,0,0,,priority.\N>>
BP can talk about safety all
+Dialogue:
0,0:00:44.51,0:00:45.88,Default,,0,0,0,,>> BP can
talk about safety all\Nthey want, but they're not
going
+Dialogue:
0,0:00:45.88,0:00:47.95,Default,,0,0,0,,they want,
but they're not going\Nto become a safer company.
+Dialogue:
0,0:00:47.95,0:00:49.52,Default,,0,0,0,,to become a
safer company.\N>> They base everything on risk.
+Dialogue:
0,0:00:49.52,0:00:50.65,Default,,0,0,0,,>> They base
everything on risk.\N"How many lives can we afford
to
+Dialogue:
0,0:00:50.65,0:00:51.65,Default,,0,0,0,,"How many
lives can we afford to\Nlose before we need to deal
with
+Dialogue:
0,0:00:51.65,0:00:52.55,Default,,0,0,0,,lose before
we need to deal with\Nthis?"
+Dialogue: 0,0:00:52.55,0:00:53.25,Default,,0,0,0,,this?"\N>> 9-1.
+Dialogue:
0,0:00:53.25,0:00:55.22,Default,,0,0,0,,>> 9-1.\N>>
Yes, this plant just blew up!
+Dialogue:
0,0:00:55.22,0:00:57.19,Default,,0,0,0,,>> Yes, this
plant just blew up!\N>> From Texas and Alaska to the
+Dialogue:
0,0:00:57.19,0:00:58.39,Default,,0,0,0,,>> From
Texas and Alaska to the\NGulf of Mexico...
+Dialogue:
0,0:00:58.39,0:01:00.19,Default,,0,0,0,,Gulf of
Mexico...\N>> BP apologized again...
+Dialogue:
0,0:01:00.19,0:01:02.33,Default,,0,0,0,,>> BP
apologized again...\N>> ... Apology after apology.
+Dialogue:
0,0:01:02.33,0:01:03.10,Default,,0,0,0,,>> ...
Apology after apology.\N>> They pledged repeatedly
to
+Dialogue:
0,0:01:03.10,0:01:05.40,Default,,0,0,0,,>> They
pledged repeatedly to\Nrun a safer operation, yet
they
+Dialogue:
0,0:01:05.40,0:01:07.17,Default,,0,0,0,,run a safer
operation, yet they\Ncontinued to cut costs.
+Dialogue:
0,0:01:07.17,0:01:08.67,Default,,0,0,0,,continued to
cut costs.\N>> We have a facility here that
+Dialogue:
0,0:01:08.67,0:01:10.17,Default,,0,0,0,,>> We have a
facility here that\Ncould produce a cloud of gas
+Dialogue:
0,0:01:10.17,0:01:11.57,Default,,0,0,0,,could
produce a cloud of gas\Nthat would make this place
look
+Dialogue:
0,0:01:11.57,0:01:13.04,Default,,0,0,0,,that would
make this place look\Nlike Hiroshima.
+Dialogue:
0,0:01:13.04,0:01:15.21,Default,,0,0,0,,like
Hiroshima.\N>> What went wrong at BP?
+Dialogue:
0,0:01:15.21,0:01:17.11,Default,,0,0,0,,>> What went
wrong at BP?\N>> The culture of BP management
This output looks better.



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 From 13dedb5bd7df3efd88f6a3b0dbc521e869d8f6a2 Mon Sep
17 00:00:00 2001
From: Anshul Maheshwari <er.anshul.maheshw...@gmail.com>
Date: Tue, 28 Apr 2015 19:41:28 +0530
Subject: [PATCH] Correcting jumbled ass of captions

Signed-off-by: Anshul Maheshwari <er.anshul.maheshw...@gmail.com>
---
  libavcodec/ccaption_dec.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index da43ca6..540e17f 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -338,6 +338,10 @@ static int
reap_screen(CCaptionSubContext *ctx, int64_t pts)
          }
      }
+    if(screen->row_used) {
+        ctx->buffer.len -= 2;
+        ctx->buffer.str[ctx->buffer.len] = 0;
+    }
Please add an extra check about the ctx->buffer.len being >= 2

      ctx->startv_time = pts;
      ctx->end_time = pts;
      return ret;
@@ -550,7 +554,7 @@ static int decode(AVCodecContext
*avctx, void *data, int *got_sub, AVPacket *avp
              int start_time =
av_rescale_q(ctx->start_time, avctx->time_base,
(AVRational){ 1, 100 });
              int end_time = av_rescale_q(ctx->end_time,
avctx->time_base, (AVRational){ 1, 100 });
              av_dlog(ctx, "cdp writing data
(%s)\n",ctx->buffer.str);
-            ret = ff_ass_add_rect(sub, ctx->buffer.str,
start_time, end_time - start_time , 0);
+            ret = ff_ass_add_rect_bprint(sub,
&ctx->buffer, start_time, end_time - start_time);
              if (ret < 0)
                  return ret;
              sub->pts = av_rescale_q(ctx->start_time,
avctx->time_base, AV_TIME_BASE_Q);
LGTM otherwise

New patch attached.

-Anshul
 From 5e801ff90102f886507213bab7a7e1e827e5b2ab Mon Sep 17
00:00:00 2001
From: Anshul Maheshwari <er.anshul.maheshw...@gmail.com>
Date: Sun, 3 May 2015 18:37:40 +0530
Subject: [PATCH] correcting line breaks in cc

Signed-off-by: Anshul Maheshwari <er.anshul.maheshw...@gmail.com>
---
  libavcodec/ccaption_dec.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index da43ca6..264d21c 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -338,6 +338,10 @@ static int
reap_screen(CCaptionSubContext *ctx, int64_t pts)
          }
        }
+    if(screen->row_used && ctx->buffer.len >= 2 ) {
+        ctx->buffer.len -= 2;
+        ctx->buffer.str[ctx->buffer.len] = 0;
+    }
      ctx->startv_time = pts;
      ctx->end_time = pts;
      return ret;
@@ -550,7 +554,7 @@ static int decode(AVCodecContext *avctx,
void *data, int *got_sub, AVPacket *avp
              int start_time = av_rescale_q(ctx->start_time,
avctx->time_base, (AVRational){ 1, 100 });
              int end_time = av_rescale_q(ctx->end_time,
avctx->time_base, (AVRational){ 1, 100 });
              av_dlog(ctx, "cdp writing data (%s)\n",ctx->buffer.str);
-            ret = ff_ass_add_rect(sub, ctx->buffer.str,
start_time, end_time - start_time , 0);
+            ret = ff_ass_add_rect_bprint(sub, &ctx->buffer,
start_time, end_time - start_time);
              if (ret < 0)
                  return ret;
              sub->pts = av_rescale_q(ctx->start_time,
avctx->time_base, AV_TIME_BASE_Q);
LGTM (apart from the style, but the whole file is broken in that regard
anyway)
patch applied

[...]


Actual test case patch is still not applied. we applied line break
patch which was by-product of the actual patch.
I am attaching patch again here.

-Anshul
Here is link of video which need to copied in the correct place.
http://gsocdev.ccextractor.org/~anshul/test_video/Closedcaption_rollup.ts
this is 31mb, that does not seem reasonable for a subtitle test,
please provide a smaller file, like one without unused/untested
video streams or a shorter one

also fate fails with the patch applied
probably line endings arent preserved somewhere, probably best to
push to github and post a link

--- ./tests/ref/fate/sub-cc     2015-05-04 16:28:48.471575601 +0200
+++ tests/data/fate/sub-cc      2015-05-04 17:09:24.243626916 +0200
@@ -1,38 +1,38 @@
-[Script Info]
-; Script generated by FFmpeg/Lavc
-ScriptType: v4.00+
-PlayResX: 384
-PlayResY: 288
-
-[V4+ Styles]
-Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, 
OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, 
Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, 
MarginV, Encoding
-Style: 
Default,Arial,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,0
-
-[Events]
-Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
-Dialogue: 0,0:00:36.87,0:00:38.37,Default,,0,0,0,,( explosion )
-Dialogue: 0,0:00:38.37,0:00:40.51,Default,,0,0,0,,( explosion )\N( inaudible 
radio chatter )
-Dialogue: 0,0:00:40.51,0:00:41.88,Default,,0,0,0,,( inaudible radio chatter 
)\N>> Safety remains our number one
-Dialogue: 0,0:00:41.88,0:00:43.34,Default,,0,0,0,,>> Safety remains our number 
one\Npriority.
-Dialogue: 0,0:00:43.34,0:00:44.51,Default,,0,0,0,,priority.\N>> BP can talk 
about safety all
-Dialogue: 0,0:00:44.51,0:00:45.88,Default,,0,0,0,,>> BP can talk about safety 
all\Nthey want, but they're not going
-Dialogue: 0,0:00:45.88,0:00:47.95,Default,,0,0,0,,they want, but they're not 
going\Nto become a safer company.
-Dialogue: 0,0:00:47.95,0:00:49.52,Default,,0,0,0,,to become a safer 
company.\N>> They base everything on risk.
-Dialogue: 0,0:00:49.52,0:00:50.65,Default,,0,0,0,,>> They base everything on 
risk.\N"How many lives can we afford to
-Dialogue: 0,0:00:50.65,0:00:51.65,Default,,0,0,0,,"How many lives can we 
afford to\Nlose before we need to deal with
-Dialogue: 0,0:00:51.65,0:00:52.55,Default,,0,0,0,,lose before we need to deal 
with\Nthis?"
-Dialogue: 0,0:00:52.55,0:00:53.25,Default,,0,0,0,,this?"\N>> 9-1.
-Dialogue: 0,0:00:53.25,0:00:55.22,Default,,0,0,0,,>> 9-1.\N>> Yes, this plant 
just blew up!
-Dialogue: 0,0:00:55.22,0:00:57.19,Default,,0,0,0,,>> Yes, this plant just blew 
up!\N>> From Texas and Alaska to the
-Dialogue: 0,0:00:57.19,0:00:58.39,Default,,0,0,0,,>> From Texas and Alaska to 
the\NGulf of Mexico...
-Dialogue: 0,0:00:58.39,0:01:00.19,Default,,0,0,0,,Gulf of Mexico...\N>> BP 
apologized again...
-Dialogue: 0,0:01:00.19,0:01:02.33,Default,,0,0,0,,>> BP apologized 
again...\N>> ... Apology after apology.
-Dialogue: 0,0:01:02.33,0:01:03.10,Default,,0,0,0,,>> ... Apology after 
apology.\N>> They pledged repeatedly to
-Dialogue: 0,0:01:03.10,0:01:05.40,Default,,0,0,0,,>> They pledged repeatedly 
to\Nrun a safer operation, yet they
-Dialogue: 0,0:01:05.40,0:01:07.17,Default,,0,0,0,,run a safer operation, yet 
they\Ncontinued to cut costs.
-Dialogue: 0,0:01:07.17,0:01:08.67,Default,,0,0,0,,continued to cut costs.\N>> 
We have a facility here that
-Dialogue: 0,0:01:08.67,0:01:10.17,Default,,0,0,0,,>> We have a facility here 
that\Ncould produce a cloud of gas
-Dialogue: 0,0:01:10.17,0:01:11.57,Default,,0,0,0,,could produce a cloud of 
gas\Nthat would make this place look
-Dialogue: 0,0:01:11.57,0:01:13.04,Default,,0,0,0,,that would make this place 
look\Nlike Hiroshima.
-Dialogue: 0,0:01:13.04,0:01:15.21,Default,,0,0,0,,like Hiroshima.\N>> What 
went wrong at BP?
-Dialogue: 0,0:01:15.21,0:01:17.11,Default,,0,0,0,,>> What went wrong at 
BP?\N>> The culture of BP management
+[Script Info]
+; Script generated by FFmpeg/Lavc
+ScriptType: v4.00+
+PlayResX: 384
+PlayResY: 288
+
+[V4+ Styles]
+Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, 
OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, 
Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, 
MarginV, Encoding
+Style: 
Default,Arial,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,0
+
+[Events]
+Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
+Dialogue: 0,0:00:36.87,0:00:38.37,Default,,0,0,0,,( explosion )
+Dialogue: 0,0:00:38.37,0:00:40.51,Default,,0,0,0,,( explosion )\N( inaudible 
radio chatter )
+Dialogue: 0,0:00:40.51,0:00:41.88,Default,,0,0,0,,( inaudible radio chatter 
)\N>> Safety remains our number one
+Dialogue: 0,0:00:41.88,0:00:43.34,Default,,0,0,0,,>> Safety remains our number 
one\Npriority.
+Dialogue: 0,0:00:43.34,0:00:44.51,Default,,0,0,0,,priority.\N>> BP can talk 
about safety all
+Dialogue: 0,0:00:44.51,0:00:45.88,Default,,0,0,0,,>> BP can talk about safety 
all\Nthey want, but they're not going
+Dialogue: 0,0:00:45.88,0:00:47.95,Default,,0,0,0,,they want, but they're not 
going\Nto become a safer company.
+Dialogue: 0,0:00:47.95,0:00:49.52,Default,,0,0,0,,to become a safer 
company.\N>> They base everything on risk.
+Dialogue: 0,0:00:49.52,0:00:50.65,Default,,0,0,0,,>> They base everything on 
risk.\N"How many lives can we afford to
+Dialogue: 0,0:00:50.65,0:00:51.65,Default,,0,0,0,,"How many lives can we 
afford to\Nlose before we need to deal with
+Dialogue: 0,0:00:51.65,0:00:52.55,Default,,0,0,0,,lose before we need to deal 
with\Nthis?"
+Dialogue: 0,0:00:52.55,0:00:53.25,Default,,0,0,0,,this?"\N>> 9-1.
+Dialogue: 0,0:00:53.25,0:00:55.22,Default,,0,0,0,,>> 9-1.\N>> Yes, this plant 
just blew up!
+Dialogue: 0,0:00:55.22,0:00:57.19,Default,,0,0,0,,>> Yes, this plant just blew 
up!\N>> From Texas and Alaska to the
+Dialogue: 0,0:00:57.19,0:00:58.39,Default,,0,0,0,,>> From Texas and Alaska to 
the\NGulf of Mexico...
+Dialogue: 0,0:00:58.39,0:01:00.19,Default,,0,0,0,,Gulf of Mexico...\N>> BP 
apologized again...
+Dialogue: 0,0:01:00.19,0:01:02.33,Default,,0,0,0,,>> BP apologized 
again...\N>> ... Apology after apology.
+Dialogue: 0,0:01:02.33,0:01:03.10,Default,,0,0,0,,>> ... Apology after 
apology.\N>> They pledged repeatedly to
+Dialogue: 0,0:01:03.10,0:01:05.40,Default,,0,0,0,,>> They pledged repeatedly 
to\Nrun a safer operation, yet they
+Dialogue: 0,0:01:05.40,0:01:07.17,Default,,0,0,0,,run a safer operation, yet 
they\Ncontinued to cut costs.
+Dialogue: 0,0:01:07.17,0:01:08.67,Default,,0,0,0,,continued to cut costs.\N>> 
We have a facility here that
+Dialogue: 0,0:01:08.67,0:01:10.17,Default,,0,0,0,,>> We have a facility here 
that\Ncould produce a cloud of gas
+Dialogue: 0,0:01:10.17,0:01:11.57,Default,,0,0,0,,could produce a cloud of 
gas\Nthat would make this place look
+Dialogue: 0,0:01:11.57,0:01:13.04,Default,,0,0,0,,that would make this place 
look\Nlike Hiroshima.
+Dialogue: 0,0:01:13.04,0:01:15.21,Default,,0,0,0,,like Hiroshima.\N>> What 
went wrong at BP?
+Dialogue: 0,0:01:15.21,0:01:17.11,Default,,0,0,0,,>> What went wrong at 
BP?\N>> The culture of BP management
Test sub-cc failed. Look at tests/data/fate/sub-cc.err for details.
make: *** [fate-sub-cc] Error 1
make: *** Waiting for unfinished jobs....


[...]


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I have created pull request here.
https://github.com/FFmpeg/FFmpeg/pull/136

and here is shorter file then previous one.
http://gsocdev.ccextractor.org/~anshul/test_video/Closedcaption_atsc_rollup.ts

-Anshul
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to