feat: oiiotool --mergemeta pt 2: fold into --pastemeta (AcademySoftwareFoundation#4674)

After merging AcademySoftwareFoundation#4672, we noticed some things that could be improved, so
this is a further revision (and partial retraction).

There was already an oiiotool flag, `--metamerge`, which in retrospect
make it seem unnecessarily confusing to add --mergemeta. So this PR
removes this extra new command, instead simply piggy-backing on the
existing --pastemeta by adding to it optional modifiers:

- `:merge=` controls the merge strategy, 0 (default) means discard the
existing metadata entirely and use only the pasted, matching the
original --pastemeta behavior; 1 means merge nondestructively (no
existing metadata will be altered or removed); 2 means destructive
merge, where pasted metadata will overwrite existing metadata having the
same name, but conflict-free existing metadata will remain.

- `:pattern=` regex selects only some metadata.

Additionally, we noticed an interesting corner case that surprised users
when testing the new metadata merging feature from the last PR: What
happens if the metadata source image has fewer subimages than the pixel
source images? The behavior generally for oiiotool is that when binary
operations (those taking two images as input) are on images with
differing numbers of subimages, the output gets the number of subimges
as the FIRST input (assuming -a instructed us to consider subimages at
all). But that's not what we want here! If the metadata source has one
subimage and the pixel source has many, clearly the intuitive result is
for that one metadata set to be pasted into all of the subimages from
the pixel-supplying image.

So we fix that so that for --pastemeta, it's the second input image (not
the first) that takes the lead in determining the number of subimages we
will process. This in turn took a bit of extra infrastructure fixing in
the OiiotoolOp class that controls this logic.

Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Scott Wilson <[email protected]>

Author: lgritz

src/doc/oiiotool.rst

@@ -2996,47 +2996,40 @@ current top image.
 
 .. option:: --pastemeta <location>
 
-    Takes two images -- the first will be a source of metadata only, and the
-    second the source of pixels -- and produces a new copy of the second
-    image with the metadata from the first image.
-
-    Example::
-
-        # Add all the metadata from meta.exr to pixels.exr and write the
-        # combined image to out.exr.
-        oiiotool meta.exr pixels.exr --pastemeta -o out.exr
-
-
-.. option:: --mergemeta <location>
-
-    Takes two images -- the first will be a source of metadata only, and the
-    second the source of pixels -- and produces a new copy of the second
-    image with the metadata from the first image added added to the second
-    image, item by item (i.e. not wholly removing the metadata that was
-    there before).
+    Takes two images -- the first image will be a source of metadata only, and the
+    second "destination" image will supply the pixels -- and produces a combined
+    image.
 
     Optional appended modifiers include:
 
-    - `override=` *int* : If zero (the default), no
-      existing metadata in the destination will be altered, i.e., the only
-      metadata copied from source to destination will be those that did not
-      previously exist in the destination. If the override value is nonzero,
-      then all metadata items in the source will *replace* any existing
-      correspondingly named items in the destination.
+    - `merge=` *int* : Determines the metadata merging strategy. If `0` (the
+      default), performs a full replacement -- the metadata from the
+      destination (second) image will be discarded entirely and re-populated
+      with the metadata from the metadata source (first) image. If `1`,
+      metadata from the source image will be non-destructively added to that
+      of the existing metadata of the destination image, item by item, if it
+      doesn't already contain metadata with the same name. If `2`, metadata
+      from the source image will be destructively added to that of the
+      existing metadata of the destination image, item by item, and will
+      replace any metadata already present having the same name. (This
+      modifier was added in OIIO 3.0.4.)
 
     - `pattern=` *regex* : If supplied, only copies metadata whose name
       matches has a substring matching the regular expression. The special
       character `^` indicates the beginning of the string and `$` indicates
-      the end of the string. 
+      the end of the string. (This modifier was added in OIIO 3.0.4.)
 
-    Example::
+    Examples::
+
+        # Add all the metadata from meta.exr to pixels.exr, discarding any
+        # metadata it previously had, and write the combined image to out.exr.
+        oiiotool meta.exr pixels.exr --pastemeta -o out.exr
 
         # Add all of meta.exr's metadata whose name begins with "camera:"
-        # to pixels.exr, replacing any similarly named items, and write
-        # the combined image to out.exr.
-        oiiotool meta.exr pixels.exr --pastemeta:pattern="^camera:override=1" -o out.exr
+        # to pixels.exr, replacing any identically named items but leaving
+        # others as they were, and write the combined image to out.exr.
+        oiiotool meta.exr pixels.exr --pastemeta:merge=2:pattern="^camera:override=1" -o out.exr
 
