AcademySoftwareFoundation / OpenImageIO

Reading, writing, and processing images in a wide variety of file formats, using a format-agnostic API, aimed at VFX applications.
https://openimageio.readthedocs.org
Apache License 2.0
1.98k stars 597 forks source link

[BUG] OIIO does not compile with NDEBUG in precompile distribution #2882

Open MichaelRoyceCarroll opened 3 years ago

MichaelRoyceCarroll commented 3 years ago

Describe the bug Ubuntu precompiled distributions of OIIO do not compile with NDEBUG defined for xmp.cpp. The image loading output is confusing when metadata is not parsed. Client applications use OIIO to load images and emit unknown tags to stderr. It appears as if either: A. OIIO is not guarding the metadata dropping notices of xmp.cpp source as OIIO intends OR B. the Ubuntu distributions should be building with NDEBUG defined.

To Reproduce Steps to reproduce the behavior:

  1. Install openimageio via precompiled library on Ubuntu 20.10 via package manager. Docker can make this quick. The version i used was from the repository as libopenimageio2.1/groovy, now 2.1.19.0~dfsg0-1build1
  2. Load an image with "offending"/"not catalogued" metadata with ImageInput::open(...), that will drop through to the add_attrib xmp.cpp function bailout... specifically oiiotype is not one of the sections of the if/else blocks.
  3. Observe errors emitted to stderr by openimageio up through the client application. I observed the same issue on Ubuntu 18.04 and Ubuntu 20.04 and 20.10.

Unfortunately, I don't completely follow the metadata labeling tables that are exposing parsing omissions/errors.

Expected behavior Expected behavior is somewhat subjective, but it could be:

  1. Omit reporting metadata that is not parsed by openimageio. Accomplish this by: 1a. Requesting upstream sources like the Ubuntu repository build with NDEBUG on 1b. Use an environment variable or some other compile time guarding mechanism to enable openimageio to verbosely report metadata that is not parsed. Regarding NDEBUG usage, per a colleague: "Yet, if I correctly understand the intention of NDEBUG (as a standard preprocessor define) it is it control the firing of non-debug asserts. It is not meant as a control for “should I be verbose in message reporting”
  2. Refer unknown tags to a source to make them known and ensure developers can reconcile correct behavior

It is possible this issue sighting could go to Ubuntu package maintainers... but perhaps it is best to start in the OIIO repository.

Evidence Do you have error messages? (please quote exactly) Screenshots? Example command lines or scripts that reliably reproduce the error? If it only happens with certain image files, can you attach the smallest image you can make that reproduces the problem? I do not own the images to share for reproduction. Please observe the buildlog for the Ubuntu groovy gorilla package linked through here: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4246-deletedppa/+build/19915647

Notice in the log that the xmp.cpp build omits NDEBUG:

