[BUG] OpenEXR PixelAspectRatio

[demoulinv]

Describe the bug

The PixelAspectRatio metadata is not always propagated in the right way to an exr image whether it is set as a float or as a string. It must be set as a float to be correctly propagated. However, when reading back as a float in the image spec, just after the setting as a string, the returned value is correct.

To Reproduce Switch the 2 commented lines of the following code between setting as a float and setting as a string.

    const std::string filename = "simple.exr";
    const int xres = 320, yres = 240, channels = 3;
    float pixels[xres * yres * channels] = { 0.5f };
    std::unique_ptr<OIIO::ImageOutput> out = OIIO::ImageOutput::create(filename);
    if (out)
    {
        std::cout << "save exr file ..." << std::endl;
        OIIO::ImageSpec spec_out(xres, yres, channels, OIIO::TypeDesc::FLOAT);

        float par = 2.0f;
        spec_out.attribute("PixelAspectRatio", par);
        //std::string par = "2.0";
        //spec_out.attribute("PixelAspectRatio", par);

        float par_read = spec_out.get_float_attribute("PixelAspectRatio", 4.0);
        std::cout << "PAR read in image spec before saving: " << par_read << std::endl;

        out->open(filename, spec_out);
        out->write_image(OIIO::TypeDesc::FLOAT, pixels);
        out->close();

        std::unique_ptr<OIIO::ImageInput> in(OIIO::ImageInput::open(filename));
        if (in)
        {
            std::cout << "read exr file metadata ..." << std::endl;

            OIIO::ImageSpec spec_in = in->spec();
            in->close();

            float parFromNewImage;
            spec_in.getattribute("PixelAspectRatio", OIIO::TypeDesc::FLOAT, &parFromNewImage);

            std::cout << "PAR read in image spec of saved image: " << parFromNewImage << std::endl;
        }
        else
        {
            std::cout << "cannot read the new exr file !!!" << std::endl;
        }
    }
    else
    {
        std::cout << "cannot create a new exr file !!!" << std::endl;
    }

Expected behavior What we get when setting as a float (expected behavior):

save exr file ...
PAR read in image spec before saving: 2
read exr file metadata ...
PAR read in image spec of saved image: 2

Evidence What we get when setting as a string:

save exr file ...
PAR read in image spec before saving: 2
read exr file metadata ...
PAR read in image spec of saved image: 1

Platform information:

• OIIO branch/version: pick up from vcpkg
• OS: Windows
• C++ compiler: Visual Studio 2022
• Any non-default build flags when you build OIIO:

[lgritz]

Well, first of all, according to the OpenEXR documentation, it's a float, so I think all bets are off (especially for downstream apps not based on OpenImageIO) if you incorrectly write it as a string. I do not recommend allowing that to happen.

But let's just consider the issue of how flexible OpenImageIO's attribute retrieval can and should be.

spec_in.getattribute("PixelAspectRatio", OIIO::TypeDesc::FLOAT, &parFromNewImage);

I think you are seeing the correct behavior, because the FLOAT here is intentionally restricting the lookup to the specified type, and does not perform any type conversions. The docs for this method say,

If the ImageSpec contains the named attribute and its type matches type, copy the attribute value into the memory pointed to by val ...etc...

