Improve RGB channel detection heuristics for the HT256 encoder

src/lib/OpenEXRCore/internal_ht_common.cpp

@@ -6,34 +6,138 @@
 #include "internal_ht_common.h"
 #include <vector>
 #include <string>
+#include <string.h>
 #include <cassert>
+#include <algorithm>
+#include <cctype>
+
+const std::string RED_CH_FULLNAME = "red";
+const std::string GREEN_CH_FULLNAME = "green";
+const std::string BLUE_CH_FULLNAME = "blue";
+
+struct RGBChannelParams
+{
+    const char* r_suffix;
+    const char* g_suffix;
+    const char* b_suffix;
+    int         r_index;
+    int         g_index;
+    int         b_index;
+    const char* prefix;
+    size_t      prefix_len;
+};
+
+static inline bool areEqual(const char* a, const char* b) {
+#ifdef _MSC_VER
+    return _stricmp (a, b) == 0;
+#else
+    return strcasecmp (a, b) == 0;
+#endif
+}
 
 bool
 make_channel_map (
-    int channel_count, exr_coding_channel_info_t* channels, std::vector<CodestreamChannelInfo>& cs_to_file_ch)
+    int                                 channel_count,
+    exr_coding_channel_info_t*          channels,
+    std::vector<CodestreamChannelInfo>& cs_to_file_ch)
 {
-    int r_index = -1;
-    int g_index = -1;
-    int b_index = -1;
+    /** Heuristic detection of RGB channels so that the JPEG 2000 Reversible
+      * Color Transform (RCT), a decorrelation transform, can be applied.
+      *
+      * RGB channels are present if the either (a) the names of the channels in
+      * their entirety or (b) following the "." character match one of the
+      * triplets defined in {params}. Order of the channels and case of the
+      * channel names are ignored.
+      *
+      * Example 1: "main.b", "main.g", "main.r" match
+      * Example 2: "MainR", "MainG", "MainB" do not match
+      * Example 3: "R", "B", "B" match
+      * Example 4: "red", "green", "blue" match
+      */
 
-    cs_to_file_ch.resize(channel_count);
+    RGBChannelParams params[] = {
+        {"r", "g", "b", -1, -1, -1, NULL, 0},
+        {"red", "green", "blue", -1, -1, -1, NULL, 0},
+        {"red", "grn", "blu", -1, -1, -1, NULL, 0}};
+    constexpr size_t params_count = sizeof (params) / sizeof (params[0]);
+
+    cs_to_file_ch.resize (channel_count);
 
     for (size_t i = 0; i < channel_count; i++)
     {
-        std::string c_name(channels[i].channel_name);
+        const char* channel_name = channels[i].channel_name;
+        const char* suffix       = strrchr (channel_name, '.');
+        const char* prefix       = channel_name;
+        size_t      prefix_len   = 0;
+        if (suffix) {
+            suffix += 1;
+            prefix_len = suffix - prefix - 1;
+        } else {
+            suffix = channel_name;
+        }
+
+        for (size_t j = 0; j < params_count; j++)
+        {
+            if (params[j].prefix != NULL &&
+                (params[j].prefix_len != prefix_len ||
+                 strncmp (params[j].prefix, prefix, params[j].prefix_len)))
+            {
+                /* skip to the next potential match if a prefix has already been
+                record and does not match the channel prefix */
+                continue;
+            }
 
-        if (c_name == "R") { r_index = i; }
-        else if (c_name == "G") { g_index = i; }
-        else if (c_name == "B") { b_index = i; }
+            bool match = false;
+            if (areEqual (suffix, params[j].r_suffix) &&
+                params[j].r_index < 0)
+            {
+                params[j].r_index = i;
+                match = true;
+            }
+            else if (
+                areEqual (suffix, params[j].g_suffix) &&
+                params[j].g_index < 0)
+            {
+                params[j].g_index = i;
+                match = true;
+            }
+            else if (
+                areEqual (suffix, params[j].b_suffix) &&
+                params[j].b_index < 0)
+            {
+                params[j].b_index = i;
+                match = true;
+            }
+
+            if (match) {
+                /* record the prefix if one is not already recorded and move to
+                the next channel */
+                params[j].prefix = prefix;
+                params[j].prefix_len = prefix_len;
+                break;
+            }
+        }
     }
 
-    bool isRGB = r_index >= 0 && g_index >= 0 && b_index >= 0 &&
-                 channels[r_index].data_type == channels[g_index].data_type &&
-                 channels[r_index].data_type == channels[b_index].data_type &&
-                 channels[r_index].x_samples == channels[g_index].x_samples &&
-                 channels[r_index].x_samples == channels[b_index].x_samples &&
-                 channels[r_index].y_samples == channels[g_index].y_samples &&
-                 channels[r_index].y_samples == channels[b_index].y_samples;
+    int  r_index;
+    int  g_index;
+    int  b_index;
+    bool isRGB = false;
+
+    for (size_t j = 0; (!isRGB) && j < params_count; j++)
+    {
+        r_index = params[j].r_index;
+        g_index = params[j].g_index;
+        b_index = params[j].b_index;
+
+        isRGB = r_index > -1 && g_index > -1 && b_index > -1 &&
+                channels[r_index].data_type == channels[g_index].data_type &&
+                channels[r_index].data_type == channels[b_index].data_type &&
+                channels[r_index].x_samples == channels[g_index].x_samples &&
+                channels[r_index].x_samples == channels[b_index].x_samples &&
+                channels[r_index].y_samples == channels[g_index].y_samples &&
+                channels[r_index].y_samples == channels[b_index].y_samples;
+    }
 
     if (isRGB)
     {

src/test/OpenEXRCoreTest/CMakeLists.txt

@@ -118,6 +118,7 @@ define_openexrcore_tests(
  testB44ACompression
  testDWAACompression
  testDWABCompression
+ testHTChannelMap
  testDeepNoCompression
  testDeepZIPCompression
  testDeepZIPSCompression

src/test/OpenEXRCoreTest/compression.cpp

@@ -36,6 +36,9 @@
 #include <ImfOutputFile.h>
 #include <ImfTiledOutputFile.h>
 #include <half.h>
+
+#include "internal_ht_common.cpp"
+
 #ifdef __linux
 #    include <sys/types.h>
 #    include <sys/stat.h>
@@ -1693,6 +1696,60 @@ testDWABCompression (const std::string& tempdir)
     testComp (tempdir, EXR_COMPRESSION_DWAB);
 }
 
+struct ht_channel_map_tests {
+    exr_coding_channel_info_t   channels[6];
+    int                         channel_count;
+    bool                        pass;
+    int                         rgb_index[3];
+};
+
+void
+testHTChannelMap (const std::string& tempdir)
+{
+    std::vector<CodestreamChannelInfo> cs_to_file_ch;
+    ht_channel_map_tests               tests[] = {
+        {{{"R"}, {"G"}, {"B"}}, 3, true, {0, 1, 2}},
+        {{{"r"}, {"G"}, {"b"}}, 3, true, {0, 1, 2}},
+        {{{"B"}, {"G"}, {"R"}}, 3, true, {2, 1, 0}},
+        {{{"red"}, {"green"}, {"blue"}}, 3, true, {0, 1, 2}},
+        {{{"Red"}, {"Green"}, {"Blue"}, {"alpha"}}, 4, true, {0, 1, 2}},
+        {{{"hello.R"}, {"hello.G"}, {"hello.B"}}, 3, true, {0, 1, 2}},
+        {{{"hello.R"}, {"bye.R"}, {"hello.G"}, {"bye.R"}, {"hello.B"}, {"bye.B"}}, 6, true, {0, 2, 4}},
+        {{{"red"}, {"green"}, {"blue"}}, 3, true, {0, 1, 2}},
+        /* the following are expected to fail */
+        {{{"redqueen"}, {"greens"}, {"blueberry"}}, 3, false, {0, 1, 2}},
+        {{{"hello.R"}, {"bye.G"}, {"hello.B"}}, 3, false, {0, 2, 4}},
+    };
+    int test_count = sizeof(tests) / sizeof(ht_channel_map_tests);
+
+    for (size_t i = 0; i < test_count; i++)
+    {
+        EXRCORE_TEST (
+            make_channel_map (
+                tests[i].channel_count, tests[i].channels, cs_to_file_ch) ==
+            tests[i].pass);
+        if (tests[i].pass)
+        {
+            for (size_t j = 0; j < 3; j++)
+            {
+                EXRCORE_TEST (
+                    tests[i].rgb_index[j] == cs_to_file_ch[j].file_index);
+            }
+        }
+    }
+
+    exr_coding_channel_info_t channels_1[] = {
+        { "R" }, { "G" }, { "B" }
+    };
+
+    exr_coding_channel_info_t channels_2[] = {
+        { "R" }, { "G" }, { "1.B" }
+    };
+
+    EXRCORE_TEST (make_channel_map (3, channels_1, cs_to_file_ch));
+    EXRCORE_TEST (! make_channel_map (3, channels_2, cs_to_file_ch));
+}
+
 void
 testDeepNoCompression (const std::string& tempdir)
 {}

src/test/OpenEXRCoreTest/compression.h

@@ -22,6 +22,7 @@ void testB44Compression (const std::string& tempdir);
 void testB44ACompression (const std::string& tempdir);
 void testDWAACompression (const std::string& tempdir);
 void testDWABCompression (const std::string& tempdir);
+void testHTChannelMap (const std::string& tempdir);
 
 void testDeepNoCompression (const std::string& tempdir);
 void testDeepZIPCompression (const std::string& tempdir);

src/test/OpenEXRCoreTest/main.cpp

@@ -206,6 +206,7 @@ main (int argc, char* argv[])
     TEST (testB44ACompression, "core_compression");
     TEST (testDWAACompression, "core_compression");
     TEST (testDWABCompression, "core_compression");
+    TEST (testHTChannelMap, "core_compression");
 
     TEST (testDeepNoCompression, "core_compression");
     TEST (testDeepZIPCompression, "core_compression");