[FFmpeg-devel] [PATCH] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
Aman Karmani
ffmpeg at tmm1.net
Thu Apr 8 23:24:41 EEST 2021
On Wed, Jan 27, 2021 at 3:40 PM <levi at snapstream.com> wrote:
> From: Levi Dooley <i.am.stickfigure at gmail.com>
>
> There was an assumption in the existing code that indentation would not
> occur more than once on the same row.
> This was a bad assumption. There are examples of 608 streams which call
> handle_pac multiple times on the same row with different indentation.
> As the code was before this change, the new indentation would overwrite
> existing text with spaces.
> These changes make indentation skip over columns instead. Text gets
> cleared with spaces on handle_edm.
> Instead of relying on the null character, trailing spaces are trimmed off
> the end of a row.
> This is necessary so that a null character doesn't end up between two
> words.
>
> Signed-off-by: Levi Dooley <i.am.stickfigure at gmail.com>
>
Tried this out and it looks good to me. I can confirm it fixes subtle
issues where content was previously being overwritten.
Aman
> ---
> libavcodec/ccaption_dec.c | 56 ++++++++++++++++++++++++++++++++-------
> tests/ref/fate/sub-scc | 2 +-
> 2 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index a208e19b95..525e010897 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -352,6 +352,17 @@ static void write_char(CCaptionSubContext *ctx,
> struct Screen *screen, char ch)
> }
> }
>
> +/**
> + * Increment the cursor_column by 1, and ensure that there are no null
> characters left behind in the row.
> + */
> +static void skip_char(CCaptionSubContext *ctx, struct Screen *screen)
> +{
> + if (!screen->characters[ctx->cursor_row][ctx->cursor_column])
> + write_char(ctx, screen, ' ');
> + else
> + ctx->cursor_column++;
> +}
> +
> /**
> * This function after validating parity bit, also remove it from data
> pair.
> * The first byte doesn't pass parity, we replace it with a solid blank
> @@ -459,6 +470,7 @@ static int capture_screen(CCaptionSubContext *ctx)
> if (CHECK_FLAG(screen->row_used, i)) {
> const char *row = screen->characters[i];
> const char *charset = screen->charsets[i];
> +
> j = 0;
> while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN)
> j++;
> @@ -476,13 +488,19 @@ static int capture_screen(CCaptionSubContext *ctx)
> const char *color = screen->colors[i];
> const char *charset = screen->charsets[i];
> const char *override;
> - int x, y, seen_char = 0;
> + int x, y, row_end, seen_char = 0;
> j = 0;
>
> /* skip leading space */
> while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN &&
> j < tab)
> j++;
>
> + /* skip trailing space */
> + row_end = SCREEN_COLUMNS-1;
> + while (row_end >= 0 && row[row_end] == ' ' &&
> charset[row_end] == CCSET_BASIC_AMERICAN) {
> + row_end--;
> + }
> +
> 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);
> @@ -490,7 +508,7 @@ static int capture_screen(CCaptionSubContext *ctx)
> for (; j < SCREEN_COLUMNS; j++) {
> const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag
> = "";
>
> - if (row[j] == 0)
> + if (j > row_end || row[j] == 0)
> break;
>
> if (prev_font != font[j]) {
> @@ -624,7 +642,8 @@ static void handle_textattr(CCaptionSubContext *ctx,
> uint8_t hi, uint8_t lo)
> ctx->cursor_font = pac2_attribs[i][1];
>
> SET_FLAG(screen->row_used, ctx->cursor_row);
> - write_char(ctx, screen, ' ');
> +
> + skip_char(ctx, screen);
> }
>
> static void handle_pac(CCaptionSubContext *ctx, uint8_t hi, uint8_t lo)
> @@ -644,13 +663,13 @@ static void handle_pac(CCaptionSubContext *ctx,
> uint8_t hi, uint8_t lo)
> lo &= 0x1f;
>
> ctx->cursor_row = row_map[index] - 1;
> - ctx->cursor_color = pac2_attribs[lo][0];
> + ctx->cursor_color = pac2_attribs[lo][0];
> ctx->cursor_font = pac2_attribs[lo][1];
> ctx->cursor_charset = CCSET_BASIC_AMERICAN;
> ctx->cursor_column = 0;
> indent = pac2_attribs[lo][2];
> for (i = 0; i < indent; i++) {
> - write_char(ctx, screen, ' ');
> + skip_char(ctx, screen);
> }
> }
>
> @@ -667,6 +686,14 @@ static int handle_edm(CCaptionSubContext *ctx)
> screen->row_used = 0;
> ctx->bg_color = CCCOL_BLACK;
>
> + for (int i = 0; i < SCREEN_ROWS+1; ++i) {
> + memset(screen->characters[i], ' ',
> SCREEN_COLUMNS);
> + memset(screen->colors[i], CCCOL_WHITE,
> SCREEN_COLUMNS);
> + memset(screen->bgs[i], CCCOL_BLACK,
> SCREEN_COLUMNS);
> + memset(screen->charsets[i], CCSET_BASIC_AMERICAN,
> SCREEN_COLUMNS);
> + memset(screen->fonts[i], CCFONT_REGULAR,
> SCREEN_COLUMNS);
> + }
> +
> // In realtime mode, emit an empty caption so the last one doesn't
> // stay on the screen.
> if (ctx->real_time)
> @@ -687,6 +714,7 @@ static int handle_eoc(CCaptionSubContext *ctx)
> ret = handle_edm(ctx);
>
> ctx->cursor_column = 0;
> + ctx->cursor_row = 0;
>
> // In realtime mode, we display the buffered contents (after
> // flipping the buffer to active above) as soon as EOC arrives.
> @@ -731,7 +759,6 @@ static void handle_char(CCaptionSubContext *ctx, char
> hi, char lo)
> if (lo) {
> write_char(ctx, screen, lo);
> }
> - write_char(ctx, screen, 0);
>
> if (ctx->mode != CCMODE_POPON)
> ctx->screen_touched = 1;
> @@ -742,6 +769,18 @@ static void handle_char(CCaptionSubContext *ctx, char
> hi, char lo)
> ff_dlog(ctx, "(%c)\n", hi);
> }
>
> +static void handle_spacing(CCaptionSubContext *ctx, char lo)
> +{
> + int i;
> + struct Screen *screen = get_writing_screen(ctx);
> +
> + SET_FLAG(screen->row_used, ctx->cursor_row);
> +
> + for (i = 0; i < lo - 0x20; i++) {
> + skip_char(ctx, screen);
> + }
> +}
> +
> static int process_cc608(CCaptionSubContext *ctx, uint8_t hi, uint8_t lo)
> {
> int ret = 0;
> @@ -823,11 +862,8 @@ static int process_cc608(CCaptionSubContext *ctx,
> uint8_t hi, uint8_t lo)
> handle_char(ctx, hi, lo);
> ctx->prev_cmd[0] = ctx->prev_cmd[1] = 0;
> } else if (hi == 0x17 && lo >= 0x21 && lo <= 0x23) {
> - int i;
> /* Tab offsets (spacing) */
> - for (i = 0; i < lo - 0x20; i++) {
> - handle_char(ctx, ' ', 0);
> - }
> + handle_spacing(ctx, lo);
> } else {
> /* Ignoring all other non data code */
> ff_dlog(ctx, "Unknown command 0x%hhx 0x%hhx\n", hi, lo);
> diff --git a/tests/ref/fate/sub-scc b/tests/ref/fate/sub-scc
> index 62cbf6fa4a..e4c1a1d004 100644
> --- a/tests/ref/fate/sub-scc
> +++ b/tests/ref/fate/sub-scc
> @@ -16,7 +16,7 @@ Dialogue:
> 0,0:00:00.69,0:00:03.29,Default,,0,0,0,,{\an7}{\pos(115,228)}[ Crowd ]
> Dialogue: 0,0:00:03.30,0:00:07.07,Default,,0,0,0,,{\an7}{\pos(38,197)}HOW
> DO YOU KNOW\N{\an7}{\pos(38,213)}SHE IS A WITCH ?\N{\an7}{\pos(153,243)}SHE
> LOOKS LIKE ONE !
> Dialogue: 0,0:00:07.07,0:00:09.27,Default,,0,0,0,,{\an7}{\pos(192,228)}[
> Shouting\N{\an7}{\pos(192,243)}\h\hAffirmations ]
> Dialogue:
> 0,0:00:09.26,0:00:11.06,Default,,0,0,0,,{\an7}{\pos(38,243)}BRING HER
> FORWARD.
> -Dialogue:
> 0,0:00:11.07,0:00:14.27,Default,,0,0,0,,{\an7}{\pos(115,228)}I’M NOT A
> WITCH.\N{\an7}{\pos(115,243)}\hI’M{\i1} NOT{\i0} A WITCH.
> +Dialogue:
> 0,0:00:11.07,0:00:14.27,Default,,0,0,0,,{\an7}{\pos(115,228)}I’M NOT A
> WITCH.\N{\an7}{\pos(115,243)}\hI’M {\i1}NOT{\i0} A WITCH.
> Dialogue: 0,0:00:14.26,0:00:16.03,Default,,0,0,0,,{\an7}{\pos(38,228)}BUT
> YOU ARE DRESSED\N{\an7}{\pos(38,243)}AS ONE.
> Dialogue:
> 0,0:00:16.03,0:00:19.03,Default,,0,0,0,,{\an7}{\pos(76,197)}THEY DRESSED ME
> UP\N{\an7}{\pos(76,213)}LIKE THIS.\N{\an7}{\pos(76,243)}\h\h\h\h\h\h\h\hNO
> ! WE DIDN’T !
> Dialogue:
> 0,0:00:19.03,0:00:22.95,Default,,0,0,0,,{\an7}{\pos(115,228)}AND THIS ISN’T
> MY NOSE.\N{\an7}{\pos(115,243)}IT’S A FALSE ONE.
> --
> 2.25.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list