Conversation
ImageBuf::name() and ImageBuf::file_format_name() say which file and which format reader produced an ImageBuf's pixels, but only for the ImageBuf that did the reading. Once the spec is copied, fetched from the ImageCache, or carried into the result of an ImageBufAlgo operation, that answer is gone. Changed behavior: specs that an ImageBuf or the ImageCache reads from a file now carry two new attributes, "oiio:SourcePath" (the name of the file that was opened) and "oiio:SourceFormat" (the reader's format_name()), and ImageBufAlgo results keep them. They therefore show up wherever such specs are listed: ImageBuf::spec(), ImageCache and TextureSystem specs, Python spec dumps, and oiiotool's --printinfo and metadata expressions for images it has read. An ImageInput does not set them, so `oiiotool --info` and `iinfo` of a file are unchanged. ImageOutput::check_open(), which every writer in the library calls from open(), now removes both attributes, so no writer stores them in a file. Before this change the FITS writer wrote every attribute as a header card and would have written the path; the other writers already skip "oiio:" metadata or write only names they know. An application that sets attributes with these names itself will no longer see them in written files, or in ImageOutput::spec() after open(). One private helper sets the attributes at the two places a read fills a spec: ImageBuf's direct read and the ImageCache's per-subimage spec. Not in scope: ImageInput::open() deliberately does not set them. A reader rebuilds its spec when it seeks to a subimage, so they would appear for some formats and not others, and every `oiiotool --info` and `iinfo` reference would change. Nothing in the library reads them yet. The result of an operation with several inputs keeps only the first input's; the documentation says so. testsuite/sourceprovenance makes its own images and checks the attributes after a direct ImageBuf read (including subimage 1 and the native spec), through the ImageCache and a cache-backed ImageBuf, and after an ImageBufAlgo operation. It then writes with every writer in the build and checks that neither the path nor the format name appears in the file's bytes or in any attribute an ImageInput reads back. The oiiotool-control, png and python-imagebuf references gain the two attributes where they print specs of images read from files. Assisted-by: Claude Code / Claude Opus 5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
"Format" can mean either the image file format or the pixel data type (as ImageSpec::format does), so name the new attribute for what it holds: the name of the file format reader that read the image. The doc and comments now say "file format reader" as well. Assisted-by: Claude Code / Claude Opus 5.5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
| // Where the image was read from is never written to a file: the path | ||
| // can reveal private details of the local file system. | ||
| m_spec.erase_attribute("oiio:SourcePath"); | ||
| m_spec.erase_attribute("oiio:SourceFormat"); | ||
|
|
There was a problem hiding this comment.
I'm not sure this is needed. Our policy is that attributes in the output spec that are prefixed with either "oiio:" or "FMT:" (for any FMT that is the name of another format we support) are hints to the OIIO system, or to the format it names. They are meant to be automatically ignored by any writers capable of outputting arbitrary metadata (those that can't, already don't). The logic for OpenEXR is here: for "oiio:" and for other format prefixes.
There was a problem hiding this comment.
Ha! See, I've even done it here -- it's attributes prefixed by the name of a reader, not a file format, that are ignored. I've been very sloppy about this over the years.
|
I'm fine with this, other than the two things I mentioned. Some food for thought: By making the reader have no part in this (which is generally good, as that means they don't all need to replicate the logic, and aren't given the opportunity to screw it up), one thing we leave on the table is the ability to get more granular in the distinction between reader and file format. The ones where it comes to mind are ffmpeg and raw, which each handle a whole bunch of file formats. Is it possible that we would really want to know if it's "avi" rather than "ffmpeg", or "cr2" rather than "raw"? Three (not necessarily mutually exclusive) design choices might be:
|
The FITS writer stored every ImageSpec attribute as a header card, including "oiio:" attributes and attributes prefixed with another format's name, which are hints to OpenImageIO or to that format rather than metadata (the names were also truncated to 8 characters, e.g. "OIIO:COL"). Skip them, as the OpenEXR writer does. Assisted-by: Claude Code / Claude Opus 5.5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
"oiio:SourcePath" and "oiio:SourceFileFormat" are "oiio:" attributes, which writers already treat as hints and do not store, so ImageOutput::check_open() does not need to erase them. The docs now say so, and the sourceprovenance test still checks every writer. Assisted-by: Claude Code / Claude Opus 5.5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
|
Ignore the failures for the VFX2026 tests, they are unrelated and will have its own PR to fix. |
I'm really not sure, although I certainly would not object, if there were a particular use case. And likewise, do we care if the heif reader is reading an avif? (And is our DPX reader also responsible for CIN files, and is that important?) My intent with capturing the "file format" was to provide a centralized location for each format to define default behavior, where color is concerned; and perhaps, down the line, use a custom "extension" syntax like "oiio:dpx:10i" with OCIO FileRules in an internal "oiio-file-format-defaults" failover config, which could also provide the means for users to customize OIIO's default behaviors on a per-format / bitdepth level of granularity... food or thought, as you say! Right now, I think this is probably more of a cross-that-bridge kinda thing, but I'm happy to push this further. |
2 participants
Source file and reader information is lost when an image spec is cached, copied,
or passed through ImageBufAlgo. This records that provenance on specs produced
by
ImageBufandImageCache. Like otheroiio:attributes they are hints,so writers don't store them in files.
Breaking and changed behavior
ImageBufandImageCachegainoiio:SourcePathandoiio:SourceFileFormat.oiio:attributes, or attributes prefixedwith another format's name, as header cards. It was the only writer that
did.
ImageInputspecs are unchanged, soiinfoandoiiotool --infokeeptheir existing output.
Motivation
ImageBuf::name()andfile_format_name()describe the object that performedthe read, not a spec carried beyond it. Applications otherwise have to track
the source out of band.
What this PR does
boundaries.
What this PR deliberately does not do
ImageInput::open()results.Testing
testsuite/sourceprovenancechecks direct and cached reads, native andsubimage specs, ImageBufAlgo propagation, and every writer in the build. It
also verifies that neither attribute appears in file bytes or read-back specs.
Validation
and debug; full CTest 221/221 normal and 221/221 with
OPENIMAGEIO_DEBUG=1.output.
change. Before it, CI passed on my fork, including the Linux gcc, Windows,
sanitizer and ABI jobs: https://github.com/zachlewis/OpenImageIO/actions/runs/36108522310
Documentation
stdmetadata.mddocuments names, boundaries, propagation, and that writers don't store them.Part of #5510.
Assisted-by: Claude Code / Claude Opus 5