sd_add: add terrible hack for (xy-)vsfilter compatibility

Much has been said about this topic, we don't need to say even more.

See additions to options.rst.

8 files changed, 177 insertions, 9 deletions

diff --git a/DOCS/man/en/options.rst b/DOCS/man/en/options.rst
index 66cdadd120..ad463daffc 100644
--- a/DOCS/man/en/options.rst
+++ b/DOCS/man/en/options.rst
@@ -174,6 +174,31 @@
 
     Enabled by default.
 
+``--ass-vsfilter-color-compat=<basic|full|force-601|no``
+    Mangle colors like (xy-)vsfilter do (default: basic). Historically, VSFilter
+    was not colorspace aware. This was no problem as long as the colorspace
+    used for SD video (BT.601) was used. But when everything switched to HD
+    (BT.709), VSFilter was still converting RGB colors to BT.601, rendered
+    them into the video frame, and handled the frame to the video output, which
+    would use BT.709 for conversion to RGB. The result were mangled subtitle
+    colors. Later on, bad hacks were added on top of the ASS format to control
+    how colors are to be mangled.
+
+    :basic: Handle only BT.601->BT.709 mangling, if the subtitles seem to
+            indicate that this is required (default).
+    :full:  Handle the full ``YCbCr Matrix`` header with all video colorspaces
+            supported by libass and mpv. This might lead to bad breakages in
+            corner cases and is not strictly needed for compatibility
+            (hopefully), which is why this is not default.
+    :force-601: Force BT.601->BT.709 mangling, regardless of subtitle headers
+            or video colorspace.
+    :no:    Disable color mangling completely. All colors are RGB.
+
+    Choosing anything other than ``no`` will make the subtitle color depend on
+    the video colorspace, and it's for example in theory not possible to reuse
+    a subtitle script with another video file. The ``--ass-style-override``
+    option doesn't affect how this option is interpreted.
+
 ``--audio-demuxer=<[+]name>``
     Use this audio demuxer type when using ``--audiofile``. Use a '+' before the
     name to force it; this will skip some checks. Give the demuxer name as
diff --git a/core/mplayer.c b/core/mplayer.c
index 95a31aa15b..dafa68172b 100644
--- a/core/mplayer.c
+++ b/core/mplayer.c
@@ -1746,6 +1746,12 @@ static void update_subtitles(struct MPContext *mpctx, double refpts_tl)
     assert(track && sh_sub);
     struct dec_sub *dec_sub = sh_sub->dec_sub;
 
+    if (mpctx->sh_video) {
+        struct mp_image_params params;
+        if (get_video_params(mpctx->sh_video, &params) > 0)
+            sub_control(dec_sub, SD_CTRL_SET_VIDEO_PARAMS, &params);
+    }
+
     mpctx->osd->video_offset = track->under_timeline ? mpctx->video_offset : 0;
 
     double refpts_s = refpts_tl - mpctx->osd->video_offset;
