fix(oiiotool): fixes to --missingfile behavior (AcademySoftwareFoundation#4803)

Fix some `oiiotool --missingfile` behavioral corner cases:

When substituting, warn rather than making a true error. This didn't
seem like a big deal for a single command line, but when doing a loop
over frames, it would prevent correct behavior from pressing on to the
next iteration.

How big is the substitute image? It uses the rez and channels of the
first image successfully read (if available). But it when in a frame
loop, it resets this on every iteration, so any missing frame looks like
it doesn't have a good size guess -- but it does, as long as it's not
the first iteration of the loop. This involves safely gettingthe first
image dimensions from the *parent* (each iteration is a child Oiiotool
object).

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/oiiotool/oiiotool.cpp

@@ -174,8 +174,8 @@ Oiiotool::clear_options()
     printinfo_nometamatch.clear();
     printinfo_verbose = false;
     clear_input_config();
-    first_input_dimensions = ImageSpec();
-    output_dataformat      = TypeDesc::UNKNOWN;
+    m_first_input_dimensions = ImageSpec();
+    output_dataformat        = TypeDesc::UNKNOWN;
     output_channelformats.clear();
     output_bitspersample      = 0;
     output_scanline           = false;
@@ -5239,12 +5239,14 @@ input_file(Oiiotool& ot, cspan<const char*> argv)
         if (!ot.imagecache->get_image_info(ustring(filename), 0, 0,
                                            ustring("exists"), TypeInt, &exists))
             exists = 0;
-        // If the image doesn't appear t exist, but it's a procedural image
-        // generator, then that's ok.
+        // N.B. ImageCache "exists" really means "can be opened and read."
         if (!exists) {
             auto input = ImageInput::create(filename);
-            if (input && input->supports("procedural"))
+            if (input && input->supports("procedural")) {
+                // If the image doesn't appear to exist, but it's a procedural
+                // image generator, then that's ok.
                 exists = 1;
+            }
             if (!input) {
                 // If the create call failed, eat any stray global errors it
                 // may have issued.
@@ -5253,36 +5255,49 @@ input_file(Oiiotool& ot, cspan<const char*> argv)
         }
         ImageBufRef substitute;  // possible substitute for missing image
         if (!exists) {
-            // Try to get a more precise error message to report
-            if (!Filesystem::exists(filename))
-                ot.errorfmt("read", "File does not exist: \"{}\"", filename);
-            else {
+            // It neither can be opened nor is it a procedural.
+            std::string errmsg;
+            if (!Filesystem::exists(filename)) {
+                // It literally is a nonexistant file.
+                errmsg = Strutil::format("File does not exist: \"{}\"",
+                                         filename);
+            } else {
+                // The file exists, but it can't be opened as an image.
+                // Try to get a more precise error message to report.
                 auto in         = ImageInput::open(filename);
                 std::string err = in ? in->geterror() : OIIO::geterror();
-                ot.error("read", ot.format_read_error(filename, err));
+                errmsg = Strutil::format(ot.format_read_error(filename, err));
             }
             // Second chances: do we have a substitute image policy?
+            ImageSpec substitute_spec;
+            if (ot.first_input_dimensions_is_set())
+                ot.get_first_input_dimensions(substitute_spec);
+            else if (ot.parent_oiiotool
+                     && ot.parent_oiiotool->first_input_dimensions_is_set())
+                ot.parent_oiiotool->get_first_input_dimensions(substitute_spec);
+            else {
+                // How big to make the substitute image if we haven't yet read
+                // any image successfully? Punt and guess HD res RGBA.
+                substitute_spec = ImageSpec(1920, 1080, 4);
+            }
             if (ot.missingfile_policy == "black") {
-                ImageSpec substitute_spec = ot.first_input_dimensions;
-                if (substitute_spec.format == TypeUnknown
-                    || !substitute_spec.width || !substitute_spec.height
-                    || !substitute_spec.nchannels)
-                    substitute_spec = ImageSpec(1920, 1080, 4);
                 substitute.reset(
                     new ImageBuf(substitute_spec, InitializePixels::Yes));
             } else if (ot.missingfile_policy == "checker") {
-                ImageSpec substitute_spec = ot.first_input_dimensions;
-                if (substitute_spec.format == TypeUnknown
-                    || !substitute_spec.width || !substitute_spec.height
-                    || !substitute_spec.nchannels)
-                    substitute_spec = ImageSpec(1920, 1080, 4);
                 substitute.reset(new ImageBuf(substitute_spec));
                 ImageBufAlgo::checker(*substitute, 64, 64, 1,
                                       { 0.0f, 0.0f, 0.0f, 1.0f },
                                       { 1.0f, 1.0f, 1.0f, 1.0f });
             }
-            if (!substitute)
-                break;
+            if (!exists) {
+                if (substitute) {
+                    ot.warningfmt("read", "{}, Substituting {}", errmsg,
+                                  ot.missingfile_policy);
+                } else {
+                    ot.error("read", errmsg);
+                    break;
+                }
+            }
         }
         if (channel_set.size()) {
             ot.input_channel_set = channel_set;
@@ -5306,11 +5321,15 @@ input_file(Oiiotool& ot, cspan<const char*> argv)
                 ot.read(policy, channel_set);
             } else
                 ot.read_nativespec();
-            if (ot.first_input_dimensions.format == TypeUnknown) {
-                ot.first_input_dimensions.copy_dimensions(
-                    *ot.curimg->nativespec());
-                ot.first_input_dimensions.channelnames
+            if (!ot.first_input_dimensions_is_set()) {
+                ImageSpec new_first_dims;
+                new_first_dims.copy_dimensions(*ot.curimg->nativespec());
+                new_first_dims.channelnames
                     = ot.curimg->nativespec()->channelnames;
+                ot.set_first_input_dimensions(new_first_dims);
+                if (ot.parent_oiiotool)
+                    ot.parent_oiiotool->set_first_input_dimensions(
+                        new_first_dims);
             }
         }
         if ((printinfo || ot.printstats || ot.dumpdata || ot.hash)
@@ -7208,8 +7227,9 @@ one_sequence_iteration(Oiiotool& otmain, size_t i, int frame_number,
     }
 
     Oiiotool otit;  // Oiiotool for this iteration
-    otit.imagecache   = otmain.imagecache;
-    otit.frame_number = frame_number;
+    otit.imagecache      = otmain.imagecache;
+    otit.frame_number    = frame_number;
+    otit.parent_oiiotool = &otmain;
     otit.getargs((int)seq_argv.size(), (char**)&seq_argv[0]);
 
     if (otit.ap.aborted()) {

src/oiiotool/oiiotool.h

@@ -89,12 +89,13 @@ class Oiiotool {
     std::string printinfo_nometamatch;
     std::string printinfo_format;
     std::string missingfile_policy;
-    ImageSpec input_config;  // configuration options for reading
-    ImageSpec first_input_dimensions;
+    ImageSpec input_config;         // configuration options for reading
     std::string input_channel_set;  // Optional input channel set
     ParamValueList uservars;        // User-defined variables (with --set)
     ArgParse ap;                    // Command-line argument parser
     ErrorHandler eh;
+    // If this is a frame iteration child, retain a pointer to the parent
+    Oiiotool* parent_oiiotool = nullptr;
 
     struct ControlRec {
         std::string command;  // control command: "if", "while", etc.
@@ -162,6 +163,8 @@ class Oiiotool {
     TypeDesc input_dataformat;
     int input_bitspersample = 0;
     std::map<std::string, std::string> input_channelformats;
+    ImageSpec m_first_input_dimensions;
+    mutable spin_mutex m_first_input_dimensions_mutex;
 
     // stat_mutex guards when we are merging another ot's stats into this one
     std::mutex m_stat_mutex;
@@ -373,6 +376,27 @@ class Oiiotool {
 
     ColorConfig& colorconfig();
 
+    // Copy the internal 'm_first_input_dimensions` to `dims`.
+    void get_first_input_dimensions(ImageSpec& dims) const
+    {
+        std::lock_guard lock(m_first_input_dimensions_mutex);
+        dims.copy_dimensions(m_first_input_dimensions);
+    }
+
+    bool first_input_dimensions_is_set() const
+    {
+        return m_first_input_dimensions.format.basetype != TypeDesc::UNKNOWN;
+    }
+
+    // If the internal 'm_first_input_dimensions` is not yet set, set it to
+    // the values in `dims`.
+    void set_first_input_dimensions(const ImageSpec& dims)
+    {
+        std::lock_guard lock(m_first_input_dimensions_mutex);
+        if (!first_input_dimensions_is_set())
+            m_first_input_dimensions.copy_dimensions(dims);
+    }
+
 private:
     CallbackFunction m_pending_callback;
     std::vector<const char*> m_pending_argv;