[FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Oneric]

Changes from v3:
 - None.  Just rebased ontop of master to allow
   Patchwork to properly process the series now
   that 99d33cc661fbd04e8657831 was merged.

Changes from v2:
 - rebased ontop of the recently pushed eol normalisation.
   As a result no more CRLFs in here and Patchwork should be happy
 - added a fourth cosmetic commit adjusting
   explicit linebreaks to the new normalisation

Changes from v1:
 - ff_ass_bprint_text_event now only inserts a word-joiner
   if there isn’t already one anyway
 - added a third commit improving the handling of
   curly brackets for standard ASS renderers

Oneric (4):
  avcodec/webvttdec: honour bidi marks
  avcodec/{ass,webvttdec}: fix handling of backslashes
  avcodec/{ass,webvttdec}: more portable curly brace escapes
  avocdec/ass: simplify linebreaks

 libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
 libavcodec/webvttdec.c     |  4 ++--
 tests/ref/fate/sub-webvtt  |  2 +-
 tests/ref/fate/sub-webvtt2 |  2 +-
 4 files changed, 33 insertions(+), 22 deletions(-)

-- 
2.39.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 1/4] avcodec/webvttdec: honour bidi marks

[Oneric]

---
 libavcodec/webvttdec.c     | 2 +-
 tests/ref/fate/sub-webvtt2 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