diff --git a/core/options.c b/core/options.c
index c28817db68..09b2f4aad6 100644
--- a/core/options.c
+++ b/core/options.c
@@ -500,6 +500,8 @@ const m_option_t mp_opts[] = {
     OPT_FLOATRANGE("ass-line-spacing", ass_line_spacing, 0, -1000, 1000),
     OPT_FLAG("ass-use-margins", ass_use_margins, 0),
     OPT_FLAG("ass-vsfilter-aspect-compat", ass_vsfilter_aspect_compat, 0),
+    OPT_CHOICE("ass-vsfilter-color-compat", ass_vsfilter_color_compat, 0,
+               ({"no", 0}, {"basic", 1}, {"full", 2}, {"force-601", 3})),
     OPT_FLAG("embeddedfonts", use_embedded_fonts, 0),
     OPT_STRINGLIST("ass-force-style", ass_force_style_list, 0),
     OPT_STRING("ass-styles", ass_styles_file, 0),
@@ -791,6 +793,7 @@ const struct MPOpts mp_default_opts = {
 #endif
     .sub_scale = 1,
     .ass_vsfilter_aspect_compat = 1,
+    .ass_vsfilter_color_compat = 1,
     .ass_style_override = 1,
     .use_embedded_fonts = 1,
     .suboverlap_enabled = 0,
diff --git a/core/options.h b/core/options.h
index 1088ee78c1..cfc786262e 100644
--- a/core/options.h
+++ b/core/options.h
@@ -190,6 +190,7 @@ typedef struct MPOpts {
     float ass_line_spacing;
     int ass_use_margins;
     int ass_vsfilter_aspect_compat;
+    int ass_vsfilter_color_compat;
     int use_embedded_fonts;
     char **ass_force_style_list;
     char *ass_styles_file;
diff --git a/sub/dec_sub.h b/sub/dec_sub.h
index 3fb35deace..b269326cac 100644
--- a/sub/dec_sub.h
+++ b/sub/dec_sub.h
@@ -18,6 +18,7 @@ struct sd;
 
 enum sd_ctrl {
     SD_CTRL_SUB_STEP,
+    SD_CTRL_SET_VIDEO_PARAMS,
 };
 
 struct dec_sub *sub_create(struct MPOpts *opts);
diff --git a/sub/sd_ass.c b/sub/sd_ass.c
index 0200dd4e68..8091a163b4 100644
--- a/sub/sd_ass.c
+++ b/sub/sd_ass.c
@@ -27,6 +27,8 @@
 #include "core/options.h"
 #include "core/mp_common.h"
 #include "core/mp_msg.h"
+#include "video/csputils.h"
+#include "video/mp_image.h"
 #include "sub.h"
 #include "dec_sub.h"
 #include "ass_mp.h"
@@ -38,13 +40,17 @@
 
 struct sd_ass_priv {
     struct ass_track *ass_track;
-    bool vsfilter_aspect;
+    bool is_converted;
     bool incomplete_event;
     struct sub_bitmap *parts;
     bool flush_on_seek;
     char last_text[500];
+    struct mp_image_params video_params;
+    struct mp_image_params last_params;
 };
 
+static void mangle_colors(struct sd *sd, struct sub_bitmaps *parts);
+
 static bool supports_format(const char *format)
 {
     // ass-text is produced by converters and the subreader.c ssa parser; this
@@ -68,15 +74,16 @@ static int init(struct sd *sd)
     if (!sd->ass_library || !sd->ass_renderer || !sd->codec)
         return -1;
 
-    bool is_converted = sd->converted_from != NULL;
-
     struct sd_ass_priv *ctx = talloc_zero(NULL, struct sd_ass_priv);
     sd->priv = ctx;
+
+    ctx->is_converted = sd->converted_from != NULL;
+
     if (sd->ass_track) {
         ctx->ass_track = sd->ass_track;
     } else {
         ctx->ass_track = ass_new_track(sd->ass_library);
-        if (!is_converted)
+        if (!ctx->is_converted)
             ctx->ass_track->track_type = TRACK_TYPE_ASS;
     }
 
@@ -87,7 +94,6 @@ static int init(struct sd *sd)
 
     mp_ass_add_default_styles(ctx->ass_track, opts);
 
-    ctx->vsfilter_aspect = !is_converted;
     return 0;
 }
 
@@ -181,17 +187,21 @@ static void get_bitmaps(struct sd *sd, struct mp_osd_res dim, double pts,
     if (pts == MP_NOPTS_VALUE || !sd->ass_renderer)
         return;
 
+    bool use_vs_aspect = !ctx->is_converted &&
+        opts->ass_style_override ? opts->ass_vsfilter_aspect_compat : 1;
+
+    ASS_Renderer *renderer = sd->ass_renderer;
     double scale = dim.display_par;
-    bool use_vs_aspect = opts->ass_style_override
-                         ? opts->ass_vsfilter_aspect_compat : 1;
-    if (ctx->vsfilter_aspect && use_vs_aspect)
+    if (use_vs_aspect)
         scale = scale * dim.video_par;
-    ASS_Renderer *renderer = sd->ass_renderer;
     mp_ass_configure(renderer, opts, &dim);
     ass_set_aspect_ratio(renderer, scale, 1);
     mp_ass_render_frame(renderer, ctx->ass_track, pts * 1000 + .5,
                         &ctx->parts, res);
     talloc_steal(ctx, ctx->parts);
+
+    if (!ctx->is_converted)
+        mangle_colors(sd, res);
 }
 
 struct buf {
@@ -325,6 +335,9 @@ static int control(struct sd *sd, enum sd_ctrl cmd, void *arg)
         double *a = arg;
         a[0] = ass_step_sub(ctx->ass_track, a[0] * 1000 + .5, a[1]) / 1000.0;
         return CONTROL_OK;
+    case SD_CTRL_SET_VIDEO_PARAMS:
+        ctx->video_params = *(struct mp_image_params *)arg;
+        return CONTROL_OK;
     }
     default:
         return CONTROL_UNKNOWN;
@@ -344,3 +357,113 @@ const struct sd_functions sd_ass = {
     .reset = reset,
     .uninit = uninit,
 };
+
+// Disgusting hack for (xy-)vsfilter color compatibility.
+static void mangle_colors(struct sd *sd, struct sub_bitmaps *parts)
+{
+    struct MPOpts *opts = sd->opts;
+    struct sd_ass_priv *ctx = sd->priv;
+    ASS_Track *track = ctx->ass_track;
+    enum mp_csp csp = 0;
+    enum mp_csp_levels levels = 0;
+    if (opts->ass_vsfilter_color_compat == 0) // "no"
+        return;
+    bool force_601 = opts->ass_vsfilter_color_compat == 3;
+#if LIBASS_VERSION >= 0x01020000
+    static const int ass_csp[] = {
+        [YCBCR_BT601_TV]        = MP_CSP_BT_601,
+        [YCBCR_BT601_PC]        = MP_CSP_BT_601,
+        [YCBCR_BT709_TV]        = MP_CSP_BT_709,
+        [YCBCR_BT709_PC]        = MP_CSP_BT_709,
+        [YCBCR_SMPTE240M_TV]    = MP_CSP_SMPTE_240M,
+        [YCBCR_SMPTE240M_PC]    = MP_CSP_SMPTE_240M,
+    };
+    static const int ass_levels[] = {
+        [YCBCR_BT601_TV]        = MP_CSP_LEVELS_TV,
+        [YCBCR_BT601_PC]        = MP_CSP_LEVELS_PC,
+        [YCBCR_BT709_TV]        = MP_CSP_LEVELS_TV,
+        [YCBCR_BT709_PC]        = MP_CSP_LEVELS_PC,
+        [YCBCR_SMPTE240M_TV]    = MP_CSP_LEVELS_TV,
+        [YCBCR_SMPTE240M_PC]    = MP_CSP_LEVELS_PC,
+    };
+    int trackcsp = track->YCbCrMatrix;
+    if (force_601)
+        trackcsp = YCBCR_BT601_TV;
+    // NONE is a bit random, but the intention is: don't modify colors.
+    if (trackcsp == YCBCR_NONE)
+        return;
+    if (trackcsp < sizeof(ass_csp) / sizeof(ass_csp[0]))
+        csp = ass_csp[trackcsp];
+    if (trackcsp < sizeof(ass_levels) / sizeof(ass_levels[0]))
+        levels = ass_levels[trackcsp];
+    if (trackcsp == YCBCR_DEFAULT) {
+        csp = MP_CSP_BT_601;
+        levels = MP_CSP_LEVELS_TV;
+    }
+    // Unknown colorspace (either YCBCR_UNKNOWN, or a valid value unknown to us)
+    if (!csp || !levels)
+        return;
+#endif
+
+    struct mp_image_params params = ctx->video_params;
+
+    if (force_601) {
+        params.colorspace = MP_CSP_BT_709;
+        params.colorlevels = MP_CSP_LEVELS_TV;
+    }
+
+    if (csp == params.colorspace && levels == params.colorlevels)
+        return;
+
+    bool basic_conv = params.colorspace == MP_CSP_BT_709 &&
+                      params.colorlevels == MP_CSP_LEVELS_TV &&
+                      csp == MP_CSP_BT_601 &&
+                      levels == MP_CSP_LEVELS_TV;
+
+    // With "basic", only do as much as needed for basic compatibility.
+    if (opts->ass_vsfilter_color_compat == 1 && !basic_conv)
+        return;
+
+    if (params.colorspace != ctx->last_params.colorspace ||
+        params.colorlevels != ctx->last_params.colorlevels)
+    {
+        int msgl = basic_conv ? MSGL_V : MSGL_WARN;
+        ctx->last_params = params;
+        mp_msg(MSGT_SUBREADER, msgl, "[sd_ass] mangling colors like vsfilter: "
+               "RGB -> %s %s -> %s %s -> RGB\n", mp_csp_names[csp],
+               mp_csp_levels_names[levels], mp_csp_names[params.colorspace],
+               mp_csp_levels_names[params.colorlevels]);
+    }
+
+    // Conversion that VSFilter would use
+    struct mp_csp_params vs_params = MP_CSP_PARAMS_DEFAULTS;
+    vs_params.colorspace.format = csp;
+    vs_params.colorspace.levels_in = levels;
+    vs_params.int_bits_in = 8;
+    vs_params.int_bits_out = 8;
+    float vs_yuv2rgb[3][4], vs_rgb2yuv[3][4];
+    mp_get_yuv2rgb_coeffs(&vs_params, vs_yuv2rgb);
+    mp_invert_yuv2rgb(vs_rgb2yuv, vs_yuv2rgb);
+
+    // Proper conversion to RGB
+    struct mp_csp_params rgb_params = MP_CSP_PARAMS_DEFAULTS;
+    rgb_params.colorspace.format = params.colorspace;
+    rgb_params.colorspace.levels_in = params.colorlevels;
+    rgb_params.int_bits_in = 8;
+    rgb_params.int_bits_out = 8;
+    float vs2rgb[3][4];
+    mp_get_yuv2rgb_coeffs(&rgb_params, vs2rgb);
+
+    for (int n = 0; n < parts->num_parts; n++) {
+        struct sub_bitmap *sb = &parts->parts[n];
+        uint32_t color = sb->libass.color;
+        int r = (color >> 24u) & 0xff;
+        int g = (color >> 16u) & 0xff;
+        int b = (color >>  8u) & 0xff;
+        int a = color & 0xff;
+        int c[3] = {r, g, b};
+        mp_map_int_color(vs_rgb2yuv, 8, c);
+        mp_map_int_color(vs2rgb, 8, c);
+        sb->libass.color = (c[0] << 24u) | (c[1] << 16) | (c[2] << 8) | a;
+    }
+}
diff --git a/video/csputils.c b/video/csputils.c
index 8face3164c..f25d20e517 100644
--- a/video/csputils.c
+++ b/video/csputils.c
@@ -45,6 +45,12 @@ char * const mp_csp_names[MP_CSP_COUNT] = {
     "YCgCo",
 };
 
+char * const mp_csp_levels_names[MP_CSP_LEVELS_COUNT] = {
+    "Autoselect",
+    "TV",
+    "PC",
+};
+
 char * const mp_csp_equalizer_names[MP_CSP_EQ_COUNT] = {
     "brightness",
     "contrast",
diff --git a/video/csputils.h b/video/csputils.h
index 7c528d1b86..8c9806a1a3 100644
--- a/video/csputils.h
+++ b/video/csputils.h
@@ -53,6 +53,9 @@ enum mp_csp_levels {
     MP_CSP_LEVELS_COUNT,
 };
 
+// Any enum mp_csp_levels value is a valid index (except MP_CSP_LEVELS_COUNT)
+extern char * const mp_csp_levels_names[MP_CSP_LEVELS_COUNT];
+
 struct mp_csp_details {
     enum mp_csp format;
     enum mp_csp_levels levels_in;      // encoded video