vo_opengl: cleanup icc + runtime option changing behavior

Commit 026b75e7 actually enabled changing icc options at runtime (via
vo_cmdline), but it didn't quite work. In particular, changing the icc-
profile option just kept the old profile, because it was cached
accordingly.

As part of this, change gl_lcms.opts from a struct to a pointer to a
struct. We properly copy it, instead of allowing possibly dangling
strings, like it was done in a working but unclean way before.

Also, reinit the whole rendering chain when the auto icc profile
changes, just like it's done when icc options are changed.

3 files changed, 72 insertions, 43 deletions

diff --git a/video/out/opengl/lcms.c b/video/out/opengl/lcms.c
index d847906dcd..eaeb86fa8d 100644
--- a/video/out/opengl/lcms.c
+++ b/video/out/opengl/lcms.c
@@ -26,6 +26,7 @@
 #include "common/common.h"
 #include "misc/bstr.h"
 #include "common/msg.h"
+#include "options/m_config.h"
 #include "options/m_option.h"
 #include "options/path.h"
 #include "video/csputils.h"
@@ -42,14 +43,14 @@
 struct gl_lcms {
     void *icc_data;
     size_t icc_size;
-    char *icc_path;
+    bool using_memory_profile;
     bool changed;
     enum mp_csp_prim prev_prim;
     enum mp_csp_trc prev_trc;
 
     struct mp_log *log;
     struct mpv_global *global;
-    struct mp_icc_opts opts;
+    struct mp_icc_opts *opts;
 };
 
 static bool parse_3dlut_size(const char *arg, int *p1, int *p2, int *p3)
@@ -100,25 +101,28 @@ static void lcms2_error_handler(cmsContext ctx, cmsUInt32Number code,
     MP_ERR(p, "lcms2: %s\n", msg);
 }
 
-static bool load_profile(struct gl_lcms *p)
+static void load_profile(struct gl_lcms *p)
 {
-    if (p->icc_data && p->icc_size)
-        return true;
+    talloc_free(p->icc_data);
+    p->icc_data = NULL;
+    p->icc_size = 0;
+    p->using_memory_profile = false;
 
-    if (!p->icc_path)
-        return false;
+    if (!p->opts->profile || !p->opts->profile[0])
+        return;
 
-    char *fname = mp_get_user_path(NULL, p->global, p->icc_path);
+    char *fname = mp_get_user_path(NULL, p->global, p->opts->profile);
     MP_VERBOSE(p, "Opening ICC profile '%s'\n", fname);
     struct bstr iccdata = stream_read_file(fname, p, p->global,
                                            100000000); // 100 MB
     talloc_free(fname);
     if (!iccdata.len)
-        return false;
+        return;
+
+    talloc_free(p->icc_data);
 
     p->icc_data = iccdata.start;
     p->icc_size = iccdata.len;
-    return true;
 }
 
 struct gl_lcms *gl_lcms_init(void *talloc_ctx, struct mp_log *log,
@@ -129,44 +133,55 @@ struct gl_lcms *gl_lcms_init(void *talloc_ctx, struct mp_log *log,
         .global = global,
         .log = log,
         .changed = true,
+        .opts = m_sub_options_copy(p, &mp_icc_conf, mp_icc_conf.defaults),
     };
     return p;
 }
 
 void gl_lcms_set_options(struct gl_lcms *p, struct mp_icc_opts *opts)
 {
-    p->opts = *opts;
-    p->icc_path = talloc_strdup(p, p->opts.profile);
-    load_profile(p);
+    struct mp_icc_opts *old_opts = p->opts;
+    p->opts = m_sub_options_copy(p, &mp_icc_conf, opts);
+
+    if ((p->using_memory_profile && !p->opts->profile_auto) ||
+        !bstr_equals(bstr0(p->opts->profile), bstr0(old_opts->profile)))
+    {
+        load_profile(p);
+    }
+
     p->changed = true; // probably
+
+    talloc_free(old_opts);
 }
 
 // Warning: profile.start must point to a ta allocation, and the function
 //          takes over ownership.
