Require that gainMap->gainMapMax[c] be greater than or equal to gainMap->gainMapMin[c]

[wantehchang]

While searching for the word "shall" in ISO/CD 21496-1, I found the following requirements:

5.2.5.3 Per‐component max values
[...]
For each component, maxሺGሻ shall be greater than or equal to the minሺGሻ value (5.2.5) for that
component.

I checked src/read.c and src/gainmap.c. It seems that we are not checking this requirement. Could you please double check?

Also, in src/gainmap.c, I see the following code:

    // Scale the gain map values to map [min, max] range to [0, 1].
    for (int c = 0; c < numGainMapChannels; ++c) {
        const float range = gainMapMaxLog2[c] - gainMapMinLog2[c];
        if (range <= 0.0f) {
            continue;
        }
        ...
    }

Is the continue statement correct?

It seems that avifFindMinMaxWithoutOutliers() may return *rangeMin greater than *rangeMax because of the following code:

    int leftOutliers = 0;
    for (int i = 0; i < numBuckets; ++i) {
        leftOutliers += histogram[i];
        if (leftOutliers > maxOutliersOnEachSide) {
            break;
        }
        if (histogram[i] == 0) {
            // +1 to get the higher end of the bucket.
            *rangeMin = avifBucketIdxToValue(i + 1, min, max, numBuckets);
        }
    }

    int rightOutliers = 0;
    for (int i = numBuckets - 1; i >= 0; --i) {
        rightOutliers += histogram[i];
        if (rightOutliers > maxOutliersOnEachSide) {
            break;
        }
        if (histogram[i] == 0) {
            *rangeMax = avifBucketIdxToValue(i, min, max, numBuckets);
        }
    }

If both the for loops execute their respective if (histogram[i] == 0) statement and histogram[i] is equal to 0 for only one i, then I think *rangeMin will be greater than *rangeMax. Can this happen?

[maryla-uc]

Indeed we don't seem to check that max>=min. This requirement did not exist in early versions of the gain map concept. We can add a check in avifGainMapValidateMetadata.

I think the continue is meant to skip range == 0 (because we divide by range). Although it was written as range <= 0, it should not be possible for the range to be negative. The scenario you described cannot happen because if there is only one i where the histogram is 0, then at least one of the two loops will break in the if above. I can add more tests to check.

The case range == 0 can happen if the gain map has the same value for all pixels. In this case when denormalizing the gain map it will be multiplied by (max-min) i.e. by 0 so the values in the gain map don't actually matter. ... Except I think we still need to clamp them because otherwise we'll end up with values outside of [0;1] and will hit the assert in avifSetRGBAPixel.

[wantehchang]

Indeed we don't seem to check that max>=min. This requirement did not exist in early versions of the gain map concept. We can add a check in avifGainMapValidateMetadata.

Thank you for the confirmation. I wrote #2577 to add a check in avifGainMapValidateMetadata.

Thanks also for answering my other questions.

[wantehchang]

I abandoned my pull request #2577 because we also need to update ArbitraryAvifImageWithGainMap so that per-channel max >= per-channel min.