Improve RGB channel detection heuristics for the HT256 encoder

[palemieux]

Updated algorithm:

  * 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

[peterhillman]

This will consider R as a red channel, but not a channel which has a layer name with a R suffix e.g.diffuse.R?
Also, if the image happens to have both a R and a r channel, it'll pick the r channel as the red channel not R, which is a little unfortunate, since the standard name is R

Ideally it should look for R as the channel name, and only fall back to using a channel with a non-standard name, or whose name starts with a layer name, if no R channel is found.

[palemieux]

Proposed algorithm:

• look for R, G and B (case sensitive)
• if not found, look for:
  • r, g and b
  • .r, .g and .b suffixes (case insensitive)
  • red, green, blue (case insensitive)

Anything else, like:

• -red, -green and -blue suffixes (case insensitive)
• red*, green* and blue* (case insensitive)
• -r, -g and -b suffixes (case insensitive)

[peterhillman]

The first two points seem sensible. The standard says that a red channel must be R, or a *.R. It's quite common to see .red as a suffix, and it would be worth checking for that too, as well as the full channel name

Being too generous with names may cause problems, such as redrawn.G being treated as a red channel, or blueberry.R as a blue channel.

[palemieux]

What about the RGBA interface? It looks like it simply appends R, G and B to a common prefix. If so, should the encoder look for R, G and B at the end of the channel name?

[palemieux]

NVM, it looks like the prefix in the RGBA interface is computed with prefixFromLayerName(), which ensures a . is present.

[palemieux]

See revised algorithm.

[meshula]

The algorithm below captures two years of use and feedback from many contributors and is believed to be good for this purpose.

// strcasecmp is not available in the MSVC stdlib.
#ifdef _MSC_VER
#define strcasecmp _stricmp
#endif

static bool strIsComponent(const char* layerName, 
                           const char* str, 
                           const char* component)
{
    if (!str || !component)
        return false;

    // if the layername is specified, it must match the beginning of the string.
    // if layername was specified, move the string pointer past it
    if (layerName) {
        if (strncmp(layerName, str, strlen(layerName)) != 0)
            return false;
        str += strlen(layerName);
    }

    // search backwards in str for a dot. If found, move the string pointer
    // past it
    const char* dot = strrchr(str, '.');
    if (dot) {
        str = dot + 1;
    }

    // if one character is left, then do a case insensitive comparison with the
    // first character of the component.
    if (strlen(str) == 1) {
        return tolower(str[0]) == tolower(component[0]);
    }

    // The final check is to return true if a case insensitive comparison of
    // the string with the component is true.
    return strcasecmp(str, component) == 0;
}

static exr_result_t _nanoexr_rgba_decoding_initialize(
    exr_context_t exr,
    nanoexr_ImageData_t* img,
    const char* layerName,
    int partIndex, exr_chunk_info_t* cinfo, exr_decode_pipeline_t* decoder,
    int* rgba)
{
    exr_result_t rv = EXR_ERR_SUCCESS;
    rv = exr_decoding_initialize(exr, partIndex, cinfo, decoder);
    if (rv != EXR_ERR_SUCCESS) {
        fprintf(stderr, "exr_decoding_initialize failed: %s\n", 
                exr_get_default_error_message(rv));
        return rv;
    }
    int bytesPerChannel = nanoexr_getPixelTypeSize(img->pixelType);
    for (int i = 0; i < img->channelCount; ++i) {
        rgba[i] = -1;
    }
    for (int c = 0; c < decoder->channel_count; ++c) {
        int channelIndex = -1;
        decoder->channels[c].decode_to_ptr = NULL;
        if (strIsComponent(layerName, decoder->channels[c].channel_name, "red")
            || (rgba[0] == -1 && strIsComponent(layerName, decoder->channels[c].channel_name, "y"))) {
            rgba[0] = c;
            channelIndex = 0;
        }
        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "green")) {
            rgba[1] = c;
            channelIndex = 1;
        }
        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "blue")) {
            rgba[2] = c;
            channelIndex = 2;
        }
        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "alpha")) {
            rgba[3] = c;
            channelIndex = 3;
        }
        if (channelIndex == -1 || channelIndex >= img->channelCount) {
            continue;
        }
        // calculate start of pixel data for the target channel
        decoder->channels[c].decode_to_ptr = (uint8_t*) (ptrdiff_t) (channelIndex * bytesPerChannel);
    }
    return rv;
}

[meshula]

The rgba detection is much more robust now! However, there is one more fly in the ointment.

The algorithm I posted takes a layer name. The point there is that if an exr image contains layers named say

albedo.{rgba}
beauty.{rgba}

one should take care to group albedo.{rgba} together, and beauty.{rgba} together. A fault that can arise if you don't also aggregate layer names is this:

albedo.{rgb} beauty.{rgba}

will yield rgba = {albedo.r, albedo.g, albedo.b, beauty.a }

or this scenario:

odd.{rb}
even.{ga}

yielding rgba={odd.r, even.g, odd.b, even.a}

[meshula]

Also, I was wondering about the red, grn, blu, suffixes. In the code I posted we only allow red, green, blue, and r, g, b. I've never seen grn, blu arise, and was curious if you use tools that produce them?

[palemieux]

Also, I was wondering about the red, grn, blu, suffixes. In the code I posted we only allow red, green, blue, and r, g, b. I've never seen grn, blu arise, and was curious if you use tools that produce them?

This came from the DWA code.

[palemieux]

The rgba detection is much more robust now! However, there is one more fly in the ointment.

To summarize: the prefix, if it exists, should match across the R, G and B channels, right?

[palemieux]

@meshula I have added RGB detection heuristics tests at src/test/OpenEXRCoreTest/compression.cpp. I would appreciate your quick review. Feel free to suggest additional combinations.

[meshula]

I still wonder if there's a tool that produces red, gin, blu, in other words, what's the motivation or including them?

[palemieux]

I am happy to remove them... your call.

[meshula]

My opinion is that it suggests to the reader of the code that those abbreviations are expected as legal, but we don't do that anywhere else so I prefer to elide them without something to ground them.

[meshula]

This version seems robust against the combinations I can think of, thanks!

[meshula]

thanks