[ 19%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/xmp.cpp.o
cd "/<<PKGBUILDDIR>>/build/src/libOpenImageIO" && /usr/bin/c++  -DEMBED_PLUGINS=1 -DOpenImageIO_EXPORTS -DUSE_BOOST_ASIO=1 -DUSE_DCMTK=1 -DUSE_FFMPEG -DUSE_FREETYPE=1 -DUSE_GIF -DUSE_LIBRAW=1 -DUSE_OCIO=1 -DUSE_OPENCOLORIO=1 -DUSE_OPENCV=1 -DUSE_OPENJPEG -DUSE_OPENVDB=1 -DUSE_STD_REGEX -DUSE_TBB=1 -DUSE_WEBP=1 -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -I"/<<PKGBUILDDIR>>/build/include" -I"/<<PKGBUILDDIR>>/build/src/include" -I"/<<PKGBUILDDIR>>/src/include" -I/usr/include/OpenEXR -I/usr/include/openjpeg-2.3 -I/usr/include/opencv4 -I/usr/include/freetype2  -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC   -Wall -Wno-unused-local-typedefs -Wno-unused-result -Wno-aligned-new -Wno-noexcept-type -fno-math-errno -std=c++11 -Wno-error=placement-new -o CMakeFiles/OpenImageIO.dir/xmp.cpp.o -c "/<<PKGBUILDDIR>>/src/libOpenImageIO/xmp.cpp"

For reference, the code in question from xmp.cpp is from the add_attrib function:

#if (!defined(NDEBUG) || DEBUG_XMP_READ)
    else {
        std::cerr << "iptc xml add_attrib unknown type " << xmlname << ' '
                  << oiiotype.c_str() << "\n";
    }
#endif

I'm using the ImageInput::open routine and observing the unknown reports with pngs and tiffs... apparently via the related call stack: spec(), read_spec(...), decode_xmp(...), add_attrib(...).

At the risk of being pedantic.... Here is an example of tags that fall through for a tiff

iptc xml add_attrib unknown type photoshop:ColorMode unknown
iptc xml add_attrib unknown type photoshop:ICCProfile unknown
iptc xml add_attrib unknown type photoshop:DocumentAncestors unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:parameters unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:parameters unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stRef:instanceID unknown
iptc xml add_attrib unknown type stRef:documentID unknown
iptc xml add_attrib unknown type stRef:originalDocumentID unknown

From a png:

iptc xml add_attrib unknown type photoshop:ColorMode unknown
iptc xml add_attrib unknown type photoshop:DocumentAncestors unknown
iptc xml add_attrib unknown type photoshop:DocumentAncestors unknown
iptc xml add_attrib unknown type photoshop:DocumentAncestors unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type stEvt:action unknown
iptc xml add_attrib unknown type stEvt:instanceID unknown
iptc xml add_attrib unknown type stEvt:when unknown
iptc xml add_attrib unknown type stEvt:softwareAgent unknown
iptc xml add_attrib unknown type stEvt:changed unknown

From another tiff:

iptc xml add_attrib unknown type tiff:ImageWidth unknown
iptc xml add_attrib unknown type tiff:ImageLength unknown
iptc xml add_attrib unknown type tiff:BitsPerSample unknown
iptc xml add_attrib unknown type tiff:BitsPerSample unknown
iptc xml add_attrib unknown type tiff:BitsPerSample unknown
iptc xml add_attrib unknown type tiff:SamplesPerPixel unknown
iptc xml add_attrib unknown type tiff:NativeDigest unknown
iptc xml add_attrib unknown type xap:ModifyDate unknown
iptc xml add_attrib unknown type xap:CreatorTool unknown
iptc xml add_attrib unknown type xap:CreateDate unknown
iptc xml add_attrib unknown type xap:MetadataDate unknown
iptc xml add_attrib unknown type photoshop:ColorMode unknown
iptc xml add_attrib unknown type rdf:parseType unknown
iptc xml add_attrib unknown type xapMM:InstanceID unknown
iptc xml add_attrib unknown type xapMM:DocumentID unknown
iptc xml add_attrib unknown type exif:NativeDigest unknown

My first intuition when seeing these errors was to think I was missing a prerequisite library that contained these attributes. I became really concerned that image data would be parsed incorrectly, or maybe needed to be transformed. Is there any primer on imaging xml attributes? I'm liable to see images written by unknown applications with varying metadata and it could be useful to understand best practices... or what is an acceptable gap w.r.t. OIIO loading.

Platform information:

Thanks for your attention... I hope this report is constructive. Really appreciate OIIO... its a great library.

-MichaelC

MichaelRoyceCarroll commented 3 years ago

FYI: Also... it appears issue https://github.com/OpenImageIO/oiio/issues/1242 has some developer commentary related to this issue.

lgritz commented 3 years ago

I think that several things are going on here that perhaps need untangling and feedback.

  1. A recent PR #2865 improves XMP handling and makes good guesses about the data types, and I think should result in this particular error message almost never getting triggered any more. This is both in the current master as well as the latest supported release, v2.2.12.0. So maybe it's just totally a non-issue. (Can you test 2.2.12 or master and see if any of these errors are still printed for your files that encountered them before, or if the improved xmp handling now parses that metadata without trouble?)

  2. There are a lot of error messages printed, but nearly all are guarded by DEBUG_XMP_READ (i.e., it's only for me to make a change to the source code when debugging to cause it to print). But JUST THIS ONE also happens automatically when in debug build mode. I think maybe (especially in light of the improvements of #2865) we can change that clause because there's no good reason to print that unconditionally for Debug builds. (I'm curious about your feedback/opinion on this.)

  3. Certainly NDEBUG ought to be defined, and that makes me suspicious that these error messages are the least of your problems, and all sorts of other things are in low-performing debug mode or lack flags that are intended to be used for release/optimized builds. Indeed, looking at your build logs, I think the culprit is that when you kick off the build, there is -DCMAKE_BUILD_TYPE=None. Should that not be "Release"?

MichaelRoyceCarroll commented 3 years ago

Hi Larry,

Thanks for the follow up.

  1. Sure, I will test 2.2.12.0 with my application.

  2. I think toggling printing the error message output is useful. It's well managed with an obscure environment variable. i.e. export OIIO_PRINT_UNKNOWN_TAGS=1. I'll admit, my suggested approach is very subjective.

  3. That is likely... I'm not familiar with the Ubuntu build process but it does seem like something is fishy there. -DCMAKE_BUILD_TYPE=None is in the configure step in the hyperlinked log as you suspected.: cd build && cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=None -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_INSTALL_LOCALSTATEDIR=/var -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON -DCMAKE_INSTALL_RUNSTATEDIR=/run -DCMAKE_SKIP_INSTALL_ALL_DEPENDENCY=ON "-GUnix Makefiles" -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_LIBDIR=lib/x86_64-linux-gnu -DBUILD_MISSING_PYBIND11=OFF -DROBINMAP_INCLUDE_DIR=/usr/include/ -DCMAKE_SKIP_RPATH=ON -DINSTALL_FONTS=OFF -DPYTHON_VERSION=3.8 -DSTOP_ON_WARNING=OFF -DUSE_FIELD3D=OFF -DUSE_OPENGL=ON .. -O2 is on the compile line at least for xmp.cpp, so maybe not all is lost optimization wise?? I'm wondering if Noneinstead of Release is the norm for the package management maintainers. I'll look into their issue tracking reporting process.

Performance speculation sidebar: We have a customer that uses OIIO through compositing software plugins. They had been suggesting offhand that OIIO performance was more limited that expected... perhaps this build configuration could be at fault?

-MichaelC

MichaelRoyceCarroll commented 3 years ago

Hi Larry/All,

I filed a request to the repo maintainers via launchpad here:

https://bugs.launchpad.net/ubuntu/+source/openimageio/+bug/1918351

Thanks,

-MichaelC