diff mbox series

[FFmpeg-devel] Make font shaping a shared subtitle filter option

Message ID a7948e77-c3d2-2d6e-9130-aeb161ae7968@gmail.com
State New
Headers show
Series [FFmpeg-devel] Make font shaping a shared subtitle filter option | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

yorwba March 18, 2021, 6:40 p.m. UTC
Commit 4b58349bc8ff2ff5dfbc9eef1e5856fd16e1f517 introduced a new option
for selecting the font shaping engine used for rendering ASS subtitles.
However, the shaping value was used in code shared with the ordinary
subtitles filter (config_input in vf_subtitles.c), where it is left
uninitialized, as opposed to defaulting to -1. This means that the
subtitles filter is forced to use the "simple" shaping engine instead of
automatically selecting the best available option. As a result, Indic
scripts requiring "complex" shaping are not rendered correctly. This was
reported as bug #8738. (Aside: Thanks to cehoyos for pointing to the
responsible commit on the bug tracker.)

Because I don't see a reason why font shaping should be an ASS-only
option (especially considering the implementation was already shared),
I moved it into the common options.

Signed-off-by: Yorwba <yorwb4@gmail.com>
---
 doc/filters.texi           | 35 +++++++++++++++--------------------
 libavfilter/vf_subtitles.c |  8 ++++----
 2 files changed, 19 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index 30dee5531d..36a9e7613a 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -7099,26 +7099,6 @@  Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
 and libavformat to work. On the other hand, it is limited to ASS (Advanced
 Substation Alpha) subtitles files.
 
-This filter accepts the following option in addition to the common options from
-the @ref{subtitles} filter:
-
-@table @option
-@item shaping
-Set the shaping engine
-
-Available values are:
-@table @samp
-@item auto
-The default libass shaping engine, which is the best available.
-@item simple
-Fast, font-agnostic shaper that can do only substitutions
-@item complex
-Slower shaper using OpenType for substitutions and positioning
-@end table
-
-The default is @code{auto}.
-@end table
-
 @section atadenoise
 Apply an Adaptive Temporal Averaging Denoiser to the video input.
 
@@ -19227,6 +19207,21 @@  These fonts will be used in addition to whatever the font provider uses.
 @item alpha
 Process alpha channel, by default alpha channel is untouched.
 
+@item shaping
+Set the shaping engine
+
+Available values are:
+@table @samp
+@item auto
+The default libass shaping engine, which is the best available.
+@item simple
+Fast, font-agnostic shaper that can do only substitutions
+@item complex
+Slower shaper using OpenType for substitutions and positioning
+@end table
+
+The default is @code{auto}.
+
 @item charenc
 Set subtitles input character encoding. @code{subtitles} filter only. Only
 useful if not UTF-8.
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index de74afa2b7..1c10bd6aad 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -71,6 +71,10 @@  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 }, \
+    {"shaping",        "set shaping engine",                                       OFFSET(shaping),    AV_OPT_TYPE_INT,        {.i64 = -1  }, -1, 1, FLAGS, "shaping_mode"}, \
+        {"auto",    NULL,              0, AV_OPT_TYPE_CONST, {.i64 = -1},                  INT_MIN, INT_MAX, FLAGS, "shaping_mode"}, \
+        {"simple",  "simple shaping",  0, AV_OPT_TYPE_CONST, {.i64 = ASS_SHAPING_SIMPLE},  INT_MIN, INT_MAX, FLAGS, "shaping_mode"}, \
+        {"complex", "complex shaping", 0, AV_OPT_TYPE_CONST, {.i64 = ASS_SHAPING_COMPLEX}, INT_MIN, INT_MAX, FLAGS, "shaping_mode"}, \
 
 /* libass supports a log level ranging from 0 to 7 */
 static const int ass_libavfilter_log_level_map[] = {
@@ -216,10 +220,6 @@  static const AVFilterPad ass_outputs[] = {
 
 static const AVOption ass_options[] = {
     COMMON_OPTIONS
-    {"shaping", "set shaping engine", OFFSET(shaping), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 1, FLAGS, "shaping_mode"},
-        {"auto", NULL,                 0, AV_OPT_TYPE_CONST, {.i64 = -1},                  INT_MIN, INT_MAX, FLAGS, "shaping_mode"},
-        {"simple",  "simple shaping",  0, AV_OPT_TYPE_CONST, {.i64 = ASS_SHAPING_SIMPLE},  INT_MIN, INT_MAX, FLAGS, "shaping_mode"},
-        {"complex", "complex shaping", 0, AV_OPT_TYPE_CONST, {.i64 = ASS_SHAPING_COMPLEX}, INT_MIN, INT_MAX, FLAGS, "shaping_mode"},
     {NULL},
 };