Re: [FFmpeg-devel] [PATCH v4 1/2] avcodec/ccaption_dec: don't print multiple \an and \pos tags

2024-03-25 Thread Marth64
This should be withheld until after 7.0. In the same bug report it appears
that 1 user is making use of the erroneous tags via an external script for
a valid purpose. I don’t want to break people’s workflow even though it’s
invalid ASS.

After 7.0 I will revive this patchset and with additional fixes.

I apologize for the inconvenience. Please don’t merge, and I’ll be more
careful next time..
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v4 1/2] avcodec/ccaption_dec: don't print multiple \an and \pos tags

2024-03-25 Thread Marth64
This also fixes https://trac.ffmpeg.org/ticket/10927 (recently reported by
a user)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v4 1/2] avcodec/ccaption_dec: don't print multiple \an and \pos tags

2024-03-23 Thread Marth64
Closed Captions decoder prints multiple \pos ASS tags per cue,
and preceding the \pos tag is a fixed \an7 that fixes rendering
position from the top left corner and enforces justification.

Printing multiple \an and \pos tags in this way is invalid behavior,
because only the first \pos tag in a cue is honored by ASS anyway.
Don't write multiple \an and \pos tags. Also, update tests accordingly.

Note that when applying the patch, whitespace warnings may appear
about the test data. The subtitle stream in this test data has
intentional whitespace at the end of line for some cues, this is OK.

Signed-off-by: Marth64 
---
 libavcodec/ccaption_dec.c  |  13 ++-
 tests/ref/fate/sub-cc  |   2 +-
 tests/ref/fate/sub-cc-realtime |   8 +-
 tests/ref/fate/sub-cc-scte20   |   4 +-
 tests/ref/fate/sub-scc | 146 -
 5 files changed, 91 insertions(+), 82 deletions(-)

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index d8b992bb94..0b01765ccf 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -20,6 +20,8 @@
  */
 
 #include 
+#include 
+
 #include "avcodec.h"
 #include "ass.h"
 #include "codec_internal.h"
@@ -457,7 +459,7 @@ static void roll_up(CCaptionSubContext *ctx)
 
 static int capture_screen(CCaptionSubContext *ctx)
 {
-int i, j, tab = 0;
+int i, j, tab = 0, seen_row = 0;
 struct Screen *screen = ctx->screen + ctx->active_screen;
 enum cc_font prev_font = CCFONT_REGULAR;
 enum cc_color_code prev_color = CCCOL_WHITE;
@@ -497,7 +499,14 @@ static int capture_screen(CCaptionSubContext *ctx)
 
 x = ASS_DEFAULT_PLAYRESX * (0.1 + 0.0250 * j);
 y = ASS_DEFAULT_PLAYRESY * (0.1 + 0.0533 * i);
-av_bprintf(>buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y);
+
+if (!seen_row) {
+av_bprintf(>buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y);
+seen_row = 1;
+}
+av_log(ctx, AV_LOG_TRACE, "rendering row: start_pts=%" PRId64 " 
end_pts=%" PRId64 " "
+  "x=%d y=%d\n",
+  ctx->buffer_time[0], 
ctx->buffer_time[1], x, y);
 
 for (; j < SCREEN_COLUMNS; j++) {
 const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag = "";
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index f5216b3de2..d4ab974c2f 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -13,4 +13,4 @@ Style: 
Default,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
 Dialogue: 0,0:00:00.83,0:00:02.97,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} 
inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:02.97,0:00:04.34,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number 
one
+Dialogue: 0,0:00:02.97,0:00:04.34,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N>> Safety remains our number one
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index bda890bfe5..da06307221 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -16,7 +16,7 @@ Dialogue: 
0,0:00:00.97,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}(
 Dialogue: 0,0:00:01.17,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} 
inaudibl
 Dialogue: 0,0:00:01.37,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} 
inaudible radio chat
 Dialogue: 0,0:00:01.57,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} 
inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:03.10,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>>
-Dialogue: 0,0:00:03.30,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety rema
-Dialogue: 0,0:00:03.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our numb
-Dialogue: 0,0:00:03.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number 
one
+Dialogue: 0,0:00:03.10,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N>>
+Dialogue: 0,0:00:03.30,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N>> Safety rema
+Dialogue: 0,0:00:03.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N>> Safety remains our numb
+Dialogue: 0,0:00:03.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} 
inaudible radio chatter{\i0} )\N>> Safety remains our number one
diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index 49715301de..09fa3fb330 100644
--- a/tests/ref/fate/sub-cc-scte20
+++ b/tests/ref/fate/sub-cc-scte20
@@ -13,5 +13,5 @@ Style: