Extend exrstdattr to add -erase option (#1688)

* add -erase option to exrstdattr

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

* reorder operations and add test for erase in exrstdattr

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

---------

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

src/bin/exrstdattr/main.cpp

@@ -206,6 +206,7 @@ usageMessage (ostream& stream, const char* program_name, bool verbose = false)
                "\n"
                "Other options:\n"
                "\n"
+	       "  -erase s      remove attribute with given name\n" 
                "  -h, --help    print this message\n"
                "      --version print version information\n"
                "\n"
@@ -224,7 +225,17 @@ struct SetAttr
     {}
 };
 
+struct EraseAttr
+{
+    string name;
+    int part;
+     EraseAttr (const string& name, int part)
+         : name (name), part (part)
+     {}
+};
+
 typedef vector<SetAttr> SetAttrVector;
+typedef vector<EraseAttr> EraseAttrVector;
 
 void
 isNonNegative (const char attrName[], float f)
@@ -445,6 +456,8 @@ getNameAndString (int argc, char** argv, int& i, int part, SetAttrVector& attrs)
     i += 3;
 }
 
+
+
 void
 getNameAndFloat (int argc, char** argv, int& i, int part, SetAttrVector& attrs)
 {
@@ -467,6 +480,17 @@ getNameAndInt (int argc, char** argv, int& i, int part, SetAttrVector& attrs)
     i += 3;
 }
 
+void
+getName (int argc, char** argv, int& i, int part, EraseAttrVector& attrs)
+{
+    if (i > argc - 2) throw invalid_argument ("Expected a name and an integer");
+
+    const char* attrName = argv[i + 1];
+    attrs.push_back (EraseAttr(attrName, part));
+    i += 2;
+}
+
+
 void
 getChromaticities (
     const char     attrName[],
@@ -603,6 +627,7 @@ main (int argc, char** argv)
         const char* outFileName = 0;
 
         SetAttrVector attrs;
+        EraseAttrVector eraseattrs;
         int           part = -1;
         int           i    = 1;
 
@@ -726,6 +751,10 @@ main (int argc, char** argv)
             {
                 getNameAndInt (argc, argv, i, part, attrs);
             }
+            else if (!strcmp (argv[i], "-erase"))
+            {
+                getName ( argc,argv,i,part,eraseattrs);
+            }
             else if (!strcmp (argv[i], "-h") || !strcmp (argv[i], "--help"))
             {
                 usageMessage (cout, "exrstdattr", true);
@@ -771,10 +800,52 @@ main (int argc, char** argv)
         int                numParts = in.parts ();
         vector<Header>     headers;
 
+        //
+        // Treat attributes added to a header in its constructor
+        // as critical and don't allow them to be deleted.
+        // 'name' and 'type' are only required in multipart
+        // file and errors will be reported if they
+        // are erased
+        //
+        Header stdHdr;
+
         for (int part = 0; part < numParts; ++part)
         {
             Header h = in.header (part);
 
+            //
+            // process attributes to erase first, so they can be reinserted
+            // with a different type
+            //
+            for (size_t i = 0 ; i < eraseattrs.size() ; ++i)
+            {
+                const EraseAttr& attr = eraseattrs[i];
+                if (attr.part == -1 || attr.part == part)
+                {
+                    if( stdHdr.find(attr.name)!=stdHdr.end() )
+                    {
+                        cerr << "Cannot erase attribute " << attr.name
+                             << ". "
+                             << "It is an essential attribute" << endl;
+                        return 1;
+                    }
+                    h.erase( attr.name );
+                }
+                else if (attr.part < 0 || attr.part >= numParts)
+                {
+                    cerr << "Invalid part number " << attr.part
+                         << ". "
+                            "Part numbers in file "
+                         << inFileName
+                         << " "
+                            "go from 0 to "
+                         << numParts - 1 << "." << endl;
+
+                    return 1;
+                }
+            }
+
+
             for (size_t i = 0; i < attrs.size (); ++i)
             {
                 const SetAttr& attr = attrs[i];
@@ -801,7 +872,7 @@ main (int argc, char** argv)
         }
 
         //
-        // Crete an output file with the modified headers,
+        // Create an output file with the modified headers,
         // and copy the pixels from the input file to the
         // output file.
         //

src/test/bin/test_exrstdattr.py

@@ -20,6 +20,10 @@
 fd, outimage = tempfile.mkstemp(".exr")
 os.close(fd)
 
+fd, outimage2 = tempfile.mkstemp(".exr")
+os.close(fd)
+
+
 def cleanup():
     print(f"deleting {outimage}")
 atexit.register(cleanup)
@@ -138,4 +142,27 @@ def cleanup():
     print(result.stdout)
     raise
 
+# test for bad erase argument
+result = run ([exrstdattr, "-erase"], stdout=PIPE, stderr=PIPE, universal_newlines=True)
+print(" ".join(result.args))
+print(result.stderr)
+assert(result.returncode != 0), "\n"+result.stderr
+
+# test for errors trying to delete a critical attribute
+result = run ([exrstdattr, "-erase","dataWindow",outimage,outimage2], stdout=PIPE, stderr=PIPE, universal_newlines=True)
+print(" ".join(result.args))
+print(result.stderr)
+assert(result.returncode != 0), "\n"+result.stderr
+
+# test deleting 'comments'
+result = run ([exrstdattr, "-erase","comments",outimage,outimage2], stdout=PIPE, stderr=PIPE, universal_newlines=True)
+print(" ".join(result.args))
+assert(result.returncode == 0), "\n"+result.stderr
+assert(os.path.isfile(outimage2)), "\nMissing " + outimage2
+
+result = run ([exrinfo, "-v", outimage2], stdout=PIPE, stderr=PIPE, universal_newlines=True)
+print(" ".join(result.args))
+assert("comments" not in result.stdout)
+
+
 print("success")