player: refactor secondary subtitle options and properties

Over the years, we've accumulated several secondary subtitle related
options and properties, but the implementation was not really consistent
and it wasn't clear what the right process for adding more should be. So
to make things nicer, let's refactor all of the subtitle options with
secondary variants (sub-delay, sub-pos, and sub-visibility) and split
them off to a new, separate struct. All of the underlying values are
stored in an array instead for simplicity. Additionally, the
implementation of some secondary-sub-* properties were slightly changed
so there would be less redundancy.

9 files changed, 69 insertions, 62 deletions

diff --git a/options/options.c b/options/options.c
index 8fb566c6f6..401eda919d 100644
--- a/options/options.c
+++ b/options/options.c
@@ -283,20 +283,14 @@ const struct m_sub_options mp_sub_filter_opts = {
 
 const struct m_sub_options mp_subtitle_sub_opts = {
     .opts = (const struct m_option[]){
-        {"sub-delay", OPT_FLOAT(sub_delay[0])},
-        {"secondary-sub-delay", OPT_FLOAT(sub_delay[1])},
         {"sub-fps", OPT_FLOAT(sub_fps)},
         {"sub-speed", OPT_FLOAT(sub_speed)},
-        {"sub-visibility", OPT_BOOL(sub_visibility)},
-        {"secondary-sub-visibility", OPT_BOOL(sec_sub_visibility)},
         {"sub-forced-events-only", OPT_BOOL(sub_forced_events_only)},
         {"stretch-dvd-subs", OPT_BOOL(stretch_dvd_subs)},
         {"stretch-image-subs-to-screen", OPT_BOOL(stretch_image_subs)},
         {"image-subs-video-resolution", OPT_BOOL(image_subs_video_res)},
         {"sub-fix-timing", OPT_BOOL(sub_fix_timing)},
         {"sub-stretch-durations", OPT_BOOL(sub_stretch_durations)},
-        {"sub-pos", OPT_FLOAT(sub_pos), M_RANGE(0.0, 150.0)},
-        {"secondary-sub-pos", OPT_FLOAT(sec_sub_pos), M_RANGE(0.0, 150.0)},
         {"sub-gauss", OPT_FLOAT(sub_gauss), M_RANGE(0.0, 3.0)},
         {"sub-gray", OPT_BOOL(sub_gray)},
         {"sub-ass", OPT_BOOL(ass_enabled), .flags = UPDATE_SUB_HARD},
@@ -334,10 +328,6 @@ const struct m_sub_options mp_subtitle_sub_opts = {
     },
     .size = sizeof(OPT_BASE_STRUCT),
     .defaults = &(OPT_BASE_STRUCT){
-        .sub_visibility = true,
-        .sec_sub_visibility = true,
-        .sub_pos = 100,
-        .sec_sub_pos = 0,
         .sub_speed = 1.0,
         .ass_enabled = true,
         .sub_scale_by_window = true,
@@ -356,6 +346,28 @@ const struct m_sub_options mp_subtitle_sub_opts = {
 };
 
 #undef OPT_BASE_STRUCT
+#define OPT_BASE_STRUCT struct mp_subtitle_shared_opts
+
+const struct m_sub_options mp_subtitle_shared_sub_opts = {
+    .opts = (const struct m_option[]){
+        {"sub-delay", OPT_FLOAT(sub_delay[0])},
+        {"secondary-sub-delay", OPT_FLOAT(sub_delay[1])},
+        {"sub-pos", OPT_FLOAT(sub_pos[0]), M_RANGE(0.0, 150.0)},
+        {"secondary-sub-pos", OPT_FLOAT(sub_pos[1]), M_RANGE(0.0, 150.0)},
+        {"sub-visibility", OPT_BOOL(sub_visibility[0])},
+        {"secondary-sub-visibility", OPT_BOOL(sub_visibility[1])},
+        {0}
+    },
+    .size = sizeof(OPT_BASE_STRUCT),
+    .defaults = &(OPT_BASE_STRUCT){
+        .sub_visibility[0] = true,
+        .sub_visibility[1] = true,
+        .sub_pos[0] = 100,
+    },
+    .change_flags = UPDATE_OSD,
+};
+
+#undef OPT_BASE_STRUCT
 #define OPT_BASE_STRUCT struct mp_osd_render_opts
 
 const struct m_sub_options mp_osd_render_sub_opts = {
@@ -669,6 +681,7 @@ static const m_option_t mp_opts[] = {
     {"cover-art-whitelist", OPT_BOOL(coverart_whitelist)},
 
     {"", OPT_SUBSTRUCT(subs_rend, mp_subtitle_sub_opts)},
+    {"", OPT_SUBSTRUCT(subs_shared, mp_subtitle_shared_sub_opts)},
     {"", OPT_SUBSTRUCT(subs_filt, mp_sub_filter_opts)},
     {"", OPT_SUBSTRUCT(osd_rend, mp_osd_render_sub_opts)},
 
diff --git a/options/options.h b/options/options.h
index c0ffd2c325..86344c52f6 100644
--- a/options/options.h
+++ b/options/options.h
@@ -82,11 +82,6 @@ typedef struct mp_vo_opts {
 
 // Subtitle options needed by the subtitle decoders/renderers.
 struct mp_subtitle_opts {
-    bool sub_visibility;
-    bool sec_sub_visibility;
-    float sub_pos;
-    float sec_sub_pos;
-    float sub_delay[2];
     float sub_fps;
     float sub_speed;
     bool sub_forced_events_only;
@@ -121,6 +116,13 @@ struct mp_subtitle_opts {
     bool sub_past_video_end;
 };
 
+// Options for both primary and secondary subs.
+struct mp_subtitle_shared_opts {
+    float sub_delay[2];
+    float sub_pos[2];
+    bool sub_visibility[2];
+};
+
 struct mp_sub_filter_opts {
     bool sub_filter_SDH;
     bool sub_filter_SDH_harder;
@@ -199,6 +201,7 @@ typedef struct MPOpts {
     bool cursor_autohide_fs;
 
     struct mp_subtitle_opts *subs_rend;
+    struct mp_subtitle_shared_opts *subs_shared;
     struct mp_sub_filter_opts *subs_filt;
     struct mp_osd_render_opts *osd_rend;
 
@@ -398,6 +401,7 @@ extern const struct m_sub_options vo_sub_opts;
 extern const struct m_sub_options cuda_conf;
 extern const struct m_sub_options dvd_conf;
 extern const struct m_sub_options mp_subtitle_sub_opts;
+extern const struct m_sub_options mp_subtitle_shared_sub_opts;
 extern const struct m_sub_options mp_sub_filter_opts;
 extern const struct m_sub_options mp_osd_render_sub_opts;
 extern const struct m_sub_options filter_conf;
diff --git a/player/command.c b/player/command.c
index ea78328c77..4e29c8693c 100644
--- a/player/command.c
+++ b/player/command.c
@@ -2878,7 +2878,7 @@ static int mp_property_sub_delay(void *ctx, struct m_property *prop,
     int track_ind = *(int *)prop->priv;
     switch (action) {
     case M_PROPERTY_PRINT:
-        *(char **)arg = format_delay(opts->subs_rend->sub_delay[track_ind]);
+        *(char **)arg = format_delay(opts->subs_shared->sub_delay[track_ind]);
         return M_PROPERTY_OK;
     }
     return mp_property_generic_option(mpctx, prop, action, arg);
@@ -2903,20 +2903,9 @@ static int mp_property_sub_pos(void *ctx, struct m_property *prop,
 {
     MPContext *mpctx = ctx;
     struct MPOpts *opts = mpctx->opts;
+    int track_ind = *(int *)prop->priv;
     if (action == M_PROPERTY_PRINT) {
-        *(char **)arg = talloc_asprintf(NULL, "%4.2f%%/100", opts->subs_rend->sub_pos);
-        return M_PROPERTY_OK;
-    }
-    return mp_property_generic_option(mpctx, prop, action, arg);
-}
-
-static int mp_property_secondary_sub_pos(void *ctx, struct m_property *prop,
-                               int action, void *arg)
-{
-    MPContext *mpctx = ctx;
-    struct MPOpts *opts = mpctx->opts;
-    if (action == M_PROPERTY_PRINT) {
-        *(char **)arg = talloc_asprintf(NULL, "%4.2f%%/100", opts->subs_rend->sec_sub_pos);
+        *(char **)arg = talloc_asprintf(NULL, "%4.2f%%/100", opts->subs_shared->sub_pos[track_ind]);
         return M_PROPERTY_OK;
     }
     return mp_property_generic_option(mpctx, prop, action, arg);
@@ -2945,11 +2934,14 @@ static int mp_property_sub_ass_extradata(void *ctx, struct m_property *prop,
     return M_PROPERTY_NOT_IMPLEMENTED;
 }
 
-static int get_sub_text(void *ctx, struct m_property *prop,
-                        int action, void *arg, int sub_index)
+static int mp_property_sub_text(void *ctx, struct m_property *prop,
+                                int action, void *arg)
 {
-    int type = *(int *)prop->priv;
     MPContext *mpctx = ctx;
+    const int *def = prop->priv;
+    int sub_index = def[0];
+    int type = def[1];
+
     struct track *track = mpctx->current_track[sub_index][STREAM_SUB];
     struct dec_sub *sub = track ? track->d_sub : NULL;
     double pts = mpctx->playback_pts;
@@ -2971,18 +2963,6 @@ static int get_sub_text(void *ctx, struct m_property *prop,
     return M_PROPERTY_NOT_IMPLEMENTED;
 }
 
-static int mp_property_sub_text(void *ctx, struct m_property *prop,
-                                int action, void *arg)
-{
-    return get_sub_text(ctx, prop, action, arg, 0);
-}
-
-static int mp_property_secondary_sub_text(void *ctx, struct m_property *prop,
-                                          int action, void *arg)
-{
-    return get_sub_text(ctx, prop, action, arg, 1);
-}
-
 static struct sd_times get_times(void *ctx, struct m_property *prop,
                                 int action, void *arg)
 {
@@ -3947,15 +3927,16 @@ static const struct m_property mp_properties_base[] = {
     {"secondary-sub-delay", mp_property_sub_delay,
         .priv = (void *)&(const int){1}},
     {"sub-speed", mp_property_sub_speed},
-    {"sub-pos", mp_property_sub_pos},
-    {"secondary-sub-pos", mp_property_secondary_sub_pos},
+    {"sub-pos", mp_property_sub_pos, .priv = (void *)&(const int){0}},
+    {"secondary-sub-pos", mp_property_sub_pos,
+        .priv = (void *)&(const int){1}},
     {"sub-ass-extradata", mp_property_sub_ass_extradata},
     {"sub-text", mp_property_sub_text,
-        .priv = (void *)&(const int){SD_TEXT_TYPE_PLAIN}},
-    {"secondary-sub-text", mp_property_secondary_sub_text,
-        .priv = (void *)&(const int){SD_TEXT_TYPE_PLAIN}},
+        .priv = (void *)&(const int[]){0, SD_TEXT_TYPE_PLAIN}},
+    {"secondary-sub-text", mp_property_sub_text,
+        .priv = (void *)&(const int[]){1, SD_TEXT_TYPE_PLAIN}},
     {"sub-text-ass", mp_property_sub_text,
-        .priv = (void *)&(const int){SD_TEXT_TYPE_ASS}},
+        .priv = (void *)&(const int[]){0, SD_TEXT_TYPE_ASS}},
     {"sub-start", mp_property_sub_start,
         .priv = (void *)&(const int){0}},
     {"secondary-sub-start", mp_property_sub_start,
@@ -5406,9 +5387,9 @@ static void cmd_sub_step_seek(void *p)
         a[1] = cmd->args[0].v.i;
         if (sub_control(sub, SD_CTRL_SUB_STEP, a) > 0) {
             if (step) {
-                mpctx->opts->subs_rend->sub_delay[track_ind] -= a[0] - refpts;
+                mpctx->opts->subs_shared->sub_delay[track_ind] -= a[0] - refpts;
                 m_config_notify_change_opt_ptr_notify(mpctx->mconfig,
-                                &mpctx->opts->subs_rend->sub_delay[track_ind]);
+                                &mpctx->opts->subs_shared->sub_delay[track_ind]);
                 show_property_osd(
                     mpctx,
                     track_ind == 0 ? "sub-delay" : "secondary-sub-delay",
diff --git a/player/misc.c b/player/misc.c
index d267c51906..e267a53a46 100644
--- a/player/misc.c
+++ b/player/misc.c
@@ -150,7 +150,7 @@ double get_track_seek_offset(struct MPContext *mpctx, struct track *track)
         {
             for (int n = 0; n < num_ptracks[STREAM_SUB]; n++) {
                 if (mpctx->current_track[n][STREAM_SUB] == track)
-                    return -opts->subs_rend->sub_delay[n];
+                    return -opts->subs_shared->sub_delay[n];
             }
         }
     }
diff --git a/player/osd.c b/player/osd.c
index dc0322947d..d655ba4bcb 100644
--- a/player/osd.c
+++ b/player/osd.c
@@ -112,7 +112,7 @@ static void term_osd_update_title(struct MPContext *mpctx)
 
 void term_osd_set_subs(struct MPContext *mpctx, const char *text)
 {
-    if (mpctx->video_out || !text || !mpctx->opts->subs_rend->sub_visibility)
+    if (mpctx->video_out || !text || !mpctx->opts->subs_shared->sub_visibility[0])
         text = ""; // disable
     if (strcmp(mpctx->term_osd_subs ? mpctx->term_osd_subs : "", text) == 0)
         return;
diff --git a/sub/dec_sub.c b/sub/dec_sub.c
index 4cb2d9bb67..6ad26cba6a 100644
--- a/sub/dec_sub.c
+++ b/sub/dec_sub.c
@@ -47,7 +47,9 @@ struct dec_sub {
     struct mp_log *log;
     struct mpv_global *global;
     struct mp_subtitle_opts *opts;
+    struct mp_subtitle_shared_opts *shared_opts;
     struct m_config_cache *opts_cache;
+    struct m_config_cache *shared_opts_cache;
 
     struct mp_recorder_sink *recorder_sink;
 
@@ -91,7 +93,7 @@ static void update_subtitle_speed(struct dec_sub *sub)
 // Return the subtitle PTS used for a given video PTS.
 static double pts_to_subtitle(struct dec_sub *sub, double pts)
 {
-    struct mp_subtitle_opts *opts = sub->opts;
+    struct mp_subtitle_shared_opts *opts = sub->shared_opts;
     float delay = sub->order < 0 ? 0.0f : opts->sub_delay[sub->order];
 
     if (pts != MP_NOPTS_VALUE)
@@ -102,7 +104,7 @@ static double pts_to_subtitle(struct dec_sub *sub, double pts)
 
 static double pts_from_subtitle(struct dec_sub *sub, double pts)
 {
-    struct mp_subtitle_opts *opts = sub->opts;
+    struct mp_subtitle_shared_opts *opts = sub->shared_opts;
     float delay = sub->order < 0 ? 0.0f : opts->sub_delay[sub->order];
 
     if (pts != MP_NOPTS_VALUE)
@@ -140,6 +142,7 @@ static struct sd *init_decoder(struct dec_sub *sub)
             .global = sub->global,
             .log = mp_log_new(sd, sub->log, driver->name),
             .opts = sub->opts,
+            .shared_opts = sub->shared_opts,
             .driver = driver,
             .attachments = sub->attachments,
             .codec = sub->codec,
@@ -172,6 +175,7 @@ struct dec_sub *sub_create(struct mpv_global *global, struct track *track,
         .log = mp_log_new(sub, global->log, "sub"),
         .global = global,
         .opts_cache = m_config_cache_alloc(sub, global, &mp_subtitle_sub_opts),
+        .shared_opts_cache = m_config_cache_alloc(sub, global, &mp_subtitle_shared_sub_opts),
         .sh = track->stream,
         .codec = track->stream->codec,
         .attachments = talloc_steal(sub, attachments),
@@ -183,6 +187,7 @@ struct dec_sub *sub_create(struct mpv_global *global, struct track *track,
         .end = MP_NOPTS_VALUE,
     };
     sub->opts = sub->opts_cache->opts;
+    sub->shared_opts = sub->shared_opts_cache->opts;
     mp_mutex_init_type(&sub->lock, MP_MUTEX_RECURSIVE);
 
     sub->sd = init_decoder(sub);
@@ -292,7 +297,7 @@ bool sub_read_packets(struct dec_sub *sub, double video_pts, bool force)
             break;
 
         // (Use this mechanism only if sub_delay matters to avoid corner cases.)
-        float delay = sub->order < 0 ? 0.0f : sub->opts->sub_delay[sub->order];
+        float delay = sub->order < 0 ? 0.0f : sub->shared_opts->sub_delay[sub->order];
         double min_pts = delay < 0 || force ? video_pts : MP_NOPTS_VALUE;
 
         struct demux_packet *pkt;
@@ -457,6 +462,7 @@ int sub_control(struct dec_sub *sub, enum sd_ctrl cmd, void *arg)
         int flags = (uintptr_t)arg;
         if (m_config_cache_update(sub->opts_cache))
             update_subtitle_speed(sub);
+        m_config_cache_update(sub->shared_opts_cache);
         propagate = true;
         if (flags & UPDATE_SUB_HARD) {
             // forget about the previous preload because
@@ -491,10 +497,10 @@ void sub_set_play_dir(struct dec_sub *sub, int dir)
 
 bool sub_is_primary_visible(struct dec_sub *sub)
 {
-    return !!sub->opts->sub_visibility;
+    return sub->shared_opts->sub_visibility[0];
 }
 
 bool sub_is_secondary_visible(struct dec_sub *sub)
 {
-    return !!sub->opts->sec_sub_visibility;
+    return sub->shared_opts->sub_visibility[1];
 }
diff --git a/sub/sd.h b/sub/sd.h
index 969d1dcaca..5c2e44fae3 100644
--- a/sub/sd.h
+++ b/sub/sd.h
@@ -16,6 +16,7 @@ struct sd {
     struct mpv_global *global;
     struct mp_log *log;
     struct mp_subtitle_opts *opts;
+    struct mp_subtitle_shared_opts *shared_opts;
 
     const struct sd_functions *driver;
     void *priv;
diff --git a/sub/sd_ass.c b/sub/sd_ass.c
index 4c1ccd05bc..283aec6160 100644
--- a/sub/sd_ass.c
+++ b/sub/sd_ass.c
@@ -382,6 +382,7 @@ static void configure_ass(struct sd *sd, struct mp_osd_res *dim,
                           bool converted, ASS_Track *track, int order)
 {
     struct mp_subtitle_opts *opts = sd->opts;
+    struct mp_subtitle_shared_opts *shared_opts = sd->shared_opts;
     struct sd_ass_priv *ctx = sd->priv;
     ASS_Renderer *priv = ctx->ass_renderer;
 
@@ -407,7 +408,7 @@ static void configure_ass(struct sd *sd, struct mp_osd_res *dim,
         set_use_margins = opts->ass_use_margins;
     }
     if (converted || opts->ass_style_override) {
-        set_sub_pos = 100.0f - (order == 1 ? opts->sec_sub_pos : opts->sub_pos);
+        set_sub_pos = 100.0f - shared_opts->sub_pos[order];
         set_line_spacing = opts->ass_line_spacing;
         set_hinting = opts->ass_hinting;
         set_font_scale = opts->sub_scale;
diff --git a/sub/sd_lavc.c b/sub/sd_lavc.c
index 5bf887dad0..4f43a1d6d8 100644
--- a/sub/sd_lavc.c
+++ b/sub/sd_lavc.c
@@ -403,6 +403,7 @@ static struct sub_bitmaps *get_bitmaps(struct sd *sd, struct mp_osd_res d,
 {
     struct sd_lavc_priv *priv = sd->priv;
     struct mp_subtitle_opts *opts = sd->opts;
+    struct mp_subtitle_shared_opts *shared_opts = sd->shared_opts;
 
     priv->current_pts = pts;
 
@@ -450,8 +451,8 @@ static struct sub_bitmaps *get_bitmaps(struct sd *sd, struct mp_osd_res d,
         h = MPMAX(priv->video_params.h, current->src_h);
     }
 
-    if (opts->sub_pos != 100.0f && opts->ass_style_override) {
-        float offset = (100.0f - opts->sub_pos) / 100.0f * h;
+    if (shared_opts->sub_pos[0] != 100.0f && opts->ass_style_override) {
+        float offset = (100.0f - shared_opts->sub_pos[0]) / 100.0f * h;
 
         for (int n = 0; n < res->num_parts; n++) {
             struct sub_bitmap *sub = &res->parts[n];