-void gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile)
+// Returns whether the internal profile was changed.
+bool gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile)
 {
-    if (!p->opts.profile_auto || (p->icc_path && p->icc_path[0])) {
+    if (!p->opts->profile_auto || (p->opts->profile && p->opts->profile[0])) {
         talloc_free(profile.start);
-        return;
+        return false;
     }
 
-    if (!p->icc_path && p->icc_data && profile.start &&
+    if (p->using_memory_profile &&
+        p->icc_data && profile.start &&
         profile.len == p->icc_size &&
         memcmp(profile.start, p->icc_data, p->icc_size) == 0)
     {
         talloc_free(profile.start);
-        return;
+        return false;
     }
 
     p->changed = true;
-
-    talloc_free(p->icc_path);
-    p->icc_path = NULL;
+    p->using_memory_profile = true;
 
     talloc_free(p->icc_data);
 
     p->icc_data = talloc_steal(p, profile.start);
     p->icc_size = profile.len;
+
+    return true;
 }
 
 // Return and _reset_ whether the profile or config has changed since the last
@@ -181,6 +196,13 @@ bool gl_lcms_has_changed(struct gl_lcms *p, enum mp_csp_prim prim,
     return change;
 }
 
+// Whether a profile is set. (gl_lcms_get_lut3d() is expected to return a lut,
+// but it could still fail due to runtime errors, such as invalid icc data.)
+bool gl_lcms_has_profile(struct gl_lcms *p)
+{
+    return p->icc_size > 0;
+}
+
 static cmsHPROFILE get_vid_profile(struct gl_lcms *p, cmsContext cms,
                                    cmsHPROFILE disp_profile,
                                    enum mp_csp_prim prim, enum mp_csp_trc trc)
@@ -243,9 +265,9 @@ static cmsHPROFILE get_vid_profile(struct gl_lcms *p, cmsContext cms,
         cmsDeleteTransform(xyz2src);
 
         // Contrast limiting
-        if (p->opts.contrast > 0) {
+        if (p->opts->contrast > 0) {
             for (int i = 0; i < 3; i++)
-                src_black[i] = MPMAX(src_black[i], 1.0 / p->opts.contrast);
+                src_black[i] = MPMAX(src_black[i], 1.0 / p->opts->contrast);
         }
 
         // Built-in contrast failsafe
@@ -293,10 +315,10 @@ bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **result_lut3d,
     int s_r, s_g, s_b;
     bool result = false;
 
-    if (!parse_3dlut_size(p->opts.size_str, &s_r, &s_g, &s_b))
+    if (!parse_3dlut_size(p->opts->size_str, &s_r, &s_g, &s_b))
         return false;
 
-    if (!p->icc_data && !p->icc_path)
+    if (!gl_lcms_has_profile(p))
         return false;
 
     void *tmp = talloc_new(NULL);
@@ -305,14 +327,14 @@ bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **result_lut3d,
     cmsContext cms = NULL;
 
     char *cache_file = NULL;
-    if (p->opts.cache_dir && p->opts.cache_dir[0]) {
+    if (p->opts->cache_dir && p->opts->cache_dir[0]) {
         // Gamma is included in the header to help uniquely identify it,
         // because we may change the parameter in the future or make it
         // customizable, same for the primaries.
         char *cache_info = talloc_asprintf(tmp,
                 "ver=1.3, intent=%d, size=%dx%dx%d, prim=%d, trc=%d, "
                 "contrast=%d\n",
-                p->opts.intent, s_r, s_g, s_b, prim, trc, p->opts.contrast);
+                p->opts->intent, s_r, s_g, s_b, prim, trc, p->opts->contrast);
 
         uint8_t hash[32];
         struct AVSHA *sha = av_sha_alloc();
@@ -324,7 +346,7 @@ bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **result_lut3d,
         av_sha_final(sha, hash);
         av_free(sha);
 
-        char *cache_dir = mp_get_user_path(tmp, p->global, p->opts.cache_dir);
+        char *cache_dir = mp_get_user_path(tmp, p->global, p->opts->cache_dir);
         cache_file = talloc_strdup(tmp, "");
         for (int i = 0; i < sizeof(hash); i++)
             cache_file = talloc_asprintf_append(cache_file, "%02X", hash[i]);
@@ -364,7 +386,7 @@ bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **result_lut3d,
 
     cmsHTRANSFORM trafo = cmsCreateTransformTHR(cms, vid_profile, TYPE_RGB_16,
                                                 profile, TYPE_RGB_16,
-                                                p->opts.intent,
+                                                p->opts->intent,
                                                 cmsFLAGS_HIGHRESPRECALC |
                                                 cmsFLAGS_BLACKPOINTCOMPENSATION);
     cmsCloseProfile(profile);
@@ -436,7 +458,7 @@ struct gl_lcms *gl_lcms_init(void *talloc_ctx, struct mp_log *log,
 }
 
 void gl_lcms_set_options(struct gl_lcms *p, struct mp_icc_opts *opts) { }
-void gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile) { }
+bool gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile) {return false;}
 
 bool gl_lcms_has_changed(struct gl_lcms *p, enum mp_csp_prim prim,
                          enum mp_csp_trc trc)
@@ -444,6 +466,11 @@ bool gl_lcms_has_changed(struct gl_lcms *p, enum mp_csp_prim prim,
     return false;
 }
 
+bool gl_lcms_has_profile(struct gl_lcms *p)
+{
+    return false;
+}
+
 bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **result_lut3d,
                        enum mp_csp_prim prim, enum mp_csp_trc trc)
 {
diff --git a/video/out/opengl/lcms.h b/video/out/opengl/lcms.h
index 2c6543d953..094514ac9e 100644
--- a/video/out/opengl/lcms.h
+++ b/video/out/opengl/lcms.h
@@ -28,7 +28,8 @@ struct gl_lcms;
 struct gl_lcms *gl_lcms_init(void *talloc_ctx, struct mp_log *log,
                              struct mpv_global *global);
 void gl_lcms_set_options(struct gl_lcms *p, struct mp_icc_opts *opts);
-void gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile);
+bool gl_lcms_set_memory_profile(struct gl_lcms *p, bstr profile);
+bool gl_lcms_has_profile(struct gl_lcms *p);
 bool gl_lcms_get_lut3d(struct gl_lcms *p, struct lut3d **,
                        enum mp_csp_prim prim, enum mp_csp_trc trc);
 bool gl_lcms_has_changed(struct gl_lcms *p, enum mp_csp_prim prim,
diff --git a/video/out/opengl/video.c b/video/out/opengl/video.c
index 7eb7736c04..6c7348a1f5 100644
--- a/video/out/opengl/video.c
+++ b/video/out/opengl/video.c
@@ -496,6 +496,7 @@ static bool init_format(struct gl_video *p, int fmt, bool test_only);
 static void init_image_desc(struct gl_video *p, int fmt);
 static bool gl_video_upload_image(struct gl_video *p, struct mp_image *mpi);
 static void assign_options(struct gl_video_opts *dst, struct gl_video_opts *src);
+static void reinit_from_options(struct gl_video *p);
 static void get_scale_factors(struct gl_video *p, bool transpose_rot, double xy[2]);
 static void gl_video_setup_hooks(struct gl_video *p);
 
@@ -613,13 +614,8 @@ static void uninit_rendering(struct gl_video *p)
 //          takes over ownership.
 void gl_video_set_icc_profile(struct gl_video *p, bstr icc_data)
 {
-    gl_lcms_set_memory_profile(p->cms, icc_data);
-
-    if (p->use_lut_3d)
-        return;
-
-    p->use_lut_3d = true;
-    check_gl_features(p);
+    if (gl_lcms_set_memory_profile(p->cms, icc_data))
+        reinit_from_options(p);
 }
 
 bool gl_video_icc_auto_enabled(struct gl_video *p)
@@ -635,11 +631,12 @@ static bool gl_video_get_lut3d(struct gl_video *p, enum mp_csp_prim prim,
     if (!p->use_lut_3d)
         return false;
 
-    if (!gl_lcms_has_changed(p->cms, prim, trc))
+    if (p->lut_3d_texture && !gl_lcms_has_changed(p->cms, prim, trc))
         return true;
 
     struct lut3d *lut3d = NULL;
     if (!gl_lcms_get_lut3d(p->cms, &lut3d, prim, trc) || !lut3d) {
+        p->use_lut_3d = false;
         return false;
     }
 
@@ -2236,8 +2233,6 @@ static void pass_colormanage(struct gl_video *p, float peak_src,
         if (gl_video_get_lut3d(p, prim_orig, trc_orig)) {
             prim_dst = prim_orig;
             trc_dst = trc_orig;
-        } else {
-            p->use_lut_3d = false;
         }
     }
 
@@ -3565,10 +3560,16 @@ void gl_video_set_options(struct gl_video *p, struct gl_video_opts *opts)
 {
     assign_options(&p->opts, opts);
 
+    reinit_from_options(p);
+}
+
+static void reinit_from_options(struct gl_video *p)
+{
+    p->use_lut_3d = false;
+
     if (p->opts.icc_opts) {
         gl_lcms_set_options(p->cms, p->opts.icc_opts);
-        if (p->opts.icc_opts->profile && p->opts.icc_opts->profile[0])
-            p->use_lut_3d = true;
+        p->use_lut_3d = gl_lcms_has_profile(p->cms);
     }
 
     check_gl_features(p);