-    The `--mergemeta` command  was added in OIIO 3.0.4.
 
 .. option:: --mosaic <size>
 

src/include/OpenImageIO/paramlist.h

@@ -333,8 +333,15 @@ make_pv(string_view name, T* val)
 /// methods.
 class OIIO_UTIL_API ParamValueList : public std::vector<ParamValue> {
 public:
+    // Default constructor
     ParamValueList() {}
 
+    // Construct from a span of ParamValue items
+    ParamValueList(cspan<ParamValue> params)
+    {
+        assign(params.begin(), params.end());
+    }
+
     /// Add space for one more ParamValue to the list, and return a
     /// reference to its slot.
     reference grow()

src/oiiotool/oiiotool.cpp

@@ -4757,23 +4757,24 @@ action_paste(Oiiotool& ot, cspan<const char*> argv)
 
 
 // --pastemeta
-OIIOTOOL_OP(pastemeta, 2, [&](OiiotoolOp& op, span<ImageBuf*> img) {
-    *img[0] = *img[2];
-    img[0]->copy_metadata(*img[1]);
-    return true;
-});
-
-
-
-// --mergemeta
-OIIOTOOL_OP(mergemeta, 2, [&](OiiotoolOp& op, span<ImageBuf*> img) {
-    std::string pattern = op.options("pattern");
-    bool override       = op.options("override").get<int>();
+OIIOTOOL_OP(
+    pastemeta, 2,
+    [&](OiiotoolOp& op, span<ImageBuf*> img) {
+        std::string pattern = op.options("pattern");
+        int merge           = op.options("merge").get<int>(0);
+
+        *img[0] = *img[2];
+        if (merge == 0) {
+            // merge strategy 0 completely discards the existing metadata
+            img[0]->specmod().extra_attribs.clear();
+        }
 
-    *img[0] = *img[2];
-    img[0]->merge_metadata(*img[1], override, pattern);
-    return true;
-});
+        img[0]->merge_metadata(*img[1], merge > 1, pattern);
+        return true;
+    },
+    // Special policy to ensure we consider all subimages of second image
+    // (pixel inputs), not the default of considering only the first image.
+    { { "subimage_policy", "last" } });
 
 
 
@@ -6799,11 +6800,8 @@ Oiiotool::getargs(int argc, char* argv[])
       .help("Paste fg over bg at the given position (e.g., +100+50; '-' or 'auto' indicates using the data window position as-is; options: all=%d, mergeroi=%d)")
       .OTACTION(action_paste);
     ap.arg("--pastemeta")
-      .help("Copy the metadata from the first image to the second image and keep the combined result")
+      .help("Paste the metadata from the first image into the second image (options: merge=%d, pattern=REGEX)")
       .OTACTION(action_pastemeta);
-    ap.arg("--mergemeta")
-      .help("Merge the metadata from the first image into the second image and keep the combined result (options: pattern=REGEX, override=%d)")
-      .OTACTION(action_mergemeta);
     ap.arg("--mosaic %s:WxH")
       .help("Assemble images into a mosaic (arg: WxH; options: pad=0, fit=WxH)")
       .OTACTION(action_mosaic);

src/oiiotool/oiiotool.h

@@ -781,12 +781,14 @@ class OiiotoolOp {
     // stack.
     OiiotoolOp(Oiiotool& ot, string_view opname, cspan<const char*> argv,
                int ninputs, setup_func_t setup_func = nullptr,
-               impl_func_t impl_func = nullptr)
+               impl_func_t impl_func          = nullptr,
+               ParamValueSpan control_options = {})
         : ot(ot)
         , m_nargs((int)argv.size())
         , m_nimages(ninputs + 1)
         , m_setup_func(setup_func)
         , m_impl_func(impl_func)
+        , m_control_options(control_options)
     {
         if (Strutil::starts_with(opname, "--"))
             opname.remove_prefix(1);  // canonicalize to one dash
@@ -803,6 +805,12 @@ class OiiotoolOp {
         : OiiotoolOp(ot, opname, argv, ninputs, {}, impl_func)
     {
     }
+    OiiotoolOp(Oiiotool& ot, string_view opname, cspan<const char*> argv,
+               int ninputs, impl_func_t impl_func,
+               ParamValueSpan control_options)
+        : OiiotoolOp(ot, opname, argv, ninputs, {}, impl_func, control_options)
+    {
+    }
     virtual ~OiiotoolOp() {}
 
     // The operator(), function-call mode, does most of the work. Although
@@ -891,10 +899,12 @@ class OiiotoolOp {
             m_img.resize(nimages());
             for (int m = 0, nmip = ir(0)->miplevels(); m < nmip; ++m) {
                 m_current_miplevel = m;
-                for (int i = 0; i < nimages(); ++i)
-                    m_img[i] = &((*ir(i))(std::min(s, ir(i)->subimages() - 1),
-                                          std::min(m, ir(i)->miplevels(s))));
-
+                for (int i = 0; i < nimages(); ++i) {
+                    // If the subimage doesn't exist, just use the last
+                    int ss   = std::min(s, ir(i)->subimages() - 1);
+                    m_img[i] = &(
+                        (*ir(i))(ss, std::min(m, ir(i)->miplevels(ss))));
+                }
                 if (subimage_is_active(s)) {
                     // Call the impl kernel for this subimage
                     bool ok = impl(m_img);
@@ -1010,7 +1020,35 @@ class OiiotoolOp {
             }
         }
         all_subimages |= m_options.get_int("allsubimages", ot.allsubimages);
-        return all_subimages ? (nimages() > 1 ? ir(1)->subimages() : 1) : 1;
+
+        // How many subimages are we going to operate on? There are a few
+        // strategies for handling the decision when the input images differ
+        // in the number of subimages. The default is to operate on subimage
+        // list from the first image input.
+        int n = 1;
+        if (all_subimages) {
+            OIIO_DASSERT(nimages() >= 1);
+            std::string subimage_policy = m_control_options["subimage_policy"];
+            if (subimage_policy == "max") {
+                // operate on the maximum number of subimages of the inputs
+                for (int i = 1; i < nimages(); ++i)
+                    n = std::max(n, ir(i)->subimages());
+            } else if (subimage_policy == "min") {
+                // operate on the minimum number of subimages of the inputs
+                if (nimages() > 1)
+                    n = ir(1)->subimages();
+                for (int i = 2; i < nimages(); ++i)
+                    n = std::min(n, ir(i)->subimages());
+            } else if (subimage_policy == "last") {
+                // operate on the subimages of the last input
+                n = ir(nimages() - 1)->subimages();
+            } else /* if (subimage_policy == "first")*/ {  // default
+                // default: operate on the subimages of the first input
+                if (nimages() > 1)
+                    n = ir(1)->subimages();
+            }
+        }
+        return n;
     }
 
     // Is the given subimage in the active set to be operated on by this op?
@@ -1112,6 +1150,7 @@ class OiiotoolOp {
     new_output_imagerec_func_t m_new_output_imagerec_func;
     int m_current_subimage;  // for impl(), which subimage are we on?
     int m_current_miplevel;  // for impl(), which miplevel are we on?
+    ParamValueList m_control_options;
 };
 
 

testsuite/oiiotool-copy/ref/greenmeta.exr …/oiiotool-copy/ref/greenmeta-replace.exrtestsuite/oiiotool-copy/ref/greenmeta.exr renamed to testsuite/oiiotool-copy/ref/greenmeta-replace.exr testsuite/oiiotool-copy/ref/greenmeta.exr renamed to testsuite/oiiotool-copy/ref/greenmeta-replace.exr

@@ -83,8 +83,8 @@ green.exr            :   64 x   64, 3 channel, half openexr
     oiio:ColorSpace: "lin_rec709"
     oiio:subimages: 1
     openexr:lineOrder: "increasingY"
-Reading greenmeta.exr
-greenmeta.exr        :   64 x   64, 3 channel, half openexr
+Reading greenmeta-replace.exr
+greenmeta-replace.exr :   64 x   64, 3 channel, half openexr
     SHA-1: 8B61993247469F3C208CA894D71856727B11606A
     channel list: R, G, B
     compression: "zip"
@@ -145,6 +145,63 @@ greenmeta-merge-camera.exr :   64 x   64, 3 channel, half openexr
     oiio:ColorSpace: "lin_rec709"
     oiio:subimages: 1
     openexr:lineOrder: "increasingY"
+Reading greenmeta-merge-2.exr
+greenmeta-merge-2.exr :   64 x   64, 3 channel, half openexr
+    3 subimages: 64x64 [h,h,h], 64x64 [h,h,h], 64x64 [h,h,h]
+ subimage  0:   64 x   64, 3 channel, half openexr
+    SHA-1: 8B61993247469F3C208CA894D71856727B11606A
+    channel list: R, G, B
+    compression: "zip"
+    eyes: 2
+    hair: "black"
+    name: "subimage00"
+    PixelAspectRatio: 1
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    weight: 20.5
+    camera:lens: "AX30"
+    camera:shutter: 0.0125
+    oiio:ColorSpace: "lin_rec709"
+    oiio:subimagename: "subimage00"
+    oiio:subimages: 3
+    openexr:chunkCount: 4
+    openexr:lineOrder: "increasingY"
+ subimage  1:   64 x   64, 3 channel, half openexr
+    SHA-1: 8B61993247469F3C208CA894D71856727B11606A
+    channel list: R, G, B
+    compression: "zip"
+    eyes: 2
+    hair: "black"
+    name: "subimage01"
+    PixelAspectRatio: 1
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    weight: 20.5
+    camera:lens: "AX30"
+    camera:shutter: 0.0125
+    oiio:ColorSpace: "lin_rec709"
+    oiio:subimagename: "subimage01"
+    oiio:subimages: 3
+    openexr:chunkCount: 4
+    openexr:lineOrder: "increasingY"
+ subimage  2:   64 x   64, 3 channel, half openexr
+    SHA-1: 8B61993247469F3C208CA894D71856727B11606A
+    channel list: R, G, B
+    compression: "zip"
+    eyes: 2
+    hair: "black"
+    name: "subimage02"
+    PixelAspectRatio: 1
+    screenWindowCenter: 0, 0
+    screenWindowWidth: 1
+    weight: 20.5
+    camera:lens: "AX30"
+    camera:shutter: 0.0125
+    oiio:ColorSpace: "lin_rec709"
+    oiio:subimagename: "subimage02"
+    oiio:subimages: 3
+    openexr:chunkCount: 4
+    openexr:lineOrder: "increasingY"
 Reading nometamerge.exr
 nometamerge.exr      :   64 x   64, 6 channel, float openexr
     SHA-1: 9F13A523321C66208E90D45F87FA0CD9B370E111
@@ -202,7 +259,7 @@ Comparing "mosaic.tif" and "ref/mosaic.tif"
 PASS
 Comparing "mosaicfit.tif" and "ref/mosaicfit.tif"
 PASS
-Comparing "greenmeta.exr" and "ref/greenmeta.exr"
+Comparing "greenmeta-replace.exr" and "ref/greenmeta-replace.exr"
 PASS
 Comparing "chappend-3images.exr" and "ref/chappend-3images.exr"
 PASS

testsuite/oiiotool-copy/ref/out.txt

@@ -95,15 +95,22 @@
 # test --pastemeta
 command += oiiotool ("--pattern:type=half constant:color=0,1,0 64x64 3 -attrib hair black -o green.exr")
 command += oiiotool ("--pattern:type=half constant:color=1,0,0 64x64 3 -attrib hair brown -attrib eyes 2 -attrib weight 20.5 -attrib camera:lens AX30 --attrib camera:shutter 0.0125  -o meta.exr")
-command += oiiotool ("meta.exr green.exr --pastemeta -o greenmeta.exr")
-command += oiiotool ("meta.exr green.exr --mergemeta -o greenmeta-merge.exr")
-command += oiiotool ("meta.exr green.exr --mergemeta:override=1 -o greenmeta-merge-override.exr")
-command += oiiotool ("meta.exr green.exr --mergemeta:pattern=\"^camera:\" -o greenmeta-merge-camera.exr")
 command += info_command ("green.exr", safematch=True)
-command += info_command ("greenmeta.exr", safematch=True)
+#  straight pastemeta -- full replacement
+command += oiiotool ("meta.exr green.exr --pastemeta -o greenmeta-replace.exr")
+command += info_command ("greenmeta-replace.exr", safematch=True)
+#  paste with nondestructive merge
+command += oiiotool ("meta.exr green.exr --pastemeta:merge=1 -o greenmeta-merge.exr")
 command += info_command ("greenmeta-merge.exr", safematch=True)
+#  pastemeta with destructive merge
+command += oiiotool ("meta.exr green.exr --pastemeta:merge=2 -o greenmeta-merge-override.exr")
 command += info_command ("greenmeta-merge-override.exr", safematch=True)
+#  pastemeta with pattern-selected subset
+command += oiiotool ("meta.exr green.exr --pastemeta:merge=1:pattern=\"^camera:\" -o greenmeta-merge-camera.exr")
 command += info_command ("greenmeta-merge-camera.exr", safematch=True)
+#  pastemeta odd case with more pixel subimages than meta subimages
+command += oiiotool ("-a meta.exr green.exr --dup --dup --siappend --siappend --pastemeta:merge=1 -o greenmeta-merge-2.exr")
+command += info_command ("greenmeta-merge-2.exr", safematch=True)
 
 # test mosaic
 # Purposely test with fewer images than the mosaic array size
@@ -146,7 +153,7 @@
             "chappend-rgbaz.exr", "chname.exr",
             "crop.tif", "cut.tif", "pasted.tif",
             "mosaic.tif", "mosaicfit.tif",
-            "greenmeta.exr",
+            "greenmeta-replace.exr",
             "chappend-3images.exr",
             "out.txt"
           ]