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

2024-04-04 Thread Oneric
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.


signature.asc
Description: PGP signature
___
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 0/4] Fix some active sequences in subtitles

2024-04-03 Thread Oneric
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


signature.asc
Description: PGP signature
___
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 0/4] Fix some active sequences in subtitles

2024-02-26 Thread Oneric
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(-)


signature.asc
Description: PGP signature
___
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 4/4] avocdec/ass: simplify linebreaks

2024-02-19 Thread 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-19 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
+{"{", "\\{{}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup 
conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-19 Thread 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-19 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
+{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-19 Thread 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 {
 {"", "{\\u1}"}, {"", "{\\u0}"},
 {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
 {"", ">"}, {"", "<"},
-{"", ""}, {"", ""}, // FIXME: properly honor bidi marks
+{"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-11 Thread Oneric
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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-11 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
+{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-11 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
+{"{", "\\{{}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup 
conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-11 Thread 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 {
 {"", "{\\u1}"}, {"", "{\\u0}"},
 {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
 {"", ">"}, {"", "<"},
-{"", ""}, {"", ""}, // FIXME: properly honor bidi marks
+{"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-02-11 Thread 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2024-01-09 Thread Oneric
On Thu, Dec 28, 2023 at 01:55:02 +0100, Oneric wrote:
> On Sun, Dec 10, 2023 at 17:37:12 +0100, Oneric wrote:
> > 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
> > 
> 
> ping

ping #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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-12-27 Thread Oneric
On Sun, Dec 10, 2023 at 17:37:12 +0100, Oneric wrote:
> 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
> 
> Heads up:
> [...]

ping

(applying from the received mails works fine,
 but do not use Patchwork’s mbox/diff/series as Patchwork mangled them)
___
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 v2 3/3] avcodec/{ass, webvttdec}: more portable curly brace escapes

2023-12-10 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
+{"{", "\\{{}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid ASS markup 
conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\h"},
diff --git a/tests/ref/fate/sub-webvtt b/tests/ref/fate/sub-webvtt
index 2317c7d5a0..7f0a306361 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-requ...@ffmpeg.org with subject "unsubscribe".


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

2023-12-10 Thread 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 {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
+{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
 {"", ">"}, {"", "<"},
 {"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 1/3] avcodec/webvttdec: honour bidi marks

2023-12-10 Thread 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 {
 {"", "{\\u1}"}, {"", "{\\u0}"},
 {"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
 {"", ">"}, {"", "<"},
-{"", ""}, {"", ""}, // FIXME: properly honor bidi marks
+{"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\h"},
 };
 
diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
index 1d236eabdc..31fb5f83a7 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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 0/3] Fix some active sequences in subtitles

2023-12-10 Thread Oneric
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

Heads up:
As everytime FATE’s ASS reference samples are touched the commits
contain CRLF endings because the reference files do.
Unless it has finally been fixed, patchwork doesn’t like this and will
mangle and falsely fail to apply the patches and this particular mailing list
has also in the past been eager to rewrite the mail body, mangling the encoding
and destroying CRLF line endings in the process.
I took the usual measures to protect against the latter (explicity using base64
transfer encoding and including non-ASCII characters in the body), but if this
now also doesn't work anymore, I’ll just reply to this mail with a link to
the patches uploaded somewhere else.
(And send patches to get rid of CRLF line endings)


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

 libavcodec/ass.c   | 19 +++
 libavcodec/webvttdec.c |  4 ++--
 tests/ref/fate/sub-webvtt  |  2 +-
 tests/ref/fate/sub-webvtt2 |  2 +-
 4 files changed, 19 insertions(+), 8 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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v3] avfilter/vf_subtitles: add wrap_unicode option

2023-05-22 Thread Oneric
On Mon, May 22, 2023 at 21:26:02 +0800, Zhao Zhili wrote:
> From: Zhao Zhili 
> 
> So CJK can be wrapped automatically.
> 
> Signed-off-by: Zhao Zhili 
> ---
> v3: Fix build error with older libass
> v2: Don't overwrite wrap automatically for native ASS
>  libavfilter/version.h  |  2 +-
>  libavfilter/vf_subtitles.c | 18 ++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index ba8a6fdab2..08779130f5 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -32,7 +32,7 @@
>  #include "version_major.h"
>  
>  #define LIBAVFILTER_VERSION_MINOR   8
> -#define LIBAVFILTER_VERSION_MICRO 100
> +#define LIBAVFILTER_VERSION_MICRO 101
>  
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> index 82e140e986..3e5a19e715 100644
> --- a/libavfilter/vf_subtitles.c
> +++ b/libavfilter/vf_subtitles.c
> @@ -45,6 +45,8 @@
>  #include "formats.h"
>  #include "video.h"
>  
> +#define FF_ASS_FEATURE_WRAP_UNICODE (LIBASS_VERSION >= 0x01600010)
> +
>  typedef struct AssContext {
>  const AVClass *class;
>  ASS_Library  *library;
> @@ -61,6 +63,7 @@ typedef struct AssContext {
>  int original_w, original_h;
>  int shaping;
>  FFDrawContext draw;
> +int wrap_unicode;
>  } AssContext;
>  
>  #define OFFSET(x) offsetof(AssContext, x)
> @@ -271,6 +274,9 @@ static const AVOption subtitles_options[] = {
>  {"stream_index", "set stream index", OFFSET(stream_index), 
> AV_OPT_TYPE_INT,{ .i64 = -1 }, -1,   INT_MAX,  FLAGS},
>  {"si",   "set stream index", OFFSET(stream_index), 
> AV_OPT_TYPE_INT,{ .i64 = -1 }, -1,   INT_MAX,  FLAGS},
>  {"force_style",  "force subtitle style", OFFSET(force_style),  
> AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
> +#if FF_ASS_FEATURE_WRAP_UNICODE
> +{"wrap_unicode", "break lines according to the Unicode Line Breaking 
> Algorithm", OFFSET(wrap_unicode), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, 
> FLAGS },
> +#endif
>  {NULL},
>  };
>  
> @@ -432,6 +438,18 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
>  if (ret < 0)
>  goto end;
>  
> +#if FF_ASS_FEATURE_WRAP_UNICODE
> +/* Don't overwrite wrap automatically for native ASS */
> +if (ass->wrap_unicode == -1)
> +ass->wrap_unicode = st->codecpar->codec_id != AV_CODEC_ID_ASS;
> +if (ass->wrap_unicode) {
> +ret = ass_track_set_feature(ass->track, ASS_FEATURE_WRAP_UNICODE, 1);
> +if (ret < 0)
> +av_log(ctx, AV_LOG_WARNING,
> +   "libass wasn't built with ASS_FEATURE_WRAP_UNICODE 
> support\n");
> +}
> +#endif
> +
>  if (ass->force_style) {
>  char **list = NULL;
>  char *temp = NULL;
> -- 
> 2.34.1

The new option and its build-time requirement should probably also be
documented in doc/filters.texi. Other than that v3 looks good to me.


signature.asc
Description: PGP signature
___
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] avfilter/vf_subtitles: add wrap_unicode option

2023-05-19 Thread Oneric
On Fri, May 19, 2023 at 23:58:59 +0800, Zhao Zhili wrote:
> From: Zhao Zhili 
> 
> So CJK can be wrapped automatically.
> 
> Signed-off-by: Zhao Zhili 
> ---
>  libavfilter/version.h  |  2 +-
>  libavfilter/vf_subtitles.c | 13 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index ba8a6fdab2..08779130f5 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -32,7 +32,7 @@
>  #include "version_major.h"
>  
>  #define LIBAVFILTER_VERSION_MINOR   8
> -#define LIBAVFILTER_VERSION_MICRO 100
> +#define LIBAVFILTER_VERSION_MICRO 101
>  
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> index 82e140e986..d6a68e5cdd 100644
> --- a/libavfilter/vf_subtitles.c
> +++ b/libavfilter/vf_subtitles.c
> @@ -61,6 +61,7 @@ typedef struct AssContext {
>  int original_w, original_h;
>  int shaping;
>  FFDrawContext draw;
> +int wrap_unicode;
>  } AssContext;
>  
>  #define OFFSET(x) offsetof(AssContext, x)
> @@ -72,6 +73,7 @@ typedef struct AssContext {
>  {"original_size",  "set the size of the original video (used to scale 
> fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},  0, 0, 
> FLAGS }, \
>  {"fontsdir",   "set the directory containing the fonts to read", 
>   OFFSET(fontsdir),   AV_OPT_TYPE_STRING, {.str = NULL},  0, 0, FLAGS 
> }, \
>  {"alpha",  "enable processing of alpha channel", 
>   OFFSET(alpha),  AV_OPT_TYPE_BOOL,   {.i64 = 0   }, 0,   
>  1, FLAGS }, \
> +{"wrap_unicode",   "break lines according to the Unicode Line Breaking 
> Algorithm", OFFSET(wrap_unicode), AV_OPT_TYPE_BOOL, {.i64 = 1   }, 0, 
>1, FLAGS }, \

This feature MUST NOT uncondtionally be enabled by default.
Its documentation clearly warns that it deviates from proper ASS and
is incompatible with VSFilter (the authorative ASS reference
implementation).
Even more, it should imho NEVER be enabled when the input is native ASS.
Existing ASS subtitles and authors of ASS subs (unfortunately) rely on the
existing auto-linebwrapping semantic. For writing systems not working well
with those semantics, authors of native ASS subs already manually break
lines at the appropiate places and enabling this feautre may make the
situation worse, if some lines actually overflow their supposed width.
With the established ASS breaking rules overflowing CJK lines without
spaces will not break on overflow. If they now did, you’d get a
auto-linebreak followed by a manual linebreak one or two characters later.

If subs are not native ASS and only get converted to ASS to be able to
render it with libass (like ffmpeg does), then enabling it is fine.
(Besides converted subs, some programs internally also generate ASS-like
subtitles on-the-fly and pass them to libass, with the resulting subs
never hitting the disk and being distributed, so compatibility is not a
concern. Those programs also may use this feature)

>  
>  /* libass supports a log level ranging from 0 to 7 */
>  static const int ass_libavfilter_log_level_map[] = {
> @@ -329,6 +331,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
>  return AVERROR(EINVAL);
>  }
>  
> +if (ass->wrap_unicode) {
> +#if (LIBASS_VERSION >= 0x01600010)
> +ret = ass_track_set_feature(ass->track, ASS_FEATURE_WRAP_UNICODE, 1);
> +if (ret < 0)
> +av_log(ctx, AV_LOG_WARNING, "libass doesn't build with 
> ASS_FEATURE_WRAP_UNICODE support\n");

Nit: "libass wasn't built with libunibreak support" would imho be clearer.

> +#else
> +av_log(ctx, AV_LOG_INFO, "libass version %#x doesn't support 
> ASS_FEATURE_WRAP_UNICODE\n",
> +   LIBASS_VERSION);

Nit: This will print the version of libass ffmpeg was built against. This may
be confusing if later on the libass library was upgraded without needing a
rebuild and the new version _does_ support WRAP_UNCIDOE. I’d suggest
something along the lines of
  "ffmpeg was built against libass version %#x not supporting 
ASS_FEATURE_WRAP_UNICODE"

> +#endif
> +}
> +
>  /* Open subtitles file */
>  ret = avformat_open_input(, ass->filename, NULL, NULL);
>  if (ret < 0) {
> -- 


On Fri, May 19, 2023 at 22:14:54 +0800, Lance Wang wrote:
>
> Also, if the version is right, then why to check the return?
> +#if (LIBASS_VERSION >= 0x01600010)
> +ass_track_set_feature(ass->track, ASS_FEATURE_WRAP_UNICODE, 1);
> +#endif

libass API provides WRAP_UNICODE since 0x01600010,
but at runtime the feature is only available if libass
was built against its optional dependency libunibreak.
If not attempting to enable the feature will have
no effect and ass_track_set_feature returns -1.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing 

Re: [FFmpeg-devel] [PATCH v2 0/3] Some small ASS conversion fixes

2022-11-30 Thread Oneric
On Sun, Nov 20, 2022 at 01:15:33 +0100, Oneric wrote:
> On Sun, Nov 13, 2022 at 20:57:15 +0100, Oneric wrote:
> > This fixes colours and font selection for files converted to ASS
> 
> Ping.
> 
> As a reminder v1 and v2 are equivalent and patchwork is buggy and can
> thus neither apply v1 or v2 and its results can be ignored and its patch
> or diff downloads should not be used, because they are corrupted.
> 
> v1 can be applied from the mails received from the list, v2 can be applied
> the same way for patch one and two, but you may need to use --keep-cr and
> because it turns out not only patchwork but also the mailinglist software
> mangles the transfer encoding, patch three of v2 cannot applied from the
> Mail received by the list.
> As an alternative I previously attached an archive (so the list won’t
> mangle it again) containing the three format-patch files, see:
>   https://ffmpeg.org/pipermail/ffmpeg-devel/2022-November/303904.html
> 
> As before, if you’d prefer to get a v3 where all mails can be applied from
> the list directly and no binary diff is used, tell me and I’ll send a v3
> forcing base64-encoding from git send-email and including some non-ASCII
> characters in the patch comment, since this appeared to make the list keep
> the base64 transfer-encoding for patch 2 v2.

Ping #2


signature.asc
Description: PGP signature
___
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 v2 0/3] Some small ASS conversion fixes

2022-11-19 Thread Oneric
On Sun, Nov 13, 2022 at 20:57:15 +0100, Oneric wrote:
> This fixes colours and font selection for files converted to ASS

Ping.

As a reminder v1 and v2 are equivalent and patchwork is buggy and can
thus neither apply v1 or v2 and its results can be ignored and its patch
or diff downloads should not be used, because they are corrupted.

v1 can be applied from the mails received from the list, v2 can be applied
the same way for patch one and two, but you may need to use --keep-cr and
because it turns out not only patchwork but also the mailinglist software
mangles the transfer encoding, patch three of v2 cannot applied from the
Mail received by the list.
As an alternative I previously attached an archive (so the list won’t
mangle it again) containing the three format-patch files, see:
  https://ffmpeg.org/pipermail/ffmpeg-devel/2022-November/303904.html

As before, if you’d prefer to get a v3 where all mails can be applied from
the list directly and no binary diff is used, tell me and I’ll send a v3
forcing base64-encoding from git send-email and including some non-ASCII
characters in the patch comment, since this appeared to make the list keep
the base64 transfer-encoding for patch 2 v2.


signature.asc
Description: PGP signature
___
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 v2 0/3] Some small ASS conversion fixes

2022-11-14 Thread Oneric
On Sun, Nov 13, 2022 at 22:57:22 +0100, Oneric wrote:
> On Sun, Nov 13, 2022 at 21:01:34 +, Soft Works wrote:
> > I am unable to apply any of those patches. Neither v1 nor v2 neither
> > when downloading the series mbox or [2/3] from patchwork nor when
> > saving [2/3] from the e-mail.
> 
> Re patchwork, yes I rechecked and it appears all downloads are now broken
> (see prev message). Regarding applying the mails, all I can say is it
> works for me.

I now know why the mbox files from patchwork don’t work for v1.
For whatever reason patchwork thought it a good idea to reorder the binary
diffs and while doing so separated diff headers from diff content so
nothing makes sense to git anymore. This gives a plausible explanation for
the loongarch error message, but now I’m not sure how the x86 FATE box
manages to apply anything (but whatever it does apply is probably
corrupted explaining the test failures).

The mails received from the list — apart from v3-patch3 — or the archive
with format-patches I sent, ofc still work.

For illustration here’s a truncated bit of the difference between the
received mail after base64 decoding and what patchwork offers as a
download for v1 patch number 2:

diff --git a/tmp/0002-from-mail_v1__decoded_data.patch 
b/tmp/FFmpeg-devel-2-3-avcodec-ass-accurately-preserve-colours
.patch
index b2a7a93d7a..4691059715 100644
--- a/tmp/0002-from-mail_v1__decoded_data-.patch
+++ b/tmp/FFmpeg-devel-2-3-avcodec-ass-accurately-preserve-colours.patch
@@ -44,6 +154,95 @@ Link to libass’ docs regarding colour mangling:
  tests/ref/fate/sub-webvtt2   | Bin 1648 -> 1668 bytes
  25 files changed, 2 insertions(+)
 
+index 
13f393cc86b9f797be24f37a07aafc7272c22e59..516d26af9ab09613f939d004826e44c80b3b1054
 100644
+GIT binary patch
+delta 29
+kcmX@kcAITNgHWV%l5>%QZ(>PNW`#=4VC0HTfx{Qv*}
+
+delta 10
+Rcmcc3cARZO!^SDcnE)Fg1kC^d
+
+index 
be28084887a9b7fb80ae4ff3369cf7192a978919..a97d29f70ba1d3984887eacb6e833abb8a406902
 100644
+GIT binary patch
+delta 29
+kcmdnUew2MegHWV%l5>%QZ(>PNW`#yVqW0G!DRJ^%m!
+
+delta 10
+RcmX@gzL9-G!^SCw%m5g91P%ZI
+
[.. and a bunch more additions like this]
@@ -66,14 +265,6 @@ delta 10
 RcmZ1|*(*7rVdE5D9sm~71J3{e
 
 diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
-index 
13f393cc86b9f797be24f37a07aafc7272c22e59..516d26af9ab09613f939d004826e44c80b3b1054
 100644
-GIT binary patch
-delta 29
-kcmX@kcAITNgHWV%l5>%QZ(>PNW`#=4VC0HTfx{Qv*}
-
-delta 10
-Rcmcc3cARZO!^SDcnE)Fg1kC^d
-
 diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
 index 
169361f540e4dff43a132df395e55a19ff73..98dfef55019719911d6c5d9faa0c057cc324f227
 100644
 GIT binary patch
@@ -84,14 +275,6 @@ delta 10
 RcmeC+`NBP+VdIn%Rsb0Q1Y!UH
 
 diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
-index 
be28084887a9b7fb80ae4ff3369cf7192a978919..a97d29f70ba1d3984887eacb6e833abb8a406902
 100644
-GIT binary patch
-delta 29
-kcmdnUew2MegHWV%l5>%QZ(>PNW`#yVqW0G!DRJ^%m!
-
-delta 10
-RcmX@gzL9-G!^SCw%m5g91P%ZI
-
 diff --git a/tests/ref/fate/sub-charenc b/tests/ref/fate/sub-charenc
 index 
4efacb073d14b43002b58ab0b4aa55e9d34de029..339137ae0b5485c4c954f859316cf8b413b01505
 100644
 GIT binary patch

[.. and more]


signature.asc
Description: PGP signature
___
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 v2 3/3] avcodec/ass: specify a permissive encoding

2022-11-13 Thread Oneric
On Sun, Nov 13, 2022 at 21:15:04 +0100, Oneric wrote:
> To apply the third patch you may use v1 or the attached patch file.

The list also stripped the CRLF line endings from the (quoted-printable
encoded) attachments :(
Also no patchwork download has unmangled line endings anymore.

All v1 patches can be applied from the mail client, patch one and two of
v2 too. In case someone can't directly apply mails, I attached all
three v2 patches again bundled in an archive in another message of this
thread to stop list shenanigans. The list archive also has a download link
for the *.tar.xz archive:
  https://ffmpeg.org/pipermail/ffmpeg-devel/2022-November/303904.html
  Expected SHA256: 
8e3968f0a342c30928f8ab889efdf3bf1eb65e3cd157b42c82fb3d205815658b

When applying, make sure to use git am --keep-cr.

Regards
Oneric


signature.asc
Description: PGP signature
___
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 v2 0/3] Some small ASS conversion fixes

2022-11-13 Thread Oneric
On Sun, Nov 13, 2022 at 21:01:34 +, Soft Works wrote:
> I am unable to apply any of those patches. Neither v1 nor v2 neither
> when downloading the series mbox or [2/3] from patchwork nor when
> saving [2/3] from the e-mail.

Re patchwork, yes I rechecked and it appears all downloads are now broken
(see prev message). Regarding applying the mails, all I can say is it
works for me.
After a fresh clone to /tmp/ffmpeg, I can successfully apply v1 1/3, v1 2/3
and v1 3/3 from my mail client via pipe-menu and `git -C /tmp/ffmpeg am -`.
For v2 the first two patches also work.

> Did you notice that FATE has failed, even with the v1 patches?

While checking the current mbox download, I noticed yeah.
The error for loongarch doesn't make sense to me "could not build fake
ancestor" and the x86 runner manages to apply it, so this seems like a
false positive.

For x86_make_fate the short errors snippet that’s available only shows
tests failing which work for me. To make sure, I copied my old fate-suite
dir to the fresh clone, configured with
  --enable-libass --enable-libx264 --enable-libopus
  --enable-gpl --disable-shared --enable-static --samples=fate-suite/
and then ran `make fate-rsync`, `make` and `make fate`
with an appropriate job count for:

 - v2 1+2
 - v2 1+2 followed by v1 3
 - v1 1+2+3

all applied from the received mails and all succeeded.
I’d guess the fate runner uses --no-keep-cr, but then applying should
already fail. Perhaps there’s a similar option only "normalising" added
lines?

Checking the format-patch for v2 3/3 I attached earlier (which also only
ended up as quoted-printable not base64), shows the list again stripped
CRLF just like for the initial patch :(
It appears this list’s configuration is particular bad for CRLF-requiring
patches. (But v1 or forcing base64 should still work)

To make applying easier for you, I now attached all patches bundled as an
archive to keep the list from interfering. Make sure to use --keep-cr.

All tests were done on top of 2d25f33a7ed36003de9a62c9cb165db183663d1c.

Regards
Oneric


patches_ffmpeg_ass-fixes_20221112.tar.xz
Description: application/xz


signature.asc
Description: PGP signature
___
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 v2 0/3] Some small ASS conversion fixes

2022-11-13 Thread Oneric
On Sun, Nov 13, 2022 at 21:27:11 +0100, Oneric wrote:
> [...] You can easily verify emails are capable of it by
> applying patch 2 yourself or using patchwork’s mbox file.

A correction: it seems since last time we discussed this, some patchwork
update has expanded the line-mangling to also affect "mbox" and "series"
downloads. (Or I’m misremembering and no patchwork download ever worked)
So to apply v2 patch 2/3, using the received mail itself is the only
option. (Applying v1 patch 2/3 also does the job)

Oneric


signature.asc
Description: PGP signature
___
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 v2 0/3] Some small ASS conversion fixes

2022-11-13 Thread Oneric
On Sun, Nov 13, 2022 at 20:15:23 +, Soft Works wrote:
> 
> [...] it's not a Patchwork bug and it cannot be "fixed"
> at all in any other way than using binary diffs. The problem lies in 
> the fact that it's being sent by e-mail where the different line
> endings get lost (as the spec mandates CRLF and doesn't allow LF only).

As last time: No, emails are perfectly capable of preserving CRLF line
endings via e.g. "quoted-printable" or "base64" transfer-encoding and
that’s what git send-email automatically uses.

As a case in point of it not being "just" patchwork, there’s the
troubles with patch 3 of this series already as sent out by the list.
This is most likely to blame on the list itself and its message-editing to
add a footer though, and not on git send-email or email in general.

> Caveats when testing: Your local generated .patch files still work
> and depending on your e-mail client, even the patch in your sent items
> folder might not work. But as soon as it's been sent via SMTP the
> different line ending get lost.

And again like last, time I do ofc check the sent emails and not
local patch files. You can easily verify emails are capable of it by
applying patch 2 yourself or using patchwork’s mbox file.

Please actually focus on the patch this time. I have no need for repeating
the same long tangent about email and transfer encodings as last time, so
I’ll ignore all further messages from you not about the code changes.


signature.asc
Description: PGP signature
___
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 v2 3/3] avcodec/ass: specify a permissive encoding

2022-11-13 Thread Oneric
For some reason this message for the third patch was sent out as plain
7-bit us-ASCII, stripping the CRLF line endings.
Using the very same git send-email command line except for an added
--to= for sending it directly to myself, yields the expected
"printed-quotable" encoded message preserving the CRLF line endings.
Perhaps the list mangled it when adding its footer?

Either way, the second patch got through correctly with base64 encoding
and there areno problems with the first patch. To apply the third patch
you may use v1 or the attached patch file. If you want to instead resend
a v3 for all patches, let me know.
For the future I configured git send-email to always use base64 no matter
the content hopefully stopping the list from mangling my patches.

Oneric
From 73cacae6cb88aad4eb90c3b2b101ac1c5a6e2f69 Mon Sep 17 00:00:00 2001
From: Oneric 
Date: Sat, 12 Nov 2022 18:59:04 +0100
Subject: [PATCH] avcodec/ass: specify a permissive encoding

The Encoding field (and the \fe tag) allows to limit font selection to
only those fonts declaring support for the specified codepage in their
OS/2's table "Code Page Character Range" field.
Particularly, Encoding=0 means only font's declaring support for "ANSI",
or rather "Latin (Western European)", are allowed to be selected.
Specifying Encoding=1 allows all fonts to be considered.
We do not want to limit font selection, so specify Encoding=1.

NB: at the time of writing libass only partially supports this field,
thus hiding the issue in any libass-based renderer. A VSFilter-based
DirectShow filter or XySubFilter will reveal the issue when a font not
declaring support for latin characters is specified in a style.
---
 libavcodec/ass.c | 2 +-
 tests/ref/fate/sub-aqtitle   | 2 +-
 tests/ref/fate/sub-cc| 2 +-
 tests/ref/fate/sub-cc-realtime   | 2 +-
 tests/ref/fate/sub-cc-scte20 | 2 +-
 tests/ref/fate/sub-charenc   | 2 +-
 tests/ref/fate/sub-jacosub   | 2 +-
 tests/ref/fate/sub-microdvd  | 2 +-
 tests/ref/fate/sub-movtext   | 2 +-
 tests/ref/fate/sub-mpl2  | 2 +-
 tests/ref/fate/sub-mpsub | 2 +-
 tests/ref/fate/sub-mpsub-frames  | 2 +-
 tests/ref/fate/sub-pjs   | 2 +-
 tests/ref/fate/sub-realtext  | 2 +-
 tests/ref/fate/sub-sami  | 2 +-
 tests/ref/fate/sub-sami2 | 2 +-
 tests/ref/fate/sub-scc   | 2 +-
 tests/ref/fate/sub-srt   | 2 +-
 tests/ref/fate/sub-srt-badsyntax | 2 +-
 tests/ref/fate/sub-stl   | 2 +-
 tests/ref/fate/sub-subviewer | 2 +-
 tests/ref/fate/sub-subviewer1| 2 +-
 tests/ref/fate/sub-vplayer   | 2 +-
 tests/ref/fate/sub-webvtt| 2 +-
 tests/ref/fate/sub-webvtt2   | 2 +-
 25 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index d2ea4c62c3..5058dc8337 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -65,7 +65,7 @@ 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] */
- "0\r\n"/* Encoding */
+ "1\r\n"/* Encoding */
 
  "\r\n"
  "[Events]\r\n"
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index ae5edcd9ab..2980d39427 100644
--- a/tests/ref/fate/sub-aqtitle
+++ b/tests/ref/fate/sub-aqtitle
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,0
+Style: Default,Arial,16,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,1
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 516d26af9a..5e4f5ff7fc 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,0
+Style: Default,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,1
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 98dfef5501..22f09d5cdd 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -8,

[FFmpeg-devel] [PATCH v2 3/3] avcodec/ass: specify a permissive encoding

2022-11-13 Thread Oneric
The Encoding field (and the \fe tag) allows to limit font selection to
only those fonts declaring support for the specified codepage in their
OS/2's table "Code Page Character Range" field.
Particularly, Encoding=0 means only font's declaring support for "ANSI",
or rather "Latin (Western European)", are allowed to be selected.
Specifying Encoding=1 allows all fonts to be considered.
We do not want to limit font selection, so specify Encoding=1.

NB: at the time of writing libass only partially supports this field,
thus hiding the issue in any libass-based renderer. A VSFilter-based
DirectShow filter or XySubFilter will reveal the issue when a font not
declaring support for latin characters is specified in a style.
---
 libavcodec/ass.c | 2 +-
 tests/ref/fate/sub-aqtitle   | 2 +-
 tests/ref/fate/sub-cc| 2 +-
 tests/ref/fate/sub-cc-realtime   | 2 +-
 tests/ref/fate/sub-cc-scte20 | 2 +-
 tests/ref/fate/sub-charenc   | 2 +-
 tests/ref/fate/sub-jacosub   | 2 +-
 tests/ref/fate/sub-microdvd  | 2 +-
 tests/ref/fate/sub-movtext   | 2 +-
 tests/ref/fate/sub-mpl2  | 2 +-
 tests/ref/fate/sub-mpsub | 2 +-
 tests/ref/fate/sub-mpsub-frames  | 2 +-
 tests/ref/fate/sub-pjs   | 2 +-
 tests/ref/fate/sub-realtext  | 2 +-
 tests/ref/fate/sub-sami  | 2 +-
 tests/ref/fate/sub-sami2 | 2 +-
 tests/ref/fate/sub-scc   | 2 +-
 tests/ref/fate/sub-srt   | 2 +-
 tests/ref/fate/sub-srt-badsyntax | 2 +-
 tests/ref/fate/sub-stl   | 2 +-
 tests/ref/fate/sub-subviewer | 2 +-
 tests/ref/fate/sub-subviewer1| 2 +-
 tests/ref/fate/sub-vplayer   | 2 +-
 tests/ref/fate/sub-webvtt| 2 +-
 tests/ref/fate/sub-webvtt2   | 2 +-
 25 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index d2ea4c62c3..5058dc8337 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -65,7 +65,7 @@ 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] */
- "0\r\n"/* Encoding */
+ "1\r\n"/* Encoding */
 
  "\r\n"
  "[Events]\r\n"
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index ae5edcd9ab..2980d39427 100644
--- a/tests/ref/fate/sub-aqtitle
+++ b/tests/ref/fate/sub-aqtitle
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,0
+Style: 
Default,Arial,16,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,1
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 516d26af9a..5e4f5ff7fc 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,0
+Style: 
Default,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,1
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 98dfef5501..22f09d5cdd 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,0
+Style: 
Default,Monospace,16,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,1
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index a97d29f70b..cded979cb8 100644
--- a/tests/ref/fate/sub-cc-scte20
+++ b/tests/ref/fate/sub-cc-scte20
@@ -8,7 +8,7 @@ YCbCr Matrix: None
 
 [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: 

[FFmpeg-devel] [PATCH v2 2/3] avcodec/ass: accurately preserve colours

2022-11-13 Thread Oneric
Colour values used in ASS files without a "YCbCr Matrix" header set to
"None" are subject to colour mangling, due to how ASS was historically
conceived. A more in-depth description can be found in the documetation
inside libass' public ass_types.h header. The important part is, if this
header is not set to "None", the final output colours can deviate from
the literal value specified in the file. When converting from non-ASS
formats we do not want any colour shift to happen, so let's set the
appropiate header.

NB: ffmpeg's subtitle filter, does not follow libass' documentation
regarding colour mangling, thus hiding the bug. Anything based on
VSFilter, XySubFilter or e.g. mpv do and might show the issue.
(Of course native ASS subs, which _do_ rely on colour mangling won't
 work properly with the subtitle filter, but this can be fixed another
 time)
---
Link to libass’ docs regarding colour mangling:
  
https://github.com/libass/libass/blob/3df19c2e809b16c9cf7c925fa3bb573e2e6f4fdd/libass/ass_types.h#L152-L232
---
 libavcodec/ass.c | 1 +
 tests/ref/fate/sub-aqtitle   | 1 +
 tests/ref/fate/sub-cc| 1 +
 tests/ref/fate/sub-cc-realtime   | 1 +
 tests/ref/fate/sub-cc-scte20 | 1 +
 tests/ref/fate/sub-charenc   | 1 +
 tests/ref/fate/sub-jacosub   | 1 +
 tests/ref/fate/sub-microdvd  | 1 +
 tests/ref/fate/sub-movtext   | 1 +
 tests/ref/fate/sub-mpl2  | 1 +
 tests/ref/fate/sub-mpsub | 1 +
 tests/ref/fate/sub-mpsub-frames  | 1 +
 tests/ref/fate/sub-pjs   | 1 +
 tests/ref/fate/sub-realtext  | 1 +
 tests/ref/fate/sub-sami  | 1 +
 tests/ref/fate/sub-sami2 | 1 +
 tests/ref/fate/sub-scc   | 1 +
 tests/ref/fate/sub-srt   | 1 +
 tests/ref/fate/sub-srt-badsyntax | 1 +
 tests/ref/fate/sub-stl   | 1 +
 tests/ref/fate/sub-subviewer | 1 +
 tests/ref/fate/sub-subviewer1| 1 +
 tests/ref/fate/sub-vplayer   | 1 +
 tests/ref/fate/sub-webvtt| 1 +
 tests/ref/fate/sub-webvtt2   | 1 +
 25 files changed, 25 insertions(+)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index fdf55f36ca..d2ea4c62c3 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -41,6 +41,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "PlayResX: %d\r\n"
  "PlayResY: %d\r\n"
  "ScaledBorderAndShadow: yes\r\n"
+ "YCbCr Matrix: None\r\n"
  "\r\n"
  "[V4+ Styles]\r\n"
 
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index af0c06d7c2..ae5edcd9ab 100644
--- a/tests/ref/fate/sub-aqtitle
+++ b/tests/ref/fate/sub-aqtitle
@@ -4,6 +4,7 @@ ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
 ScaledBorderAndShadow: yes
+YCbCr Matrix: None
 
 [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
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 13f393cc86..516d26af9a 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -4,6 +4,7 @@ ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
 ScaledBorderAndShadow: yes
+YCbCr Matrix: None
 
 [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
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 169361f540..98dfef5501 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -4,6 +4,7 @@ ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
 ScaledBorderAndShadow: yes
+YCbCr Matrix: None
 
 [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
diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index be28084887..a97d29f70b 100644
--- a/tests/ref/fate/sub-cc-scte20
+++ b/tests/ref/fate/sub-cc-scte20
@@ -4,6 +4,7 @@ ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
 ScaledBorderAndShadow: yes
+YCbCr Matrix: None
 
 [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
diff --git a/tests/ref/fate/sub-charenc b/tests/ref/fate/sub-charenc
index 4efacb073d..339137ae0b 100644
--- a/tests/ref/fate/sub-charenc
+++ b/tests/ref/fate/sub-charenc
@@ -4,6 +4,7 @@ ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
 ScaledBorderAndShadow: yes
+YCbCr Matrix: None
 
 [V4+ Styles]
 Format: Name, Fontname, Fontsize, PrimaryColour, 

[FFmpeg-devel] [PATCH v2 1/3] avcodec/ass: fix comment

2022-11-13 Thread Oneric
There is no v4 ASS. There are versions 1 to 4 of SSA (with only v4
being supported by softsub renderers), ASS which declares itself as v4+
or v4.00+, and the rarely used v4++/v4.00++, which isn't fully supported
by libass.
As reflected by the [V4+ Styles] section header, FFmpeg uses ASS, not
SSA v4, so adjust the comment accordingly.
---
 libavcodec/ass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 728cfb1ab5..fdf55f36ca 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -44,7 +44,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "\r\n"
  "[V4+ Styles]\r\n"
 
- /* ASSv4 header */
+ /* ASS (v4+) header */
  "Format: Name, "
  "Fontname, Fontsize, "
  "PrimaryColour, SecondaryColour, OutlineColour, BackColour, "
-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2 0/3] Some small ASS conversion fixes

2022-11-13 Thread Oneric
This fixes colours and font selection for files converted to ASS

v2
==

On Nicolas George’s request resent with plain text diff for the updated
refernce files. There is no other change and applying the diff of v1
yields equal results to v2.

Those files do intentionally contain CRLF line endings,
but apart from tests/ref/fate/sub-scc they are not marked as
"diff eol=crlf" in .gitattributes, so the diff (intentionally) contains
some CRLF lines as well. Patchwork has or at least used to have bugs
dealing with CRLF diffs. In particular it may wrongly fail to apply the
patch (I believe this is fixed by now though) and the buttons
for downloading the diff or pasting it into the clipboard corrupt line
endings making the diff useless.
Instead directly apply the email in your inbox or use patchwork’s mbox
download. If your git is configured strictly, you may also need to use
git am --keep-cr ... . (Or just apply v1)

v1
==

Original version sent using binary diffs for reference files.


Oneric (3):
  avcodec/ass: fix comment
  avcodec/ass: accurately preserve colours
  avcodec/ass: specify a permissive encoding

 libavcodec/ass.c | 5 +++--
 tests/ref/fate/sub-aqtitle   | 3 ++-
 tests/ref/fate/sub-cc| 3 ++-
 tests/ref/fate/sub-cc-realtime   | 3 ++-
 tests/ref/fate/sub-cc-scte20 | 3 ++-
 tests/ref/fate/sub-charenc   | 3 ++-
 tests/ref/fate/sub-jacosub   | 3 ++-
 tests/ref/fate/sub-microdvd  | 3 ++-
 tests/ref/fate/sub-movtext   | 3 ++-
 tests/ref/fate/sub-mpl2  | 3 ++-
 tests/ref/fate/sub-mpsub | 3 ++-
 tests/ref/fate/sub-mpsub-frames  | 3 ++-
 tests/ref/fate/sub-pjs   | 3 ++-
 tests/ref/fate/sub-realtext  | 3 ++-
 tests/ref/fate/sub-sami  | 3 ++-
 tests/ref/fate/sub-sami2 | 3 ++-
 tests/ref/fate/sub-scc   | 3 ++-
 tests/ref/fate/sub-srt   | 3 ++-
 tests/ref/fate/sub-srt-badsyntax | 3 ++-
 tests/ref/fate/sub-stl   | 3 ++-
 tests/ref/fate/sub-subviewer | 3 ++-
 tests/ref/fate/sub-subviewer1| 3 ++-
 tests/ref/fate/sub-vplayer   | 3 ++-
 tests/ref/fate/sub-webvtt| 3 ++-
 tests/ref/fate/sub-webvtt2   | 3 ++-
 25 files changed, 51 insertions(+), 26 deletions(-)

-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/ass: accurately preserve colours

2022-11-13 Thread Oneric
Thanks for taking a look!

On Sun, Nov 13, 2022 at 20:12:46 +0100, Nicolas George wrote:
> Oneric (12022-11-13):
> >  [...]
> >  tests/ref/fate/sub-webvtt2   | Bin 1648 -> 1668 bytes
> 
> These are text files. Please fix this so that we can review the patch.

As explained in the cover letter, those files are intentionally using CRLF
line endings and patchwork bugs (or at least used to bug) out if it
receives patches with CRLF line endings. The last few times this always
caused some debate, so I used binary diffs to workaround it this time.
It just adds the "YCbCr Matrix: None" line in every file and changes
the Encoding field to "1" instead of "0".

If you prefer and believe the patchwork bugs won’t cause confusion,
I can of course immediately send a v2 with plain text diffs.

Regards

Oneric


signature.asc
Description: PGP signature
___
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 2/3] avcodec/ass: accurately preserve colours

2022-11-13 Thread Oneric
Colour values used in ASS files without a "YCbCr Matrix" header set to
"None" are subject to colour mangling, due to how ASS was historically
conceived. A more in-depth description can be found in the documetation
inside libass' public ass_types.h header. The important part is, if this
header is not set to "None", the final output colours can deviate from
the literal value specified in the file. When converting from non-ASS
formats we do not want any colour shift to happen, so let's set the
appropiate header.

NB: ffmpeg's subtitle filter, does not follow libass' documentation
regarding colour mangling, thus hiding the bug. Anything based on
VSFilter, XySubFilter or e.g. mpv do and might show the issue.
(Of course native ASS subs, which _do_ rely on colour mangling won't
 work properly with the subtitle filter, but this can be fixed another
 time)
---
Link to libass’ docs regarding colour mangling:
  
https://github.com/libass/libass/blob/3df19c2e809b16c9cf7c925fa3bb573e2e6f4fdd/libass/ass_types.h#L152-L232
---
 libavcodec/ass.c |   1 +
 tests/ref/fate/sub-aqtitle   | Bin 3213 -> 3233 bytes
 tests/ref/fate/sub-cc| Bin 839 -> 859 bytes
 tests/ref/fate/sub-cc-realtime   | Bin 1524 -> 1544 bytes
 tests/ref/fate/sub-cc-scte20 | Bin 945 -> 965 bytes
 tests/ref/fate/sub-charenc   | Bin 6008 -> 6028 bytes
 tests/ref/fate/sub-jacosub   | Bin 1783 -> 1803 bytes
 tests/ref/fate/sub-microdvd  | Bin 1375 -> 1395 bytes
 tests/ref/fate/sub-movtext   | Bin 783 -> 803 bytes
 tests/ref/fate/sub-mpl2  | Bin 870 -> 890 bytes
 tests/ref/fate/sub-mpsub | Bin 2517 -> 2537 bytes
 tests/ref/fate/sub-mpsub-frames  | Bin 736 -> 756 bytes
 tests/ref/fate/sub-pjs   | Bin 860 -> 880 bytes
 tests/ref/fate/sub-realtext  | Bin 939 -> 959 bytes
 tests/ref/fate/sub-sami  | Bin 1981 -> 2001 bytes
 tests/ref/fate/sub-sami2 | Bin 9970 -> 9990 bytes
 tests/ref/fate/sub-scc   |   1 +
 tests/ref/fate/sub-srt   | Bin 6301 -> 6321 bytes
 tests/ref/fate/sub-srt-badsyntax | Bin 1561 -> 1581 bytes
 tests/ref/fate/sub-stl   | Bin 2241 -> 2261 bytes
 tests/ref/fate/sub-subviewer | Bin 780 -> 800 bytes
 tests/ref/fate/sub-subviewer1| Bin 1494 -> 1514 bytes
 tests/ref/fate/sub-vplayer   | Bin 742 -> 762 bytes
 tests/ref/fate/sub-webvtt| Bin 1985 -> 2005 bytes
 tests/ref/fate/sub-webvtt2   | Bin 1648 -> 1668 bytes
 25 files changed, 2 insertions(+)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index fdf55f36ca..d2ea4c62c3 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -41,6 +41,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "PlayResX: %d\r\n"
  "PlayResY: %d\r\n"
  "ScaledBorderAndShadow: yes\r\n"
+ "YCbCr Matrix: None\r\n"
  "\r\n"
  "[V4+ Styles]\r\n"
 
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index 
af0c06d7c2809e2bfe8859f2f53cafd3c1808889..ae5edcd9abe039d33dd3e2da496fd991dcbe8490
 100644
GIT binary patch
delta 29
kcmeB`TqrrAK`7EW$+<|uH?gEBv%*TjFF!AJV;w_O{{R30

delta 10
RcmZ1|*(*7rVdE5D9sm~71J3{e

diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 
13f393cc86b9f797be24f37a07aafc7272c22e59..516d26af9ab09613f939d004826e44c80b3b1054
 100644
GIT binary patch
delta 29
kcmX@kcAITNgHWV%l5>%QZ(>PNW`#=4VC0HTfx{Qv*}

delta 10
Rcmcc3cARZO!^SDcnE)Fg1kC^d

diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 
169361f540e4dff43a132df395e55a19ff73..98dfef55019719911d6c5d9faa0c057cc324f227
 100644
GIT binary patch
delta 29
kcmeyu-N7@VK`7EW$+<|uH?gEBv%*TjFF!AJV_g|50H13Lk^lez

delta 10
RcmeC+`NBP+VdIn%Rsb0Q1Y!UH

diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index 
be28084887a9b7fb80ae4ff3369cf7192a978919..a97d29f70ba1d3984887eacb6e833abb8a406902
 100644
GIT binary patch
delta 29
kcmdnUew2MegHWV%l5>%QZ(>PNW`#yVqW0G!DRJ^%m!

delta 10
RcmX@gzL9-G!^SCw%m5g91P%ZI

diff --git a/tests/ref/fate/sub-charenc b/tests/ref/fate/sub-charenc
index 
4efacb073d14b43002b58ab0b4aa55e9d34de029..339137ae0b5485c4c954f859316cf8b413b01505
 100644
GIT binary patch
delta 29
kcmeyN*P}n7K`7EW$+<|uH?gEBv%*TjFF!AJW8E(?0I0?bbpQYW

delta 10
RcmeCt|DiXbVdIn^VgMYG1w{Y=

diff --git a/tests/ref/fate/sub-jacosub b/tests/ref/fate/sub-jacosub
index 
b574dda54dfdee55a07f13c808ee3baa6e100798..32086d9365399ae3a968f9f61c6e3fa83a35ff65
 100644
GIT binary patch
delta 29
kcmey)+s!wjK`7EW$+<|uH?gEBv%*TjFF!AJV_hX10HB5npa1{>

delta 10
RcmeC?`_4O|VdIo?HUJr21aJTV

diff --git a/tests/ref/fate/sub-microdvd b/tests/ref/fate/sub-microdvd
index 
4ddb254c698245958b12e3ab0e988ef6359592a8..11440c28243f00cf99f07ff91ab2eb1c23dc2cc1
 100644
GIT binary patch
delta 29
kcmcc5^_gozgHWV%l5>%QZ(>PNW`#=3_r0I1jtPXGV_

delta 10
Rcmey)RcO!^SE1SpXeE1s?za

diff --git a/tests/ref/fate/sub-movtext b/tests/ref/fate/sub-movtext
index 

[FFmpeg-devel] [PATCH 3/3] avcodec/ass: specify a permissive encoding

2022-11-13 Thread Oneric
The Encoding field (and the \fe tag) allows to limit font selection to
only those fonts declaring support for the specified codepage in their
OS/2's table "Code Page Character Range" field.
Particularly, Encoding=0 means only font's declaring support for "ANSI",
or rather "Latin (Western European)", are allowed to be selected.
Specifying Encoding=1 allows all fonts to be considered.
We do not want to limit font selection, so specify Encoding=1.

NB: at the time of writing libass only partially supports this field,
thus hiding the issue in any libass-based renderer. A VSFilter-based
DirectShow filter or XySubFilter will reveal the issue when a font not
declaring support for latin characters is specified in a style.
---
 libavcodec/ass.c |   2 +-
 tests/ref/fate/sub-aqtitle   | Bin 3233 -> 3233 bytes
 tests/ref/fate/sub-cc| Bin 859 -> 859 bytes
 tests/ref/fate/sub-cc-realtime   | Bin 1544 -> 1544 bytes
 tests/ref/fate/sub-cc-scte20 | Bin 965 -> 965 bytes
 tests/ref/fate/sub-charenc   | Bin 6028 -> 6028 bytes
 tests/ref/fate/sub-jacosub   | Bin 1803 -> 1803 bytes
 tests/ref/fate/sub-microdvd  | Bin 1395 -> 1395 bytes
 tests/ref/fate/sub-movtext   | Bin 803 -> 803 bytes
 tests/ref/fate/sub-mpl2  | Bin 890 -> 890 bytes
 tests/ref/fate/sub-mpsub | Bin 2537 -> 2537 bytes
 tests/ref/fate/sub-mpsub-frames  | Bin 756 -> 756 bytes
 tests/ref/fate/sub-pjs   | Bin 880 -> 880 bytes
 tests/ref/fate/sub-realtext  | Bin 959 -> 959 bytes
 tests/ref/fate/sub-sami  | Bin 2001 -> 2001 bytes
 tests/ref/fate/sub-sami2 | Bin 9990 -> 9990 bytes
 tests/ref/fate/sub-scc   |   2 +-
 tests/ref/fate/sub-srt   | Bin 6321 -> 6321 bytes
 tests/ref/fate/sub-srt-badsyntax | Bin 1581 -> 1581 bytes
 tests/ref/fate/sub-stl   | Bin 2261 -> 2261 bytes
 tests/ref/fate/sub-subviewer | Bin 800 -> 800 bytes
 tests/ref/fate/sub-subviewer1| Bin 1514 -> 1514 bytes
 tests/ref/fate/sub-vplayer   | Bin 762 -> 762 bytes
 tests/ref/fate/sub-webvtt| Bin 2005 -> 2005 bytes
 tests/ref/fate/sub-webvtt2   | Bin 1668 -> 1668 bytes
 25 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index d2ea4c62c3..5058dc8337 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -65,7 +65,7 @@ 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] */
- "0\r\n"/* Encoding */
+ "1\r\n"/* Encoding */
 
  "\r\n"
  "[Events]\r\n"
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index 
ae5edcd9abe039d33dd3e2da496fd991dcbe8490..2980d394278644f0a1b8361833ed2bb4bd2a506b
 100644
GIT binary patch
delta 14
WcmZ1|xlnS$D@I1c&951^aRC4;vjw35

delta 14
WcmZ1|xlnS$D@I0x&951^aRC4;tp%U}

diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 
516d26af9ab09613f939d004826e44c80b3b1054..5e4f5ff7fcdeb665d7de42a3d8856474e9ee2765
 100644
GIT binary patch
delta 14
Vcmcc3cAIU(J4Qys>j=7y*8A~

delta 14
Vcmcc3cAIU(J4Qx>>j=7y}1)~4}

diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 
98dfef55019719911d6c5d9faa0c057cc324f227..22f09d5cddbf7181760a9db32cc4aa21f83574cb
 100644
GIT binary patch
delta 14
VcmeC+>EPM$j*-!D^Ls`~762yY1iAnK

delta 14
VcmeC+>EPM$j*-z|^Ls`~762yS1i1hJ

diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index 
a97d29f70ba1d3984887eacb6e833abb8a406902..cded979cb874371943ce5eb0648b472fec9d5d1b
 100644
GIT binary patch
delta 14
WcmX@gew2N~J4Qys>knF#-TC76s%0

delta 14
WcmX@gew2N~J4Qx>>knF#-TC5C!7^

diff --git a/tests/ref/fate/sub-charenc b/tests/ref/fate/sub-charenc
index 
339137ae0b5485c4c954f859316cf8b413b01505..67e209856fccd5e1aebbacbe4c966ec769aebcd2
 100644
GIT binary patch
delta 14
VcmeCt@6q4zijmQ9^J~V1q5vy`1(^T<

delta 14
VcmeCt@6q4zijmP^^J~V1q5vy=1(*N;

diff --git a/tests/ref/fate/sub-jacosub b/tests/ref/fate/sub-jacosub
index 
32086d9365399ae3a968f9f61c6e3fa83a35ff65..9f555fb7470fa81d30e7668ede0fa883b105d729
 100644
GIT binary patch
delta 14
VcmeC?>*m|=ijmQ9^J_*0RsbhH1jGOU

delta 14
VcmeC?>*m|=ijmP^^J_*0RsbhB1j7IT

diff --git a/tests/ref/fate/sub-microdvd b/tests/ref/fate/sub-microdvd
index 
11440c28243f00cf99f07ff91ab2eb1c23dc2cc1..fa418f53db612e33dd6839b9ed1ce5ba8985e82f
 100644
GIT binary patch
delta 14
Vcmey&^_gqKCq_oY&7TA

diff --git a/tests/ref/fate/sub-mpl2 b/tests/ref/fate/sub-mpl2
index 
d740fbc365460187e6b4edcf3faa00bdb5638196..f4e46e48e98bca0002f7cdfe4c78e02fa7dd436a
 100644
GIT binary patch
delta 14
Vcmeyx_KR)9D@I1c&952z838h$1^oa3

delta 14
Vcmeyx_KR)9D@I0x&952z838hw1^fU2

diff --git a/tests/ref/fate/sub-mpsub b/tests/ref/fate/sub-mpsub
index 
4c3d37fa42206d262e221b1d0b81312289329143..28c36ea40ca5bfb08e30c2ae7add905a9135c398
 100644

[FFmpeg-devel] [PATCH 0/3] Some small ASS conversion fixes

2022-11-13 Thread Oneric
This fixes colours and font selection for files converted to ASS.

To avoid a bug with patchwork’s diff download stripping CRLFs,
I formatted the updates of reference files as binary.
They intentionally contain CRLF characters and without them do
not pass FATE. tests/ref/fate/sub-scc keeps a text diff since it
alone is marked as "diff eol=crlf" in .gitattributes.

Oneric (3):
  avcodec/ass: fix comment
  avcodec/ass: accurately preserve colours
  avcodec/ass: specify a permissive encoding

 libavcodec/ass.c |   5 +++--
 tests/ref/fate/sub-aqtitle   | Bin 3213 -> 3233 bytes
 tests/ref/fate/sub-cc| Bin 839 -> 859 bytes
 tests/ref/fate/sub-cc-realtime   | Bin 1524 -> 1544 bytes
 tests/ref/fate/sub-cc-scte20 | Bin 945 -> 965 bytes
 tests/ref/fate/sub-charenc   | Bin 6008 -> 6028 bytes
 tests/ref/fate/sub-jacosub   | Bin 1783 -> 1803 bytes
 tests/ref/fate/sub-microdvd  | Bin 1375 -> 1395 bytes
 tests/ref/fate/sub-movtext   | Bin 783 -> 803 bytes
 tests/ref/fate/sub-mpl2  | Bin 870 -> 890 bytes
 tests/ref/fate/sub-mpsub | Bin 2517 -> 2537 bytes
 tests/ref/fate/sub-mpsub-frames  | Bin 736 -> 756 bytes
 tests/ref/fate/sub-pjs   | Bin 860 -> 880 bytes
 tests/ref/fate/sub-realtext  | Bin 939 -> 959 bytes
 tests/ref/fate/sub-sami  | Bin 1981 -> 2001 bytes
 tests/ref/fate/sub-sami2 | Bin 9970 -> 9990 bytes
 tests/ref/fate/sub-scc   |   3 ++-
 tests/ref/fate/sub-srt   | Bin 6301 -> 6321 bytes
 tests/ref/fate/sub-srt-badsyntax | Bin 1561 -> 1581 bytes
 tests/ref/fate/sub-stl   | Bin 2241 -> 2261 bytes
 tests/ref/fate/sub-subviewer | Bin 780 -> 800 bytes
 tests/ref/fate/sub-subviewer1| Bin 1494 -> 1514 bytes
 tests/ref/fate/sub-vplayer   | Bin 742 -> 762 bytes
 tests/ref/fate/sub-webvtt| Bin 1985 -> 2005 bytes
 tests/ref/fate/sub-webvtt2   | Bin 1648 -> 1668 bytes
 25 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/3] avcodec/ass: fix comment

2022-11-13 Thread Oneric
There is no v4 ASS. There are versions 1 to 4 of SSA (with only v4
being supported by softsub renderers), ASS which declares itself as v4+
or v4.00+, and the rarely used v4++/v4.00++, which isn't fully supported
by libass.
As reflected by the [V4+ Styles] section header, FFmpeg uses ASS, not
SSA v4, so adjust the comment accordingly.
---
 libavcodec/ass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 728cfb1ab5..fdf55f36ca 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -44,7 +44,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "\r\n"
  "[V4+ Styles]\r\n"
 
- /* ASSv4 header */
+ /* ASS (v4+) header */
  "Format: Name, "
  "Fontname, Fontsize, "
  "PrimaryColour, SecondaryColour, OutlineColour, BackColour, "
-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avcodec/libsvtav1: Add support for multipass encoding

2022-10-30 Thread Oneric
On Sun, Oct 30, 2022 at 14:10:29 +0100, Lynne wrote:
> Sep 27, 2022, 23:06 by gustav.grus...@gmail.com:
> 
> > Implements support for 2-pass CRF and 3-pass VBR by implementing
> > reading and writing of stats file, and passing the pass number on
> > to the encoder. For 3-pass VBR, the first pass should be run with
> > '-pass 1', the second with '-pass 3' and the third with '-pass 2'.
> >
> > [...]
> 
> 2-pass doesn't seem to work, the encoder complains it's not the final pass.

fwiw, 2-pass CRF and 3-pass VBR work for me on a short sample with this
patch applied on top of 882a17068fd8e62c7d38c14e6fb160d7c9fc446a and using
SVT-AV1 1.3.0.

I'm not sure if 2-pass VBR is supposed to work, though I didn't look into
it too much. SVT-AV1 docs only mention 3-pass VBR[1] afaict and while
requesting output already in the second pass (`--pass 2` for SvtAv1EncApp
and `-pass 3` for patched ffmpeg) "works", the result misses the targeted
overall bitrate even more than singlepass VBR.

For reference here are the commands I used:

2-pass CRF:
---
  ./ffmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -crf 50 -g 300 -pass 1 -f null /dev/null \
  && ./ffmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -crf 50 -g 300 -pass 2 -f matroska crf_pass2.mkv


3-pass VBR:
---
  ./ffmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -svtav1-params 'rc=1:tbr=500k' -g 300 -pass 1 \
-f null /dev/null \
 && ./fmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -svtav1-params 'rc=1:tbr=500k' -g 300 -pass 3 \
-f null /dev/null \
 && ./fmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -svtav1-params 'rc=1:tbr=500k' -g 300 -pass 2 \
-f matroska vbr_pass2.mkv


"2-pass VBR" (probably bad idea):
-
  ./ffmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -svtav1-params 'rc=1:tbr=500k' -g 300 -pass 1 \
-f null /dev/null \
  && ./fmpeg -i org.mkv -c:v libsvtav1 \
-preset 4 -svtav1-params 'rc=1:tbr=500k' -g 300 -pass 3 \
-f matroska vbr_pass2.mkv


[1]: 
https://gitlab.com/AOMediaCodec/SVT-AV1/-/blob/91b94efb2809e83d9bf041d8575b32f234dfef27/Docs/svt-av1_encoder_user_guide.md#multi-pass-vbr-1000-kbps-at-maximum-quality-from-24fps-yuv-1920x1080-input
___
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] FFmpeg 5.0.1

2022-03-29 Thread Oneric
On Tue, Mar 29, 2022 at 01:31:02 +0200, Michael Niedermayer wrote:
> I intend to do a 5.0.1 release from the release/5.0 branch in the next days
> as its high time to make a new release with all the bugfixes
> so if you want to backport something please do so

How about the recent fix for vf_subtitles 
66901ce16271d1e36726af53f35cb5cd88b0b773:
  
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220314190638.24816-1-one...@oneric.de/
Is it eligible for backporting?

Backporting to 5.0 should be a straightforward cherry-pick, for 
backporting to 4.x see the patch from the following message instead:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294513.html
___
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] avfilter/vf_subtitles: pass storage size to libass

2022-03-23 Thread Oneric
On Tue, Mar 22, 2022 at 13:45:20 -0300, James Almer wrote:
> 
> Will apply it unless someone is against it.

Thanks for applying the patch!

In case this fix is eligible for backporting:
It applies nicely at is to the release/5.0 branch and 5.0 also already 
requires a new enough libass for ass_set_storage_size to be always 
available.
For the release/4.[0-4] branches, the attached patch can be used instead.
It applied without problems for me on all the 4.x branches and also built 
and passed FATE with the config I used.
>From 27b6deafe859eb9bddfb21498a11f2b2b613802b Mon Sep 17 00:00:00 2001
From: Oneric 
Date: Wed, 23 Mar 2022 20:43:54 +0100
Subject: [PATCH] avfilter/vf_subtitles: pass storage size to libass

Due to a quirk of the ASS format some tags depend on the exact storage
resolution of the video, so tell libass via ass_set_storage_size.
---
 libavfilter/vf_subtitles.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index de74afa2b7..b57dd80b13 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -145,9 +145,16 @@ static int config_input(AVFilterLink *inlink)
 ff_draw_init(>draw, inlink->format, ass->alpha ? FF_DRAW_PROCESS_ALPHA : 0);
 
 ass_set_frame_size  (ass->renderer, inlink->w, inlink->h);
-if (ass->original_w && ass->original_h)
+if (ass->original_w && ass->original_h) {
 ass_set_aspect_ratio(ass->renderer, (double)inlink->w / inlink->h,
  (double)ass->original_w / ass->original_h);
+#if LIBASS_VERSION > 0x0101
+ass_set_storage_size(ass->renderer, ass->original_w, ass->original_h);
+} else {
+ass_set_storage_size(ass->renderer, inlink->w, inlink->h);
+#endif
+}
+
 if (ass->shaping != -1)
 ass_set_shaper(ass->renderer, ass->shaping);
 
-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage size to libass

2022-03-22 Thread Oneric
On Mon, Mar 14, 2022 at 20:06:39 +0100, Oneric wrote:
> Due to a quirk of the ASS format some tags depend on the exact storage
> resolution of the video, so tell libass via ass_set_storage_size.
> [...]

On Mon, Mar 14, 2022 at 20:21:47 +, Soft Works wrote:
> [...]
>
> Ah, alright, the blur setting goes deeper. Thanks for the explanation.
> 
> LGTM, then!


ping
___
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] avfilter/vf_subtitles: pass storage size to libass

2022-03-14 Thread Oneric
On Mon, Mar 14, 2022 at 19:57:05 +, Soft Works wrote:
> > > ass_set_pixel_aspect() is setting settings.par and if I'm not mistaken,
> > > an existing par setting leads to the storage size setting to be ignored:
> > 
> > It’s not overridden. Only the explicit PAR is currently preferd over the
> > implicit derivation from storage and frame size. However as I stated in
> > the patch description and the comment:
> >   “some tags depend on the exact storage resolution of the video”
> >   “it actually goes farther than just the aspect ratio”
> > 
> 
> I found only one other place where storage_h is used (for determining 
> blur size) but I didn't find any other usage in the libass source code.
> That's what I'm wondering about.

Well, blur is one of the things that depend on it. If you follow the usage 
of the blur scale, you'll see it also plays a role in the projection 
matrix for 3D-transforms (what the provided samples use) and if 
ScaledBorderAndShadow is not set to "yes", it also affects some other 
scaling values.

This unfortunate dependence is a result of how SSA and then ASS 
histoically developed and required to maintain compaitibility with 
existing subtitles.
___
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] avfilter/vf_subtitles: pass storage size to libass

2022-03-14 Thread Oneric
On Mon, Mar 14, 2022 at 19:35:36 +, Soft Works wrote:
> 
> I've been at the same point some time ago where I wondered why ffmpeg is
> not setting this, but then I had found that it is overridden by the call 
> to ass_set_pixel_aspect().
>
> ass_set_pixel_aspect() is setting settings.par and if I'm not mistaken,
> an existing par setting leads to the storage size setting to be ignored:

It’s not overridden. Only the explicit PAR is currently preferd over the 
implicit derivation from storage and frame size. However as I stated in 
the patch description and the comment:
  “some tags depend on the exact storage resolution of the video”
  “it actually goes farther than just the aspect ratio”

I.e. there's more info in the storage size than just the PAR.
You can also easily test the files I linked to empirically
validate that there is in fact a difference.

> But perhaps I'm missing something..
>
> softworkz
___
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] avfilter/vf_subtitles: pass storage size to libass

2022-03-14 Thread Oneric
Due to a quirk of the ASS format some tags depend on the exact storage
resolution of the video, so tell libass via ass_set_storage_size.

---
ass_set_storage_size exists since libass 0.10.2;
ffmpeg since 5.0 already requires 0.11.0.

This resolution dependences of ASS was already recognised when the
original_size parameter was added, but it actually goes farther than
just the aspect ratio. Conveniently this parameter still has all the
required information to retain rendering after resizing :)

Sample files to show the difference can be found eg here
https://code.videolan.org/videolan/vlc/uploads/b54e0761d0d3f4f79b2947ffb83a3b59/vlc-issue_libass-storage-size.tar.xz

./ffmpeg -i test_1080p.mkv -filter:v ass=./test_1080p.ass tmp_1080.mkv
./ffmpeg -i anamorphic_s720x576_d1024x576.mkv -filter:v 
ass=./anamorphic_s720x576_d1024x576.ass tmp_anam.mkv

---
 libavfilter/vf_subtitles.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 3fc4eeb63d..af6352b315 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -146,9 +146,14 @@ static int config_input(AVFilterLink *inlink)
 ff_draw_init(>draw, inlink->format, ass->alpha ? 
FF_DRAW_PROCESS_ALPHA : 0);
 
 ass_set_frame_size  (ass->renderer, inlink->w, inlink->h);
-if (ass->original_w && ass->original_h)
+if (ass->original_w && ass->original_h) {
 ass_set_pixel_aspect(ass->renderer, (double)inlink->w / inlink->h /
  ((double)ass->original_w / ass->original_h));
+ass_set_storage_size(ass->renderer, ass->original_w, ass->original_h);
+} else {
+ass_set_storage_size(ass->renderer, inlink->w, inlink->h);
+}
+
 if (ass->shaping != -1)
 ass_set_shaper(ass->renderer, ass->shaping);
 
-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


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

2022-02-05 Thread Oneric
On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> Let's try to approach this from a different side. Which case is
> your [1/2] commit actually supposed to fix?

Escape backslashes when converting from WebVTT to not accidentally
introduce active ASS sequences and replace the wrong backslash-escape
in ff_ass_bprint_text_event with an escape that actually works.

> How did you test your patch?
> Can we please go over an example?

Take a look at the attached WebVTT file.
We expect the second event to be rendered like this,
as from WebVTT’s point of view it’s just normal text:

  our final \h approach \N into \ Coruscant.

What we currently get after conversion to ASS is like this though
(pay attention to the number of spaces):

  our final   approach
  into \ Coruscant.

If we escape \ as \\ like ff_ass_bprint_text_event currently does,
we instead get the following rendering which is also wrong:

  our final \  approach \
  into \\ Coruscant.

If instead the word-joiner is appended as in my patch, the
visual output matches the expectation (mail does not contain U+2060):

  our final \h approach \N into \ Coruscant.


You can confirm this for yourself by feeding the original WebVTT
to a native WebVTT renderer, e.g. https://zcorpan.github.io/live-webvtt-viewer/
and using ./ffmpeg -i test2.vtt -f ass tmp_conv.ass on
master, master + my patch and master + a modified patch to use \\
then watching how the converted file gets rendered by mpv, VLC or so.

I was not able to create a sample using ff_ass_bprint_text_event
as the only users seem to be teletext and some rawtext(?) subtitles
and I don't know how to create such files.
However, as nothing special happened to the \\ for WebVTT after this
sequence was inserted for the internal representation, and since
the preceding comment (wrongly) declares \\ to be an ASS rather than an
internal escape, it seems highly unlikely to behave any different.
You are very welcome to provide a sample using the
ff_ass_bprint_text_event codepath and keep_ass_markup=false
to verify or debunk this.
WEBVTT

00:00.000 --> 00:01.000
Senator, we're {A} making
 

00:01.000 --> 01:01.000
our final \h approach \N into \ Coruscant. 
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-04 Thread Oneric
On Fri, Feb 04, 2022 at 23:24:58 +, Soft Works wrote:
> You want to "pollute" gazillions of subtitle streams in the
> world from multiple subtitle formats with invisible
> characters in order to solve an escaping problem in ffmpeg?

I do not consider using characters that are explicitly recommended to be
used by Unicode to be “polluting”. Further consider that as mentioned
invisible characters in ASS are not uncommon anyway already and conversion
from ASS to something else are rare due to being generally lossy. Lossy 
with regards to typesetting that is, removing breaking hints in form of
plain Unicode characters would be a new form of lossyness.

> [From the other mail:]
> I'm not into changing ffmpeg's ass output, it's all
> about the internally used ass format and the escaping is
> a central problem there.

I’m not interested in reworking ffmpeg’s internal subtitle handling.
The proposed patch is a clear improvement over the status quo which
is plain incorrect. Within reasonable effort and sound arguments for
it adjustments to the patch can be made; reworking ffmpeg internals is
imo not “reasonable” effort to correct an uncontestedly wrong escape.

You have two options:
Either finally tell me what I asked about:
where (as in which file and function) removing wordjoiners should
even happen and where possible lingering “\\ → \” conversions presumably
are and if it’s simple enough I can add a removal accompanied by a comment
pointing out that this can go wrong.
Or go ahead and create your own patch.

~~

> > > I'm not sure whether all ffmpeg text-sub encoders can handle
> > > those chars - which could be verified of course.
> >
> > Since it's in the BMP and ffmpeg already seems happy to assume some
> > UTF-8
> > support by converting everything to it, I'm not worried about this
> > until
> > proven wrong.
>
> Proven wrong: https://github.com/libass/libass/issues/507

This issue is not at all wordjoiner specific despite the name.
As far as I recall this never lead to wrong rendering.
With HarfBuzz, the only fully featured shaping backend of libass,
control characters were and are handled by HarfBuzz.
And even with FriBiDi U+2060 was ignored since long before (2012)
the linked issue was opened.

What that issue really is about is a combination of two more general
issues. libass is currently not caching failure to lookup a glyph leading
to multiple messages and at worst a perf degradation if no font on the
font pool contained a glyph for a particular glyph. And the realisation
that libass’ font-fallback strategy is not ideal for prefix-type control
characters, characters which visibly affect both neighbours and a few
others.
The word-joiner is only highlighted here as due to its usage as an
backslash escape its commonly passed to libass and a high enough
percentage of fonts doesn’t contain it to create reports about it.


For further reference: U+2060 was added in Unicode 3.2 released 2002.
If you want to strip it because it might not render correctly you should
also strip most emoji, the uppercase eszett ẞ and several actively 
used writing systems in their entirety.
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-04 Thread Oneric
On Fri, Feb 04, 2022 at 02:30:37 +0100, Andreas Rheinhardt wrote:
> All text-based subtitles are supposed to be UTF-8 when they reach the
> decoder; if it isn't, the user has to set the appropriate -sub_charenc
> and -sub_charenc_mode.
> 
> - Andreas

Thanks for the info! Then at least the UTF-8 assumption
is no problem after all.


On Fri, Feb 04, 2022 at 01:57:48 +, Soft Works wrote:
> > There's no way of knowing whether the word-joiner comes from
> > a conversion performed by ffmpeg in the past or already existed
> > in the original source.
> 
> That might be true, but I think it's valid to say that such characters
> are very unusual "original" subtitle sources and that's why I don't
> think it's a good idea for ffmpeg to start injecting them.

Don't underestimate what subtitle authors can come up with :)

> Subtitle implementations are often rather minimal, especially in
> hardware devices and might not always cover the full range of 
> UTF-8 specifics.

The wordjoiner lies in the Basic Multilingual Plane, so even ancient UTF-8 
implementations assuming all of Unicode’s codepoints fit in 16bits
(i.e. 3-bytes max per codepoint in UTF-8) will be able to understand it.

> > However, the wordjoiner does not alter the visually appearance and
> > is unlikely to change line-breaking properties; that's why I chose
> > a word-joiner. Therefore I don't think removing (only) the inserted
> > word-joiners is possible,
> 
> Why not? As it seems to be required for ASS encoding only, all other
> output formats should remain unaffected. 

Because — as written before — it can exist in the original source.
Unicode recommends using the wordjoiner eg to prevent linebreaks
between two characters without any additional side-effects as eg
the combining-grapheme-joiner would cause.

> > but also not necessary.
> 
> I'm not sure whether all ffmpeg text-sub encoders can handle 
> those chars - which could be verified of course.

Since it's in the BMP and ffmpeg already seems happy to assume some UTF-8 
support by converting everything to it, I'm not worried about this until
proven wrong.


> Finally, those chars are a pest. I'm using them myself for a 
> specific use case, but when you don't know they are there, it can
> drive you totally mad, eventually even thinking your system or
> software is faulty.
> 
> Example: 
> 
> Open your patch file [2/2] and search for the string
> "123456\NAscending". You can see the string in two lines, but search
> will only find one of them.
> 
> Or just look at the two lines directly. They are preceded by + and -
> even though both appear identical. 

Actually, I see this with helpful colouring lost here:

  -Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: 
123456\NAscending: 123456^M
  +Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: 
<200f>123456<200e>\NAscending: 123456^M

More plain-text oriented editors likely won't show them though, yes.

On this topic, finding raw bidi-marks in ASS subtitles for RTL-languages
is not that unusual though, to give an example for "invisible characters"
being used manually in the original source.
(Because VSFilters (and libass in the interest of compatibility)
 assumes LTR by default and other things)

Even if I thought removing all wordjoiners when converting from ASS
was a good idea, I still wouldn't know where to do this (or where to
look to remove possibly lingering attempts to recollapse \\ into \).
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-04 Thread Oneric
On Fri, Feb 04, 2022 at 06:48:40 +, Soft Works wrote:
> > [two-part message removed for brevity]
>
> I've found out where the \{ and \} escaping has come from: libass

As already written in the commit-message of the first patch..


You already noticed your proposal only works with VSFilters,
but even without this it's a terrible approach. Consider:
 - fullwidth characters have different metrics then the "regular" ones
 - fullwidth and small characters have a different visual appearance
 - support for fullwidth and small characters in fonts is much rarer
   than support for plain {}
 - fullwidth characters are commonly used _as fullwidth characters_
   e.g. in text using one of the CJK writing systems.
   Replacing them with non-fullwidth variants when transforming
   away from ASS is guaranteed to be disastrous.
 - Not sure if applies, but something to keep in mind:
   {\r} is not a noop if the source-format had any sort of per-event
   styling which got prepended to the ASS event text before
   using the plain-text conversion for the rest of the event.

As noted in the discussion of the libass issue you linked,
it’s not unusual for ASS subtitle authors to employ
fullwidth curly braces for displaying curly braces
in all ASS-renderers. However, they have tight control over the
fonts used and can carefully select them to match the visual
appearance and compensate differing metrics with bespoke
local adjustments to \fs and negative \fsp.
ffmpeg does not have tight control over the fonts and it'd be silly
to require users to pass in special fonts just to render curly braces.

If you want to make the rendering in VSFilters not complettly broken,
try to do what the libass-wiki recommends and add an empty command
block after an escaped opening curly brace. This way VSFilters
will display a lone backslash instead of a opening curly brace
but otherwise work fine without swallowing up text.
If done consistently closing curly braces won't need to be
escaped at all anymore.
However, such a VSFilter-compatibility change is unrelated to
fixing the broken \\ escape which doesn't work anywhere.

(Note that the wordjoiner doesn't have font or spacing issues as
 it’s defined to be invisible and zero-width.
 If a user supplies a special font like FontoXMLWhitespace¹
 showing the word-joiner that's not a problem anymore but a
 deliberate choice)

[1]: 
https://documentation.fontoxml.com/latest/whitespace-visualization-5b5c6b154c78#id-18b2211e-79cb-4b85-8390-1e54d0f45466
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-03 Thread Oneric
On Thu, Feb 03, 2022 at 20:51:16 +, Soft Works wrote:
> I think when you inject that word-joiner as a workaround for ass
> parsing, you'll also need to make sure that it gets removed
> when encoding to other formats.

There's no way of knowing whether the word-joiner comes from
a conversion performed by ffmpeg in the past or already existed
in the original source.
However, the wordjoiner does not alter the visually appearance and
is unlikely to change line-breaking properties; that's why I chose
a word-joiner. Therefore I don't think removing (only) the inserted
word-joiners is possible, but also not necessary.

Also, was the wrong \\ recollapsed into a single \ somehwere? If yes,
then this code can be dropped as well (but I don't know where it is).

My biggest concern with this is the unconditional assumption of UTF-8 in
ff_ass_bprint_text_event? Is there a way to improve on this or does ffmpeg 
convert all text to UTF-8 before it's passed to this function?
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-02 Thread Oneric
On Wed, Feb 02, 2022 at 22:44:18 +, Soft Works wrote:
> > Sent: Wednesday, February 2, 2022 11:19 PM
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > 
> >  [claims about git and smtp: part 1]
> > 
> 
> [claims about git and smtp: part 2]
> 

None of this relates to the topic of the patches, which
is backslash-escaping in ASS and converting BIDI-marks
from WebVTT.
As written before, I see no point in advancing this offtopic talk;
please actually discuss the content of the patches in this thread.
If you are convinced that the reference files should be treated as binary, 
then submit a suitable gitattributes patch to the list and discuss it 
with whom it may interest.

I’ll ignore all further offtopic messages here.
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-02 Thread Oneric
It appears my previous mail has been interpreted as some sort of attack
against you; this is not the case. I am sorry for creating such a
misunderstanding.
The mail was merely meant to clear up the misconception that it would be 
impossible to send mixed line-endings via mail due to some SMTP property
using some illustrative examples from the current thread.
(I have no idea how SMTP works in detail)

I'll respond to the new misunderstandings below in more detail if you're 
interested, but I see little point in continuing to discuss how mails are 
transferred or encoded.
Can we now start to discuss the patches themselves instead?

If you want to apply them and have trouble with getting the correct patch out
of your client (and don't want to use Patchwork's diff), let me repeat my 
previous offer:
> > If someone wants to apply the patches and has trouble with getting the patch
> > out of the mailclient I can on request resend the patch with a binary diff
> > for the reference file.

~~

On Wed, Feb 02, 2022 at 04:44:54 +, Soft Works wrote:
> > It CAN be applied (as I've now written twice) and
> > of course I verified this with the mail received from the list.
>
> I meant that it can be applied by everybody, including Patchwork
> and Michael.

Everybody but Patchwork can apply the mail and Patchwork's _diff_ works too.
In case any mail client acts up I already offered to resend the patch
with a binary flag for the reference file on request.

> I use git format-patch, just like many others and afaik it can't create base64
> encoded content.

It doesn't need to. When attaching a format-patch (as ffmpeg's documentation 
recommends when not using send-email) the _mail client_ is supposed to
choose and perform a suitable transfer encoding.
Depending on the mail client this can also work with the main message,
but some clients have trouble preserving everything in the main message.
(Because they assume the main body to be plain text and eg
 "normalise" line-ending, automatically break long lines, 
 remove or replace special characters, etc)

> BTW. BASE64 doesn't seem to be widely used here, [...]

Note, that I wrote send-email uses the appropriate transfer-encoding 
“automatically” not “always base64”; if there are no characters in the
message beyond US-ASCII (and mayhaps only LF line-endings), using base64
or quoted-printable is not necessary.
Whether or not mixed line-endings _require_ one of the latter two encodings
due to some SMTP limitation I do not know. It's certainly _possible_ to
preserve them this way though.
(And git send-email choose to use base64 for the second patch.)

> Interestingly, even the author of those lines is sending patches with
> Content-Transfer-Encoding: 7bit :-)

see previous.
(second patch contains bidi-marks and mixed line-endings, first neither)

> LOL - so the majority on this ML is sending patches incorrectly?

No, see second last.

___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-01 Thread Oneric
On Tue, Feb 01, 2022 at 20:41:37 +, Soft Works wrote:
> > On Tue, Feb 01, 2022 at 19:44:24 +, Soft Works wrote:
> > > > On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> > > >
> > > > In case anyone is wondering why patchwork fails to apply the second 
> > > > patch,
> > > > this is probably once again because the patch updates one of FATE's ASS
> > > > reference files which use CRLF line-endings.
> > > > Locally git am applies both without a hitch for me on top of current 
> > > > master
> > > > (and FATE passes after applying each patch).
> > >
> > >
> > > You can add a .gitattributes file to tests/ref/fate/ which includes the 
> > > line
> > >
> > > sub-webvtt2 -diff
> > >
> > > Then your local git format-patch will create a binary diff for the file.
> > 
> > Thanks for your suggestion. However, a binary diff would look like this 
> > which
> > isn't great for seeing what's going on during review:
> > 
> >   [...]
> 
> Yes, I know, but the question is probably what's more important..
> 
> ..that it can be applied or that the text is viewable?

It CAN be applied (as I've now written twice) and
of course I verified this with the mail received from the list.

> > Also as noted, locally plain `git am` has no issues applying the regular
> > (non-binary) patch; 
> 
> Yes, because it hasn't been sent around via e-mail yet. SMTP requires
> CRLF line endings, so essentially it depends on the receiving e-mail client
> whether it outputs LF or CRLF. [...]

The mail content is base64 encoded during client-server and server-server 
transfer. Perhaps the raw base64 text is using all CRLF line-endings, but 
this doesn't matter. Once decoded it will byte-for-byte match the format-patch 
I created locally except for the ffmpeg-devel footer and additional headers in 
the mail version — but git am will just ignore those.
See the headers of the second patch's mail:

  Content-Type: text/plain; charset="utf-8"
  Content-Transfer-Encoding: base64

For comparison, here are the same headers for your first reply:

  Content-Type: text/plain; charset="us-ascii"
  Content-Transfer-Encoding: 7bit

If you tried to send a patch with your mail client by pasting in the diff,
then yes, presumably the line-endings would indeed get mangled. That's why 
doc/developer.texi tells you not to do this (instead recommending
git send-email or binary (i.e. base64-encoded) attachments).
  
> The most extreme example is sub-textenc (and there was another one iirc).
> It has mixed line endings - some LF and some CRLF. At least at that point
> there's no way out, because those endings will get unrecoverably lost 
> as soon as it is sent around via e-mail as text-diff.
> 
> That's how I came to the binary diff as only working option.
> (or you just don't send patches around via e-mail at all ;-)

Mixed line-endings also aren't a problem with a base64-encoded mail body
as eg git send-email will automatically use.


There's one caveat specific to (ffmpeg's) patchwork
I didn't know about before:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220116181655.6407-2-one...@oneric.de/

If I download the “diff” from the above page, CRLF and LF line-endings are 
preserved as they should be. However, the files served from the “mbox” and 
“series” buttons only contain LF line-endings. As everyone can test, the mail 
I sent evidently does use mixed CRLF and LF line-endings matching the file 
being patched (just like the result of the “diff” button).
I believe this is an issue separate from patchwork using --no-keep-cr and 
should be fixed on patchwork's side.


Tl;dr:
Mixed line-endings in a diff are no technical issue,
provided the patch is sent correctly.
The issue is purely with patchwork, which:
 a) uses --no-keep-cr or similar presumably to protect against CRLF sneaking 
into regular source files. (This is a minor annoyance when CRLF is 
actually required, but otherwise harmless)
 b) mangles the line-endings for the “mbox” and “series” options but does the 
correct thing for the “diff” option. This is harmful and imho should be 
fixed by patchwork. Nonetheless, the patch can still be correctly applied 
from the _mail_ on the list.

The patch on the list can easily be applied from the received mail by a plain 
`git am` or `git am --keep-cr` if the value of am.keepcr is set to false.
(In my case am.keepcr is not set at all and plain git am does the right thing.)
If someone wants to apply the patches and has trouble with getting the patch 
out of the mailclient I can on request resend the patch with a binary diff for 
the reference file.
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-01 Thread Oneric
On Tue, Feb 01, 2022 at 19:44:24 +, Soft Works wrote:
> > On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> > 
> > In case anyone is wondering why patchwork fails to apply the second patch,
> > this is probably once again because the patch updates one of FATE's ASS
> > reference files which use CRLF line-endings.
> > Locally git am applies both without a hitch for me on top of current master
> > (and FATE passes after applying each patch).
> 
> 
> You can add a .gitattributes file to tests/ref/fate/ which includes the line
> 
> sub-webvtt2 -diff
> 
> Then your local git format-patch will create a binary diff for the file.

Thanks for your suggestion. However, a binary diff would look like this which 
isn't great for seeing what's going on during review:

  diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
  index 
357b8178ea1cf224ad47dcf78b24f1948ece6665..4cd1d86a9a58ccf65812131bf84a17531c2c6cfa
 100644
  GIT binary patch
  delta 24
  gcmeys^NnXiIV;bjhJHgMV-r)eM-6?GYgs=70DwpeRR910
  
  delta 18
  Zcmeyy^MPkWIV+o?k+F%X+2m%{

Also as noted, locally plain `git am` has no issues applying the regular 
(non-binary) patch; iirc patchwork uses some additional flags to make it more 
restrictive because regular codefiles shouldn't use CRLF line-endings.

I recall using some .gitattribute settings to force crlf (without making 
the file binary?) were discussed last time this happened, but deemed to be not 
worth it because of some other issues with it.

> 
> softworkz
___
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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-01 Thread Oneric
On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
>  libavcodec/ass.c   | 5 -
>  libavcodec/webvttdec.c | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)

> [PATCH 2/2] avcodec/webvttdec: honour bidi marks
>  libavcodec/webvttdec.c | 2 +-
>  tests/ref/fate/sub-webvtt2 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

ping.

In case anyone is wondering why patchwork fails to apply the second patch,
this is probably once again because the patch updates one of FATE's ASS
reference files which use CRLF line-endings.
Locally git am applies both without a hitch for me on top of current master
(and FATE passes after applying each patch).
___
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 2/2] avcodec/webvttdec: honour bidi marks

2022-01-16 Thread Oneric
WebVTT files are required to be encoded as UTF-8 by its spec,
so just insert the bytes for UTF-8 encoded bidi-marks.
---
 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 8cb739697a..7d996928eb 100644
--- a/libavcodec/webvttdec.c
+++ b/libavcodec/webvttdec.c
@@ -39,7 +39,7 @@ static const struct {
 {"", "{\\u1}"}, {"", "{\\u0}"},
 {"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
 {"", ">"}, {"", "<"},
-{"", ""}, {"", ""}, // FIXME: properly honor bidi marks
+{"", "\xe2\x80\x8e"}, {"", "\xe2\x80\x8f"},
 {"", "&"}, {"", "\\h"},
 };
 
diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
index 357b8178ea..4cd1d86a9a 100644
--- a/tests/ref/fate/sub-webvtt2
+++ b/tests/ref/fate/sub-webvtt2
@@ -20,6 +20,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.30.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-requ...@ffmpeg.org with subject "unsubscribe".


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

2022-01-16 Thread Oneric
Backslashes cannot be escaped by backslashes in any ASS renderer,
but unless followed by a few specific characters it is just printed
as a regular character. Insert a word-joiner character after a backslash
to break up the active sequences without changing the visual output.
Also the existing \{ and \} escapes are specific to libass only.
---
The  patch assumes UTF-8 encoding in ff_ass_bprint_text_event
(WebVTT requires UTF-8 per sepc). If we cannot assume a particular
encoding, please advise how to best insert a word-joiner character in 
the correct encoding.
---
 libavcodec/ass.c   | 5 -
 libavcodec/webvttdec.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 725e4d42ba..461e110ca4 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -157,8 +157,11 @@ 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);
+} else if (!keep_ass_markup && *p == '\\') {
+   // append word-joiner U+2060 as UTF-8 to break up sequences like \N
+   av_bprintf(buf, "\\\xe2\x81\xa0");
 
 /* some packets might end abruptly (no \0 at the end, like for example
  * in some cases of demuxing from a classic video container), some
diff --git a/libavcodec/webvttdec.c b/libavcodec/webvttdec.c
index 0093f328fa..8cb739697a 100644
--- a/libavcodec/webvttdec.c
+++ b/libavcodec/webvttdec.c
@@ -37,7 +37,7 @@ static const struct {
 {"", "{\\i1}"}, {"", "{\\i0}"},
 {"", "{\\b1}"}, {"", "{\\b0}"},
 {"", "{\\u1}"}, {"", "{\\u0}"},
-{"{", "\\{"}, {"}", "\\}"}, // escape to avoid ASS markup conflicts
+{"{", "\\{"}, {"}", "\\}"}, {"\\", "\\\xe2\x81\xa0"}, // escape to avoid 
ASS markup conflicts
 {"", ">"}, {"", "<"},
 {"", ""}, {"", ""}, // FIXME: properly honor bidi marks
 {"", "&"}, {"", "\\h"},
-- 
2.30.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: allow using embedded fonts

2021-05-30 Thread Oneric
On Sat, May 29, 2021 at 22:54:44 +0530, Gyan Doshi wrote:
> On 2021-05-29 22:14, Oneric wrote:
> > On Sat, May 29, 2021 at 11:53:24 +0530, Gyan Doshi wrote:
> > > Would it make sense to allow users to not load embedded fonts?
> >
> > I can't think of a reason not to load embedded fonts in ffmpeg; they are
> > required to render subs as intended by the sub author. Without them some
> > fallback font from the system or mkv-attachments will be used instead
> > (which if the requested font doesn't by chance happen to also be available
> >   on the system or as a container-attachment, will be some, more or
> >   less random font).
> > VLC always enables this unconditionally, but mplayer and mpv have an option
> > for this defaulting to embedded fonts enabled, but as before I don't know 
> > why
> > anyone would want to disable this.
> 
> Security reasons?

If one expects processing a font file to expose (relevantly) more
attack surface, or be more dangerous than the rest of untrusted ASS
files, than perhaps yes.
Otherwise, I at least can't think of something right now.

Cheers
Oneric
___
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] avfilter/vf_subtitles: allow using embedded fonts

2021-05-29 Thread Oneric
On Sat, May 29, 2021 at 11:53:24 +0530, Gyan Doshi wrote:
> On 2021-05-29 11:51, Gyan Doshi wrote:
> > On 2021-05-29 11:07, Gyan Doshi wrote:
> > > I'll test this and apply.
> > 
> > Pushed as 3300625c6f148455b08d641597d54b5be4c0f76a
> 
> Would it make sense to allow users to not load embedded fonts?

Thanks!
I can't think of a reason not to load embedded fonts in ffmpeg; they are 
required to render subs as intended by the sub author. Without them some 
fallback font from the system or mkv-attachments will be used instead
(which if the requested font doesn't by chance happen to also be available
 on the system or as a container-attachment, will be some, more or
 less random font).
VLC always enables this unconditionally, but mplayer and mpv have an option 
for this defaulting to embedded fonts enabled, but as before I don't know why
anyone would want to disable this.


Oneric
___
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] Ping: [PATCH] avfilter/vf_subtitles: allow using embedded fonts

2021-05-28 Thread Oneric
On Sun, May 02, 2021 at 23:02:02 +0200, Oneric wrote:
> ASS subtitles can have encoded fonts embedded into the subtitle file
> itself. Allow libass to load those, to render subs as intended.
> ---
>  libavfilter/vf_subtitles.c | 1 +
>  1 file changed, 1 insertion(+)

another ping
___
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] Ping: [PATCH] avfilter/vf_subtitles: allow using embedded fonts

2021-05-21 Thread Oneric
On Sun, May 02, 2021 at 23:02:02 +0200, Oneric wrote:
> ASS subtitles can have encoded fonts embedded into the subtitle file
> itself. Allow libass to load those, to render subs as intended.
> ---
>  libavfilter/vf_subtitles.c | 1 +
>  1 file changed, 1 insertion(+)

pinging again
___
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] Ping: [PATCH] avfilter/vf_subtitles: allow using embedded fonts

2021-05-13 Thread Oneric
On Sun, May 02, 2021 at 23:02:02 +0200, Oneric wrote:
> ASS subtitles can have encoded fonts embedded into the subtitle file
> itself. Allow libass to load those, to render subs as intended.
> ---
>  libavfilter/vf_subtitles.c | 1 +
>  1 file changed, 1 insertion(+)

pinging for review
___
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] avfilter/vf_subtitles: allow using embedded fonts

2021-05-02 Thread Oneric
ASS subtitles can have encoded fonts embedded into the subtitle file
itself. Allow libass to load those, to render subs as intended.
---


A sample file for ASS with embedded font can be at the following link.
If everything works, the 'A' will render as a quad; see image:
https://raw.githubusercontent.com/TheOneric/libass-regression-tests-tmp/master/regression/embedded-font/efont.ass
https://raw.githubusercontent.com/TheOneric/libass-regression-tests-tmp/master/regression/embedded-font/efont-1000.png

Prior to libass commit 1140b6b885c89d37eef13dc1f31f144e9a76a4d7, included
in libass release 0.15.1, a libass-bug would have prevented this from actually 
working.
To test this a recent libass is therefore required.
'ass_extract_fonts' itself however has been part of libass' API ever since its
first standalone release, so calling this does not break compatibility with
older versions and at worst just nothing happens.
Use eg the follwoing to check:
  ./ffmpeg -f lavfi -i "color=cyan:640x480:d=5" -vf "ass=efont.ass" -f matroska 
- | ./ffplay -autoexit -

---
 libavfilter/vf_subtitles.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 493eb5f424..ab32e1b7f3 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -111,6 +111,7 @@ static av_cold int init(AVFilterContext *ctx)
 ass_set_message_cb(ass->library, ass_log, ctx);
 
 ass_set_fonts_dir(ass->library, ass->fontsdir);
+ass_set_extract_fonts(ass->library, 1);
 
 ass->renderer = ass_renderer_init(ass->library);
 if (!ass->renderer) {
-- 
2.20.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 02/15] lavc/codec_desc: add additional JPEG and BMP MIME types

2020-09-09 Thread Oneric
On Wed, Sep 09, 2020 at 15:28:05 +0200, Andreas Rheinhardt wrote:
> Oneric:
> > On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
> >> -.mime_types= MT("image/jpeg"),
> >> +.mime_types= MT("image/jpeg", "image/jpg"),
> > 
> > "image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" 
> > is).
> > That ofc doesn't mean it isn't being used, but after a quick search I can't 
> > find a reference for "image/jpg" anywhere.
> > Did you encounter a program using "image/jpg"? If not it may be better to 
> > stick with "image/jpeg" only.
> > 
> The ff_id3v2_mime_tags array (removed in patch 14 of this series)
> contains this. It has been added intentionally in
> f704eb612ba589d83741e07bfbdf1cffb8cb, yet without giving any reason
> for it.
> 
> - Andreas

Ohh, I missed it being in ff_id3v2_mime_tags, thanks. In that case 
disregard my previous mail. It's probably a good idea to keep "image/jpg" to 
not break any files, as long as "image/jpeg" stays the preferred option.
___
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 02/15] lavc/codec_desc: add additional JPEG and BMP MIME types

2020-09-09 Thread Oneric
On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
> ---
>  libavcodec/codec_desc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 3dee640360..49a00ad264 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>  .name  = "mjpeg",
>  .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>  .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
> -.mime_types= MT("image/jpeg"),
> +.mime_types= MT("image/jpeg", "image/jpg"),

"image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" is).
That ofc doesn't mean it isn't being used, but after a quick search I can't 
find a reference for "image/jpg" anywhere.
Did you encounter a program using "image/jpg"? If not it may be better to 
stick with "image/jpeg" only.


>  .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>  },
>  {
> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>  .name  = "bmp",
>  .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>  .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> -.mime_types= MT("image/x-ms-bmp"),
> +.mime_types= MT("image/x-ms-bmp", "image/bmp"),
>  },
>  {
>  .id= AV_CODEC_ID_CSCD,
> -- 
> 2.27.0
> 
___
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] Documentation: proposed changes in the structure

2020-08-21 Thread Oneric
On Fri, Aug 21, 2020 at 14:46:53 +0200, Timo Rothenpieler wrote:
> Something like Markdown would come to mind. It should have all formatting
> tools one needs for documentation.

Imho ReStructuredText would be better suited for documentation than Markdown, 
while being similarly "text-like".
Unlike Markdown, RST has a clear spec, supports automatic table of contents 
and citations etc. While this might be a matter of preference, RST's tables 
and hyperlinks are also better than Markdown's imho.
I don't know how RST compares to texinfo though.

Current users of RST are Python and the Linux Kernel among others.

As for rendering support:
Rendering RST is supported by GitHub and afaik also GitLab. Gitea can support 
rendering rst, but it needs an external renderer and a bit of configuration.


(I'm mostly following the ML (-or some parts of it at least-) out of interest; 
 if you decide on Markdown that's fine by me.
 I just thought it might be worth to point out RST as an option.)
___
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] Project orientation

2020-07-07 Thread Oneric
On Sun 2020 Jul 5 20:01 +0300 Kieran Kunhya  wrote:
>
> Going back to the original point in hand.
> Many patches aren't getting reviewed and pushed any more.
> 
> In part this is because in 2020 whether we like it or not mailing
> lists are not the way to do Git based development.
> The kernel is the exception to the rule, as Linus says it has a whole
> load of grey-bearded system maintainers who are paid full time to work
> on it.
>
> For new contributors git send-email is annoying. For people wanting to
> push, the .mbox format is annoying, Gmail doesn't support it any more.
> And you can't get new contributors to start using CLI based email
> clients or run their own mail server, that's not going to happen.
> 
> A solution like Gitlab is the only way forward. It has worked well for
> dav1d, it can run regression tests on all platforms for all commits:
> https://code.videolan.org/videolan/dav1d
> 
> Merges are done with one push of a button. Yes, the branch sprawl is
> not great but it's better than now.
> It has inline patch reviews which are nice.
> 
> Whether we like it or not web interfaces are the way 95% of the world
> does Git and we have to move with the times.

As someone who recently only submitted a single, simple patch, I haven't seen 
mail-based dev as an annoyance, rather the contrary as this avoids the burden 
of creating yet another account on some website, that I'll have to manage.

There also already seems to be some automatic testing, as Patchwork send me an 
off-list mail about failing tests. (I did not set SAMPLES when running make 
fate locally as it was not mentioned clearly on the main "Contribute" site on 
the webpage. https://ffmpeg.org/developer.html .) It can probably be improved 
though.

What was frustating however, was the lack of any form of response to the patch 
for over a month, despite ml-pings.
On top of that the revised patch with updated FATE-samples had to change files 
with CRLF line endings that are not marked as such in .gitattributes. This 
prompted patchwork to be unable to apply the patch. Even though this is the 
only correct way to update these samples and left me concerned the patch might 
be being silently ignored because it fails on patchworks.

In my experience the review activity of a project is independend of its 
development workflow being mail or web based.


I also feel compelled to point out, that a switch to a web-based interface 
instead of plaintext emails can be a hurdle for devs with impaired vision, who 
might rely on customised mail interfaces, screen readers and/or braille 
displays as a Web-page is more complicated in that regard (especially with js).
While I myself am fortunate enough to not needing to rely on such a setup, I 
personally know devs (not working on ffmpeg afaik) that do.


Regards
Oneric
___
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] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-05-16 Thread Oneric
On Fri, Apr 17, 2020 at 03:35:44 +0200, one...@oneric.de wrote:
> On Fri, Apr 17, 2020 at 02:15:17 +0200, Oneric wrote:
> > ffmpeg does not set the 'ScaledBorderAndShadow' header for ASS
> > subtitles. This currently leads to inconsistent behaviour depending on the
> > renderer
>
> Attached is the revised patch also updating affected FATE samples.
>
> From 74d3f6bd0189b0f4922404fccbefe95e1f01093d Mon Sep 17 00:00:00 2001
> From: Oneric 
> Date: Fri, 17 Apr 2020 00:38:53 +0200
> Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow
>  [...]
>  25 files changed, 25 insertions(+)


ping 3
___
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] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-05-02 Thread Oneric
On Fri, Apr 17, 2020 at 03:35:44 +0200, one...@oneric.de wrote:
> On Fri, Apr 17, 2020 at 02:15:17 +0200, Oneric wrote:
> > ffmpeg does not set the 'ScaledBorderAndShadow' header for ASS
> > subtitles. This currently leads to inconsistent behaviour depending on the
> > renderer
> 
> Sorry for the inconvenience, I missed a warning about FATE only running on 
> asubset of samples. Attached is the revised patch also updating affected FATE 
> samples.
> 
> From 74d3f6bd0189b0f4922404fccbefe95e1f01093d Mon Sep 17 00:00:00 2001
> From: Oneric 
> Date: Fri, 17 Apr 2020 00:38:53 +0200
> Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow
>  [...]
>  25 files changed, 25 insertions(+)


ping 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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-04-28 Thread Oneric
On Fri, Apr 17, 2020 at 03:35:44 +0200, one...@oneric.de wrote:
> From 74d3f6bd0189b0f4922404fccbefe95e1f01093d Mon Sep 17 00:00:00 2001
> From: Oneric 
> Date: Fri, 17 Apr 2020 00:38:53 +0200
> Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow
>  […]
>  25 files changed, 25 insertions(+)

Ping for review.
If there is any preference on how to handle the *necessary* CRLF lines, or any 
other request regarding the patch, I'd be happy to make the necessary changes.

___
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] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-04-24 Thread Oneric
On Fri, Apr 17, 2020 at 03:35:44 +0200, one...@oneric.de wrote:
> From 74d3f6bd0189b0f4922404fccbefe95e1f01093d Mon Sep 17 00:00:00 2001
> From: Oneric 
> Date: Fri, 17 Apr 2020 00:38:53 +0200
> Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow
> 
> ---
>  libavcodec/ass.c | 1 +
>  tests/ref/fate/sub-aqtitle   | 1 +
>  tests/ref/fate/sub-cc| 1 +
>  tests/ref/fate/sub-cc-realtime   | 1 +
>  tests/ref/fate/sub-cc-scte20 | 1 +
>  tests/ref/fate/sub-charenc   | 1 +
>  tests/ref/fate/sub-jacosub   | 1 +
>  tests/ref/fate/sub-microdvd  | 1 +
>  tests/ref/fate/sub-movtext   | 1 +
>  tests/ref/fate/sub-mpl2  | 1 +
>  tests/ref/fate/sub-mpsub | 1 +
>  tests/ref/fate/sub-mpsub-frames  | 1 +
>  tests/ref/fate/sub-pjs   | 1 +
>  tests/ref/fate/sub-realtext  | 1 +
>  tests/ref/fate/sub-sami  | 1 +
>  tests/ref/fate/sub-sami2 | 1 +
>  tests/ref/fate/sub-scc   | 1 +
>  tests/ref/fate/sub-srt   | 1 +
>  tests/ref/fate/sub-srt-badsyntax | 1 +
>  tests/ref/fate/sub-stl   | 1 +
>  tests/ref/fate/sub-subviewer | 1 +
>  tests/ref/fate/sub-subviewer1| 1 +
>  tests/ref/fate/sub-vplayer   | 1 +
>  tests/ref/fate/sub-webvtt| 1 +
>  tests/ref/fate/sub-webvtt2   | 1 +
>  25 files changed, 25 insertions(+)

The updated test samples have CRLF line endings, which `git am` doesn't seem 
to like. `git apply` and `git commit` do not care about the line endings. To 
apply the patch it's possible to use `git am --keep-cr`.

Alternatively, it is possible to add these files with `eol=crlf` to 
.gitattributes, which will allow a plain `git am` to work with these files in 
the future, but this has the drawback that git will report the whole file 
as having been rewritten (the actual file doesn't change, but git's view 
of the file does).
Also, adding `eol=crlf` only works for samples that *only* use CRLF line 
endings. Should there be mixed line endings on purpose (doesn't seem to be the 
case here) adding `eol=crlf` might mess things up.
___
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] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-04-16 Thread oneric
On Fri, Apr 17, 2020 at 02:15:17 +0200, Oneric wrote:
> From 51deab727958c5d64ae526f67063cdf141a01d46 Mon Sep 17 00:00:00 2001
> From: Oneric 
> Date: Fri, 17 Apr 2020 00:38:53 +0200
> Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow

Sorry for the inconvenience, I missed a warning about FATE only running on 
asubset of samples. Attached is the revised patch also updating affected FATE 
samples.
Hope I didn't miss anything this time.
>From 74d3f6bd0189b0f4922404fccbefe95e1f01093d Mon Sep 17 00:00:00 2001
From: Oneric 
Date: Fri, 17 Apr 2020 00:38:53 +0200
Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow

---
 libavcodec/ass.c | 1 +
 tests/ref/fate/sub-aqtitle   | 1 +
 tests/ref/fate/sub-cc| 1 +
 tests/ref/fate/sub-cc-realtime   | 1 +
 tests/ref/fate/sub-cc-scte20 | 1 +
 tests/ref/fate/sub-charenc   | 1 +
 tests/ref/fate/sub-jacosub   | 1 +
 tests/ref/fate/sub-microdvd  | 1 +
 tests/ref/fate/sub-movtext   | 1 +
 tests/ref/fate/sub-mpl2  | 1 +
 tests/ref/fate/sub-mpsub | 1 +
 tests/ref/fate/sub-mpsub-frames  | 1 +
 tests/ref/fate/sub-pjs   | 1 +
 tests/ref/fate/sub-realtext  | 1 +
 tests/ref/fate/sub-sami  | 1 +
 tests/ref/fate/sub-sami2 | 1 +
 tests/ref/fate/sub-scc   | 1 +
 tests/ref/fate/sub-srt   | 1 +
 tests/ref/fate/sub-srt-badsyntax | 1 +
 tests/ref/fate/sub-stl   | 1 +
 tests/ref/fate/sub-subviewer | 1 +
 tests/ref/fate/sub-subviewer1| 1 +
 tests/ref/fate/sub-vplayer   | 1 +
 tests/ref/fate/sub-webvtt| 1 +
 tests/ref/fate/sub-webvtt2   | 1 +
 25 files changed, 25 insertions(+)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 7c26e3fd6d..627741936a 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -40,6 +40,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "ScriptType: v4.00+\r\n"
  "PlayResX: %d\r\n"
  "PlayResY: %d\r\n"
+ "ScaledBorderAndShadow: yes\r\n"
  "\r\n"
  "[V4+ Styles]\r\n"
 
diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
index 87253c9a2d..af0c06d7c2 100644
--- a/tests/ref/fate/sub-aqtitle
+++ b/tests/ref/fate/sub-aqtitle
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
+ScaledBorderAndShadow: yes
 
 [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
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 4cc02d1d17..2b30a35be0 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
+ScaledBorderAndShadow: yes
 
 [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
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index be800a4d29..5a95ff5cb7 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
+ScaledBorderAndShadow: yes
 
 [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
diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
index 71fc92bfc5..be28084887 100644
--- a/tests/ref/fate/sub-cc-scte20
+++ b/tests/ref/fate/sub-cc-scte20
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
+ScaledBorderAndShadow: yes
 
 [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
diff --git a/tests/ref/fate/sub-charenc b/tests/ref/fate/sub-charenc
index a056cd1092..4efacb073d 100644
--- a/tests/ref/fate/sub-charenc
+++ b/tests/ref/fate/sub-charenc
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayResX: 384
 PlayResY: 288
+ScaledBorderAndShadow: yes
 
 [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
diff --git a/tests/ref/fate/sub-jacosub b/tests/ref/fate/sub-jacosub
index 5f282cdcf6..b574dda54d 100644
--- a/tests/ref/fate/sub-jacosub
+++ b/tests/ref/fate/sub-jacosub
@@ -3,6 +3,7 @@
 ScriptType: v4.00+
 PlayR

[FFmpeg-devel] [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow

2020-04-16 Thread Oneric
Patch attached.

Currently ffmpeg does not set the 'ScaledBorderAndShadow' header for ASS 
subtitles. This currently leads to inconsistent behaviour depending on the 
renderer(see https://github.com/libass/libass/issues/287#issuecomment-338654103)

Although libass will probably change the default to match *VSFilter soon, it is 
imho still a good idea to set this explicitly. Scaling the fontsize but not the
bordersize with the viewport is inconsistent and probably confusing.
Explicitly setting this header to "yes" is also the default in Aegisub (ASS 
authoring tool) since 2007.
https://github.com/TypesettingTools/Aegisub/commit/5269a2e2a1e1dab7aaf4ddde4055c8637cd413d4
>From 51deab727958c5d64ae526f67063cdf141a01d46 Mon Sep 17 00:00:00 2001
From: Oneric 
Date: Fri, 17 Apr 2020 00:38:53 +0200
Subject: [PATCH] avcodec/ass: explicitly set ScaledBorderAndShadow

---
 libavcodec/ass.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 7c26e3fd6d..627741936a 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -40,6 +40,7 @@ int ff_ass_subtitle_header_full(AVCodecContext *avctx,
  "ScriptType: v4.00+\r\n"
  "PlayResX: %d\r\n"
  "PlayResY: %d\r\n"
+ "ScaledBorderAndShadow: yes\r\n"
  "\r\n"
  "[V4+ Styles]\r\n"
 
-- 
2.20.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-requ...@ffmpeg.org with subject "unsubscribe".