* [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue @ 2024-03-11 0:40 Marth64 2024-03-11 16:28 ` Stefano Sabatini 2024-03-11 21:43 ` Michael Niedermayer 0 siblings, 2 replies; 6+ messages in thread From: Marth64 @ 2024-03-11 0:40 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Marth64 Currently, Closed Captions decoder prints multiple \pos ASS tags per line, per cue. This is invalid behavior, because only the first \pos tag in a cue is honored by ASS anyway. Don't write multiple \pos tags. Signed-off-by: Marth64 <marth64@proxyid.net> --- libavcodec/ccaption_dec.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c index faf058ce97..9d4a93647c 100644 --- a/libavcodec/ccaption_dec.c +++ b/libavcodec/ccaption_dec.c @@ -456,7 +456,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; @@ -496,7 +496,11 @@ 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(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y); + + if (!seen_row) { + av_bprintf(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y); + seen_row = 1; + } for (; j < SCREEN_COLUMNS; j++) { const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag = ""; -- 2.34.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue 2024-03-11 0:40 [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue Marth64 @ 2024-03-11 16:28 ` Stefano Sabatini 2024-03-11 21:43 ` Michael Niedermayer 1 sibling, 0 replies; 6+ messages in thread From: Stefano Sabatini @ 2024-03-11 16:28 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Marth64 On date Sunday 2024-03-10 19:40:47 -0500, Marth64 wrote: > Currently, Closed Captions decoder prints multiple \pos ASS tags per line, > per cue. This is invalid behavior, because only the first \pos tag in a cue > is honored by ASS anyway. Don't write multiple \pos tags. > > Signed-off-by: Marth64 <marth64@proxyid.net> > --- > libavcodec/ccaption_dec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c > index faf058ce97..9d4a93647c 100644 > --- a/libavcodec/ccaption_dec.c > +++ b/libavcodec/ccaption_dec.c > @@ -456,7 +456,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; > @@ -496,7 +496,11 @@ 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(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y); > + > + if (!seen_row) { > + av_bprintf(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y); > + seen_row = 1; > + } > > for (; j < SCREEN_COLUMNS; j++) { > const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag = ""; LGTM, thanks. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue 2024-03-11 0:40 [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue Marth64 2024-03-11 16:28 ` Stefano Sabatini @ 2024-03-11 21:43 ` Michael Niedermayer 2024-03-11 21:48 ` Marth64 1 sibling, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2024-03-11 21:43 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 1595 bytes --] On Sun, Mar 10, 2024 at 07:40:47PM -0500, Marth64 wrote: > Currently, Closed Captions decoder prints multiple \pos ASS tags per line, > per cue. This is invalid behavior, because only the first \pos tag in a cue > is honored by ASS anyway. Don't write multiple \pos tags. > > Signed-off-by: Marth64 <marth64@proxyid.net> > --- > libavcodec/ccaption_dec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) This breaks fate --- ./tests/ref/fate/sub-cc-scte20 2024-03-05 02:37:36.223299993 +0100 +++ tests/data/fate/sub-cc-scte20 2024-03-11 22:13:08.091910294 +0100 @@ -13,5 +13,5 @@ [Events] Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text Dialogue: 0,0:00:00.00,0:00:01.44,Default,,0,0,0,,{\an7}{\pos(48,182)}BESIDES THE -Dialogue: 0,0:00:01.43,0:00:03.93,Default,,0,0,0,,{\an7}{\pos(38,166)}\hBESIDES THE \N{\an7}{\pos(38,197)}SPENDING AND THIS, IS THAT CAR -Dialogue: 0,0:00:03.94,0:00:06.31,Default,,0,0,0,,{\an7}{\pos(38,182)}SPENDING AND THIS, IS THAT CAR \N{\an7}{\pos(38,197)}MANUFACTURERS ARE ABOUT AS +Dialogue: 0,0:00:01.43,0:00:03.93,Default,,0,0,0,,{\an7}{\pos(38,166)}\hBESIDES THE \NSPENDING AND THIS, IS THAT CAR +Dialogue: 0,0:00:03.94,0:00:06.31,Default,,0,0,0,,{\an7}{\pos(38,182)}SPENDING AND THIS, IS THAT CAR \NMANUFACTURERS ARE ABOUT AS Test sub-cc-scte20 failed. Look at tests/data/fate/sub-cc-scte20.err for details. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Nations do behave wisely once they have exhausted all other alternatives. -- Abba Eban [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue 2024-03-11 21:43 ` Michael Niedermayer @ 2024-03-11 21:48 ` Marth64 2024-03-11 21:51 ` Leo Izen 0 siblings, 1 reply; 6+ messages in thread From: Marth64 @ 2024-03-11 21:48 UTC (permalink / raw) To: FFmpeg development discussions and patches > This breaks fate Looking into it - thanks. PS, sorry for previous top-posting in replies. Will be more cautious of that. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue 2024-03-11 21:48 ` Marth64 @ 2024-03-11 21:51 ` Leo Izen 2024-03-12 1:21 ` Marth64 0 siblings, 1 reply; 6+ messages in thread From: Leo Izen @ 2024-03-11 21:51 UTC (permalink / raw) To: ffmpeg-devel [-- Attachment #1.1.1: Type: text/plain, Size: 274 bytes --] On 3/11/24 17:48, Marth64 wrote: >> This breaks fate > Looking into it - thanks. > It is possible that the change is correct, and that the fate test needs to be changed to reflect the correct behavior. Not necessarily, but possibly. - Leo Izen (Traneptora) [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue 2024-03-11 21:51 ` Leo Izen @ 2024-03-12 1:21 ` Marth64 0 siblings, 0 replies; 6+ messages in thread From: Marth64 @ 2024-03-12 1:21 UTC (permalink / raw) To: FFmpeg development discussions and patches Thanks, will follow up with a v3 set (consolidating/resolving feedback on the other CC fixes as well). _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-12 1:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-11 0:40 [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: don't print multiple \pos tags per cue Marth64 2024-03-11 16:28 ` Stefano Sabatini 2024-03-11 21:43 ` Michael Niedermayer 2024-03-11 21:48 ` Marth64 2024-03-11 21:51 ` Leo Izen 2024-03-12 1:21 ` Marth64
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git