index 690f00dc47..990d150f16 100644
--- a/libavcodec/webvttdec.c
+++ b/libavcodec/webvttdec.c
@@ -39,7 +39,7 @@ static const struct {
     {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
     {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
     {"&gt;", ">"}, {"&lt;", "<"},
-    {"&lrm;", ""}, {"&rlm;", ""}, // FIXME: properly honor bidi marks
+    {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
     {"&amp;", "&"}, {"&nbsp;", "\\h"},
 };
 
diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
index 90f78d904b..2925d892a0 100644
--- a/tests/ref/fate/sub-webvtt2
+++ b/tests/ref/fate/sub-webvtt2
@@ -21,6 +21,6 @@ Dialogue: 0,0:00:12.50,0:00:32.50,Default,,0,0,0,,OK, let’s go.
 Dialogue: 0,0:00:38.00,0:00:43.00,Default,,0,0,0,,I want to 愛あい love you\NThat's not proper English!
 Dialogue: 0,0:00:43.00,0:00:46.00,Default,,0,0,0,,{\i1}キツネ{\i0}じゃない　キツネじゃない\N乙女おとめは
 Dialogue: 0,0:00:50.00,0:00:55.00,Default,,0,0,0,,Some time ago in a rather distant place....
-Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: 123456\NAscending: 123456
+Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: ‏123456‎\NAscending: 123456
 Dialogue: 0,0:01:00.00,0:01:05.00,Default,,0,0,0,,>> Never gonna give you up Never gonna let you down\NNever\hgonna\hrun\haround & desert\hyou
 Dialogue: 0,0:55:00.00,1:00:00.00,Default,,0,0,0,,Transcrit par Célestes™
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 1/4] avcodec/webvttdec: honour bidi marks

[Stefano Sabatini]

On date Monday 2024-02-19 22:42:24 +0100, Oneric wrote:
> ---
>  libavcodec/webvttdec.c     | 2 +-
>  tests/ref/fate/sub-webvtt2 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
> index 690f00dc47..990d150f16 100644
> --- a/libavcodec/webvttdec.c
> +++ b/libavcodec/webvttdec.c
> @@ -39,7 +39,7 @@ static const struct {
>      {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
>      {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
>      {"&gt;", ">"}, {"&lt;", "<"},
> -    {"&lrm;", ""}, {"&rlm;", ""}, // FIXME: properly honor bidi marks
> +    {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
>      {"&amp;", "&"}, {"&nbsp;", "\\h"},
>  };
>  
> diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
> index 90f78d904b..2925d892a0 100644
> --- a/tests/ref/fate/sub-webvtt2
> +++ b/tests/ref/fate/sub-webvtt2
> @@ -21,6 +21,6 @@ Dialogue: 0,0:00:12.50,0:00:32.50,Default,,0,0,0,,OK, let’s go.
>  Dialogue: 0,0:00:38.00,0:00:43.00,Default,,0,0,0,,I want to 愛あい love you\NThat's not proper English!
>  Dialogue: 0,0:00:43.00,0:00:46.00,Default,,0,0,0,,{\i1}キツネ{\i0}じゃない　キツネじゃない\N乙女おとめは
>  Dialogue: 0,0:00:50.00,0:00:55.00,Default,,0,0,0,,Some time ago in a rather distant place....
> -Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: 123456\NAscending: 123456
> +Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: ‏123456‎\NAscending: 123456
>  Dialogue: 0,0:01:00.00,0:01:05.00,Default,,0,0,0,,>> Never gonna give you up Never gonna let you down\NNever\hgonna\hrun\haround & desert\hyou
>  Dialogue: 0,0:55:00.00,1:00:00.00,Default,,0,0,0,,Transcrit par Célestes™
> -- 
> 2.39.2

Should be good, 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".

[FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Oneric]

Backslashes cannot be escaped by a backslash in any ASS renderer,
but unless followed by specific characters it is just printed out.
Insert a word-joiner character after a backslash to break up
active sequences without changing the visual output.
---
 libavcodec/ass.c       | 9 ++++++++-
 libavcodec/webvttdec.c | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 5058dc8337..a68d3568b4 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -183,9 +183,16 @@ void ff_ass_bprint_text_event(AVBPrint *buf, const char *p, int size,
 
         /* standard ASS escaping so random characters don't get mis-interpreted
          * as ASS */
-        } else if (!keep_ass_markup && strchr("{}\\", *p)) {
+        } else if (!keep_ass_markup && strchr("{}", *p)) {
             av_bprintf(buf, "\\%c", *p);
 
+        /* append word-joiner U+2060 as UTF-8 to break up sequences like \N */
+        } else if (!keep_ass_markup && *p == '\\') {
+            if (p_end - p <= 3 || strncmp(p + 1, "\xe2\x81\xa0", 3))
+                av_bprintf(buf, "\\\xe2\x81\xa0");
+            else
+                av_bprintf(buf, "\\");
+
         /* some packets might end abruptly (no \0 at the end, like for example
          * in some cases of demuxing from a classic video container), some
          * might be terminated with \n or \r\n which we have to remove (for
diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
index 990d150f16..6e55bc5499 100644
--- a/libavcodec/webvttdec.c
+++ b/libavcodec/webvttdec.c
@@ -37,7 +37,7 @@ static const struct {
     {"<i>", "{\\i1}"}, {"</i>", "{\\i0}"},
     {"<b>", "{\\b1}"}, {"</b>", "{\\b0}"},
     {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
-    {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
+    {"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
     {"&gt;", ">"}, {"&lt;", "<"},
     {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
     {"&amp;", "&"}, {"&nbsp;", "\\h"},
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Stefano Sabatini]

On date Monday 2024-02-19 22:42:25 +0100, Oneric wrote:
> Backslashes cannot be escaped by a backslash in any ASS renderer,
> but unless followed by specific characters it is just printed out.
> Insert a word-joiner character after a backslash to break up
> active sequences without changing the visual output.
> ---
>  libavcodec/ass.c       | 9 ++++++++-
>  libavcodec/webvttdec.c | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/ass.c b/libavcodec/ass.c
> index 5058dc8337..a68d3568b4 100644
> --- a/libavcodec/ass.c
> +++ b/libavcodec/ass.c
> @@ -183,9 +183,16 @@ void ff_ass_bprint_text_event(AVBPrint *buf, const char *p, int size,
>  
>          /* standard ASS escaping so random characters don't get mis-interpreted
>           * as ASS */
> -        } else if (!keep_ass_markup && strchr("{}\\", *p)) {
> +        } else if (!keep_ass_markup && strchr("{}", *p)) {
>              av_bprintf(buf, "\\%c", *p);
>  

> +        /* append word-joiner U+2060 as UTF-8 to break up sequences like \N */

I'm confused by this, what kind of \N sequences might appear in an ASS
file? Can you show an offending sequence?

> +        } else if (!keep_ass_markup && *p == '\\') {
> +            if (p_end - p <= 3 || strncmp(p + 1, "\xe2\x81\xa0", 3))
> +                av_bprintf(buf, "\\\xe2\x81\xa0");
> +            else
> +                av_bprintf(buf, "\\");
> +
>          /* some packets might end abruptly (no \0 at the end, like for example
>           * in some cases of demuxing from a classic video container), some
>           * might be terminated with \n or \r\n which we have to remove (for
> diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
> index 990d150f16..6e55bc5499 100644
> --- a/libavcodec/webvttdec.c
> +++ b/libavcodec/webvttdec.c
> @@ -37,7 +37,7 @@ static const struct {
>      {"<i>", "{\\i1}"}, {"</i>", "{\\i0}"},
>      {"<b>", "{\\b1}"}, {"</b>", "{\\b0}"},
>      {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
> -    {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
> +    {"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
>      {"&gt;", ">"}, {"&lt;", "<"},
>      {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
>      {"&amp;", "&"}, {"&nbsp;", "\\h"},
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Marth64]

> I'm confused by this, what kind of \N
> sequences might appear in an ASS file?
> Can you show an offending sequence?

Good day Stefano,

The one I tested with was something like :
Jim\Nancy

I made this up while testing. But I have seen similar in real world
scenarios particularly in ASS files originating from subtitles that are
“summarized”. In legacy content this is common, to fit content in a 4:3
screen the subtitle writer would shorten or abbreviate what is spoken on
the screen and slashes are a replacement for “and”.

Hope this helps.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Stefano Sabatini]

On date Thursday 2024-04-04 12:56:30 -0500, Marth64 wrote:
> > I'm confused by this, what kind of \N
> > sequences might appear in an ASS file?
> > Can you show an offending sequence?
> 
> Good day Stefano,
> 

> The one I tested with was something like :
> Jim\Nancy

Is N a literal or is this impacting also sequences in the form:
Jim\Joe?

Is \N a special sequence in ASS-speak or does it mean a slash followed
by any character?
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Marth64]

>  Is \N a special sequence in ASS-speak
\N is the special line break sequence in ASS

I’m not sure of other special sequences following the \ character, I yield
to Oneric on this question.

Best,
Marth64
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Oneric]

[-- Attachment #1.1: Type: text/plain, Size: 741 bytes --]

On Thu, Apr 04, 2024 at 13:26:55 -0500, Marth64 wrote:
> >  Is \N a special sequence in ASS-speak
> \N is the special line break sequence in ASS
> 
> I’m not sure of other special sequences following the \ character, I yield
> to Oneric on this question.

Standard ASS(2) and SSAv4 have
  \N (forced linebreak)
  \n (linebreak hint)
  \h (nonbreaking space)

libass further has \{ and \} dealt with by the following commit.
Afaik no ASS renderer ever supported \\.

The added escaping happens for non-ASS format (in which those sequences
are just normal text) which get encoded into ASS by ffmpeg. If it weren’t
escaped we’d get sudden linebreaks, spaces etc and missing text.
It doesn’t affect ASS->ASS remuxing.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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".

Re: [FFmpeg-devel] [PATCH v4 2/4] avcodec/{ass, webvttdec}: fix handling of backslashes

[Stefano Sabatini]

On date Thursday 2024-04-04 20:36:50 +0200, Oneric wrote:
> On Thu, Apr 04, 2024 at 13:26:55 -0500, Marth64 wrote:
> > >  Is \N a special sequence in ASS-speak
> > \N is the special line break sequence in ASS
> > 
> > I’m not sure of other special sequences following the \ character, I yield
> > to Oneric on this question.
> 
> Standard ASS(2) and SSAv4 have
>   \N (forced linebreak)
>   \n (linebreak hint)
>   \h (nonbreaking space)
> 
> libass further has \{ and \} dealt with by the following commit.
> Afaik no ASS renderer ever supported \\.
> 
> The added escaping happens for non-ASS format (in which those sequences
> are just normal text) which get encoded into ASS by ffmpeg. If it weren’t
> escaped we’d get sudden linebreaks, spaces etc and missing text.
> It doesn’t affect ASS->ASS remuxing.

Thanks for the clarification.

I think the patcheset should be good, but I'll wait a few days for
letting more comments.

I can fix the typos before pushing, or feel free to send an update.
_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 3/4] avcodec/{ass, webvttdec}: more portable curly brace escapes

[Oneric]

Unlike what the old comment suggested, standard ASS has no character
escape mechanism, but a closing curly bracket doesn't even need one.

For manual authored sub files using a full-width variant of an apropiate
font and with scaling and psacing modifiers is a common workaround.
This is not an option here, but we can still make things much less bad.
Now the desired opening bracket still shows up in libass and
standard renders will merely display a backslash in its place
instead of stripping the following text like before.
---
 libavcodec/ass.c          | 12 ++++++++----
 libavcodec/webvttdec.c    |  2 +-
 tests/ref/fate/sub-webvtt |  2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index a68d3568b4..e7a1ac0eb5 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -181,10 +181,14 @@ void ff_ass_bprint_text_event(AVBPrint *buf, const char *p, int size,
         if (linebreaks && strchr(linebreaks, *p)) {
             av_bprintf(buf, "\\N");
 
-        /* standard ASS escaping so random characters don't get mis-interpreted
-         * as ASS */
-        } else if (!keep_ass_markup && strchr("{}", *p)) {
-            av_bprintf(buf, "\\%c", *p);
+        /* cancel curly brackets to avoid bogus override tag blocks
+         * hiding text. Standard ASS has no character escapes,
+         * though (only) libass provides \{ and \}.
+         * Unpaired closing brackets don't need escaping at all though and
+         * to make the situation less bad in standard ASS insert an empty block
+         */
+        } else if (!keep_ass_markup && *p == '{') {
+            av_bprintf(buf, "\\{{}");
 
         /* append word-joiner U+2060 as UTF-8 to break up sequences like \N */
         } else if (!keep_ass_markup && *p == '\\') {
diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
index 6e55bc5499..35bdbe805d 100644
--- a/libavcodec/webvttdec.c
+++ b/libavcodec/webvttdec.c
@@ -37,7 +37,7 @@ static const struct {
     {"<i>", "{\\i1}"}, {"</i>", "{\\i0}"},
     {"<b>", "{\\b1}"}, {"</b>", "{\\b0}"},
     {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
-    {"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
+    {"{", "\\{{}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
     {"&gt;", ">"}, {"&lt;", "<"},
     {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
     {"&amp;", "&"}, {"&nbsp;", "\\h"},
diff --git a/tests/ref/fate/sub-webvtt b/tests/ref/fate/sub-webvtt
index ea587b327c..fae50607fb 100644
--- a/tests/ref/fate/sub-webvtt
+++ b/tests/ref/fate/sub-webvtt
@@ -21,7 +21,7 @@ Dialogue: 0,0:00:22.00,0:00:24.00,Default,,0,0,0,,at the AMNH.
 Dialogue: 0,0:00:24.00,0:00:26.00,Default,,0,0,0,,Thank you for walking down here.
 Dialogue: 0,0:00:27.00,0:00:30.00,Default,,0,0,0,,And I want to do a follow-up on the last conversation we did.\Nmultiple lines\Nagain
 Dialogue: 0,0:00:30.00,0:00:31.50,Default,,0,0,0,,When we e-mailed—
-Dialogue: 0,0:00:30.50,0:00:32.50,Default,,0,0,0,,Didn't we {\b1}talk {\i1}about\N{\i0} enough{\b0} in that conversation? \{I'm not an ASS comment\}
+Dialogue: 0,0:00:30.50,0:00:32.50,Default,,0,0,0,,Didn't we {\b1}talk {\i1}about\N{\i0} enough{\b0} in that conversation? \{{}I'm not an ASS comment}
 Dialogue: 0,0:00:32.00,0:00:35.50,Default,,0,0,0,,No! No no no no; 'cos 'cos obviously 'cos
 Dialogue: 0,0:00:32.50,0:00:33.50,Default,,0,0,0,,{\i1}Laughs{\i0}
 Dialogue: 0,0:00:35.50,0:00:38.00,Default,,0,0,0,,You know I'm so excited my glasses are falling off here.
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 3/4] avcodec/{ass, webvttdec}: more portable curly brace escapes

[Stefano Sabatini]

On date Monday 2024-02-19 22:42:26 +0100, Oneric wrote:
> Unlike what the old comment suggested, standard ASS has no character
> escape mechanism, but a closing curly bracket doesn't even need one.
> 

> For manual authored sub files using a full-width variant of an apropiate

appropriate?

> font and with scaling and psacing modifiers is a common workaround.

spacing

> This is not an option here, but we can still make things much less bad.
> Now the desired opening bracket still shows up in libass and
> standard renders will merely display a backslash in its place
> instead of stripping the following text like before.
> ---
>  libavcodec/ass.c          | 12 ++++++++----
>  libavcodec/webvttdec.c    |  2 +-
>  tests/ref/fate/sub-webvtt |  2 +-
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/ass.c b/libavcodec/ass.c
> index a68d3568b4..e7a1ac0eb5 100644
> --- a/libavcodec/ass.c
> +++ b/libavcodec/ass.c
> @@ -181,10 +181,14 @@ void ff_ass_bprint_text_event(AVBPrint *buf, const char *p, int size,
>          if (linebreaks && strchr(linebreaks, *p)) {
>              av_bprintf(buf, "\\N");
>  
> -        /* standard ASS escaping so random characters don't get mis-interpreted
> -         * as ASS */
> -        } else if (!keep_ass_markup && strchr("{}", *p)) {
> -            av_bprintf(buf, "\\%c", *p);
> +        /* cancel curly brackets to avoid bogus override tag blocks
> +         * hiding text. Standard ASS has no character escapes,
> +         * though (only) libass provides \{ and \}.
> +         * Unpaired closing brackets don't need escaping at all though and
> +         * to make the situation less bad in standard ASS insert an empty block
> +         */
> +        } else if (!keep_ass_markup && *p == '{') {
> +            av_bprintf(buf, "\\{{}");
>  
>          /* append word-joiner U+2060 as UTF-8 to break up sequences like \N */
>          } else if (!keep_ass_markup && *p == '\\') {
> diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
> index 6e55bc5499..35bdbe805d 100644
> --- a/libavcodec/webvttdec.c
> +++ b/libavcodec/webvttdec.c
> @@ -37,7 +37,7 @@ static const struct {
>      {"<i>", "{\\i1}"}, {"</i>", "{\\i0}"},
>      {"<b>", "{\\b1}"}, {"</b>", "{\\b0}"},
>      {"<u>", "{\\u1}"}, {"</u>", "{\\u0}"},
> -    {"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
> +    {"{", "\\{{}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup conflicts
>      {"&gt;", ">"}, {"&lt;", "<"},
>      {"&lrm;", "\xe2\x80\x8e"}, {"&rlm;", "\xe2\x80\x8f"},
>      {"&amp;", "&"}, {"&nbsp;", "\\h"},
> diff --git a/tests/ref/fate/sub-webvtt b/tests/ref/fate/sub-webvtt
> index ea587b327c..fae50607fb 100644
> --- a/tests/ref/fate/sub-webvtt
> +++ b/tests/ref/fate/sub-webvtt
> @@ -21,7 +21,7 @@ Dialogue: 0,0:00:22.00,0:00:24.00,Default,,0,0,0,,at the AMNH.
>  Dialogue: 0,0:00:24.00,0:00:26.00,Default,,0,0,0,,Thank you for walking down here.
>  Dialogue: 0,0:00:27.00,0:00:30.00,Default,,0,0,0,,And I want to do a follow-up on the last conversation we did.\Nmultiple lines\Nagain
>  Dialogue: 0,0:00:30.00,0:00:31.50,Default,,0,0,0,,When we e-mailed—
> -Dialogue: 0,0:00:30.50,0:00:32.50,Default,,0,0,0,,Didn't we {\b1}talk {\i1}about\N{\i0} enough{\b0} in that conversation? \{I'm not an ASS comment\}
> +Dialogue: 0,0:00:30.50,0:00:32.50,Default,,0,0,0,,Didn't we {\b1}talk {\i1}about\N{\i0} enough{\b0} in that conversation? \{{}I'm not an ASS comment}
>  Dialogue: 0,0:00:32.00,0:00:35.50,Default,,0,0,0,,No! No no no no; 'cos 'cos obviously 'cos
>  Dialogue: 0,0:00:32.50,0:00:33.50,Default,,0,0,0,,{\i1}Laughs{\i0}
>  Dialogue: 0,0:00:35.50,0:00:38.00,Default,,0,0,0,,You know I'm so excited my glasses are falling off here.

Should be good otherwise (but a second look from someone more familiar
with ASS also might be good).
_______________________________________________
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".

[FFmpeg-devel] [PATCH v4 4/4] avocdec/ass: simplify linebreaks

[Oneric]

ff_ass_subtitle_header_* still used explicit CRLF linebreaks
eventhough they will get normalised to LF later since commit
7bf1b9b35769b37684dd2f18a54f01d852a540c8. Just directly use LF.
---
 libavcodec/ass.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index e7a1ac0eb5..927b801404 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -35,15 +35,15 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
                                 int border_style, int alignment)
 {
     avctx->subtitle_header = av_asprintf(
-             "[Script Info]\r\n"
-             "; Script generated by FFmpeg/Lavc%s\r\n"
-             "ScriptType: v4.00+\r\n"
-             "PlayResX: %d\r\n"
-             "PlayResY: %d\r\n"
-             "ScaledBorderAndShadow: yes\r\n"
-             "YCbCr Matrix: None\r\n"
-             "\r\n"
-             "[V4+ Styles]\r\n"
+             "[Script Info]\n"
+             "; Script generated by FFmpeg/Lavc%s\n"
+             "ScriptType: v4.00+\n"
+             "PlayResX: %d\n"
+             "PlayResY: %d\n"
+             "ScaledBorderAndShadow: yes\n"
+             "YCbCr Matrix: None\n"
+             "\n"
+             "[V4+ Styles]\n"
 
              /* ASS (v4+) header */
              "Format: Name, "
@@ -54,7 +54,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
              "Spacing, Angle, "
              "BorderStyle, Outline, Shadow, "
              "Alignment, MarginL, MarginR, MarginV, "
-             "Encoding\r\n"
+             "Encoding\n"
 
              "Style: "
              "Default,"             /* Name */
@@ -65,11 +65,11 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
              "0,0,"                 /* Spacing, Angle */
              "%d,1,0,"              /* BorderStyle, Outline, Shadow */
              "%d,10,10,10,"         /* Alignment, Margin[LRV] */
-             "1\r\n"                /* Encoding */
+             "1\n"                  /* Encoding */
 
-             "\r\n"
-             "[Events]\r\n"
-             "Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text\r\n",
+             "\n"
+             "[Events]\n"
+             "Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text\n",
              !(avctx->flags & AV_CODEC_FLAG_BITEXACT) ? AV_STRINGIFY(LIBAVCODEC_VERSION) : "",
              play_res_x, play_res_y, font, font_size,
              primary_color, secondary_color, outline_color, back_color,
-- 
2.39.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 4/4] avocdec/ass: simplify linebreaks

[Stefano Sabatini]

On date Monday 2024-02-19 22:42:27 +0100, Oneric wrote:
> ff_ass_subtitle_header_* still used explicit CRLF linebreaks
> eventhough they will get normalised to LF later since commit
> 7bf1b9b35769b37684dd2f18a54f01d852a540c8. Just directly use LF.
> ---
>  libavcodec/ass.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

Looks good to me, 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".

Re: [FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Oneric]

[-- Attachment #1.1: Type: text/plain, Size: 1693 bytes --]

ping

I forgot to again add a description to v4 so:
These small patches fix several mis-conversions for subtitles
(and additionally one cde cleanup in avcodec/ass.c):

 - WebVTT files no longer get their BiDi marks stripped
   (which mangled BiDi text)
 - backslashes are no longer duplicated for no reason
 - ffmpeg’s attempt at escaping curly braces no longer
   severly breaks in standard ASS renderers

v1 was posted over 2 years ago, would be great
if someone could finally look at this


On Mon, Feb 19, 2024 at 22:42:23 +0100, Oneric wrote:
> Changes from v3:
>  - None.  Just rebased ontop of master to allow
>    Patchwork to properly process the series now
>    that 99d33cc661fbd04e8657831 was merged.
> 
> Changes from v2:
>  - rebased ontop of the recently pushed eol normalisation.
>    As a result no more CRLFs in here and Patchwork should be happy
>  - added a fourth cosmetic commit adjusting
>    explicit linebreaks to the new normalisation
> 
> Changes from v1:
>  - ff_ass_bprint_text_event now only inserts a word-joiner
>    if there isn’t already one anyway
>  - added a third commit improving the handling of
>    curly brackets for standard ASS renderers
> 
> Oneric (4):
>   avcodec/webvttdec: honour bidi marks
>   avcodec/{ass,webvttdec}: fix handling of backslashes
>   avcodec/{ass,webvttdec}: more portable curly brace escapes
>   avocdec/ass: simplify linebreaks
> 
>  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
>  libavcodec/webvttdec.c     |  4 ++--
>  tests/ref/fate/sub-webvtt  |  2 +-
>  tests/ref/fate/sub-webvtt2 |  2 +-
>  4 files changed, 33 insertions(+), 22 deletions(-)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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".

Re: [FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Marth64]

Hi Oneric,

I was able to validate the whole set. Here are the tests I did ...
(1) test conversion from vtt->ass with bidi mark and visually check display
-- pass
(2) test slash escape fix with a string e.g. \\Nancy, verify broken before
and fixed by patch -- pass
(3) test curly brace escape fix doesn't break SRT titles with ass
positioning tags -- pass
(4) compare sha256sum before/after results for line breaks change -- pass

LGTM from a validation perspective on a Linux machine.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Marth64]

Ping on this set authored by Oneric 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".

Re: [FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Oneric]

[-- Attachment #1.1: Type: text/plain, Size: 593 bytes --]

On Mon, 19 Feb 2024 Oneric wrote:
>   avcodec/webvttdec: honour bidi marks
>   avcodec/{ass,webvttdec}: fix handling of backslashes
>   avcodec/{ass,webvttdec}: more portable curly brace escapes
>   avocdec/ass: simplify linebreaks
>
>  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
>  libavcodec/webvttdec.c     |  4 ++--
>  tests/ref/fate/sub-webvtt  |  2 +-
>  tests/ref/fate/sub-webvtt2 |  2 +-
>  4 files changed, 33 insertions(+), 22 deletions(-)

On Tue, Mar 26, 2024 at 11:01:40 -0500, Marth64 wrote:
> Ping on this set authored by Oneric as well.

ping again

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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".

Re: [FFmpeg-devel] [PATCH v4 0/4] Fix some active sequences in subtitles

[Stefano Sabatini]

On date Wednesday 2024-04-03 21:50:29 +0200, Oneric wrote:
> On Mon, 19 Feb 2024 Oneric wrote:
> >   avcodec/webvttdec: honour bidi marks
> >   avcodec/{ass,webvttdec}: fix handling of backslashes
> >   avcodec/{ass,webvttdec}: more portable curly brace escapes
> >   avocdec/ass: simplify linebreaks
> >
> >  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
> >  libavcodec/webvttdec.c     |  4 ++--
> >  tests/ref/fate/sub-webvtt  |  2 +-
> >  tests/ref/fate/sub-webvtt2 |  2 +-
> >  4 files changed, 33 insertions(+), 22 deletions(-)
> 
> On Tue, Mar 26, 2024 at 11:01:40 -0500, Marth64 wrote:
> > Ping on this set authored by Oneric as well.
> 
> ping again

will apply this soon, 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".