(As an aside, you are not checking the return value to be sure the attribute was found at all, in which case the parFromNewImage will be unmodified and therefore have some undefined value because it's uninitialized memory.)

But there is another method that may help:

parFromNewImage = spec.get_float_attribute("PixelAspectRatio", 1.0f);

The get_float_attribute is advertised specifically to be very liberal with the search and do any reasonable type conversions:

Retrieve the named metadata attribute and return its value as a float. Any integer or floating point type will convert to float in the obvious way (like a C cast), and so will string metadata if its contents consist of of the text representation of one floating point value. ...etc...

Additionally, that optional parameter 1.0f at the end provides a default to use for cases where the attribute did not exist in the file.

[demoulinv]

Hi Larry, Thank you for your fast reply. As you mention OpenImageIO is very versatile when it comes to metadata type and that's a great thing. About the specific case of exr image writing, in the OpenImageIO code, https://github.com/AcademySoftwareFoundation/OpenImageIO/blob/master/src/openexr.imageio/exroutput.cpp#L775, you use get_float_attribute to get the pixel aspect ratio before writing it in the exr meta. So I don't understand why the meta is not correctly embedded in the exr image.

[lgritz]

Hi, sorry for the delay over the holidays. I'm still confused about whether there is an actual bug here.

PixelAspectRatio is supposed to be a float, according to the OpenEXR documentation. If you write it as a float, you can read it again as a float when the file is read, presumably recognized by any app or library that reads OpenEXR files by any method.

If you write it (incorrectly) as a string, then when the file is read, it will still be a string. An OIIO-based application could use the get_float_attribute() method and because it does some conversions automatically, you'd get the right value even though it was stored as the wrong type in the file. But if an OIIO-based application uses the getattribute() method restricting the search to a float, it (correctly, as advertised) will not find the string attribute. Additionally, you might expect that any non-OIIO-based application will also not see it, since they wouldn't particularly be expected to figure it out if it was stored with the wrong type.

In the section of code that you reference above, I think you are misunderstanding what it's doing. If the user passes in a value for PixelAspectRatio, it will save that in the file (as whatever type the user specified). In that section of, it is handling the special case of PixelAspectRatio NOT being set by the user, but they did set both XResolution and YResolution, which imply a specific PAR, so we set that.

[demoulinv]

Hi, I agree, it's not really a bug preventing exr images from being written correctly. But perhaps this is a feature that OIIO could take care of. If I incorrectly write PixelAspectRatio as a string, maybe OIIO could convert it to a float before writing it as exr metadata. In this way, the OIIO would allow even more flexibility in how users manage their own metadata.

[lgritz]

My claim is that we already provide the flexibility you want -- on the app side, just use get_float_attribute instead of getattribute, if you want automatic conversion to float. A quick grep seems to reveal that OIIO itself, when it needs to extract PAR from a spec, always uses get_float_attribute, so it will get the automatic conversion. Apps should do that, too. The raw getattribute() call with the FLOAT type selector is meant for when you want no conversion and you want it to not find the attribute if it doesn't have the matching type.

[demoulinv]

I let you test the following piece of code where I use get_float_attribute() instead of getattribute(). I obtain the same result when I write the meta PAR as a string. The conversion does not seem automatically handled by OIIO. For sure, we can live without but I think that could be a nice feature to have.

    const std::string filename = "C:/Temp/simple.exr";
    const int xres = 320, yres = 240, channels = 3;
    float pixels[xres * yres * channels] = { 0.5f };

    std::unique_ptr<OIIO::ImageOutput> out = OIIO::ImageOutput::create(filename);
    if (out)
    {
        std::cout << "save exr file ..." << std::endl;
        OIIO::ImageSpec spec_out(xres, yres, channels, OIIO::TypeDesc::FLOAT);

        //float par = 2.0f;
        //spec_out.attribute("PixelAspectRatio", par);
        std::string par = "2.0";
        spec_out.attribute("PixelAspectRatio", par);

        float par_read = spec_out.get_float_attribute("PixelAspectRatio", 4.0);
        std::cout << "PAR read from created image: " << par_read << std::endl;

        out->open(filename, spec_out);
        out->write_image(OIIO::TypeDesc::FLOAT, pixels);
        out->close();

        std::unique_ptr<OIIO::ImageInput> in(OIIO::ImageInput::open(filename));
        if (in)
        {
            std::cout << "read exr file metadata ..." << std::endl;

            OIIO::ImageSpec spec_in = in->spec();
            in->close();

            float parFromNewImage;
            //spec_in.getattribute("PixelAspectRatio", OIIO::TypeDesc::FLOAT, &parFromNewImage);
            parFromNewImage = spec_in.get_float_attribute("PixelAspectRatio");

            std::cout << "PAR read from new image: " << parFromNewImage << std::endl;
        }
        else
        {
            std::cout << "cannot read the new exr file !!!" << std::endl;
        }
    }
    else
    {
        std::cout << "cannot create a new exr file !!!" << std::endl;
    }