Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add selected SMPTE camdkit or camdkit-enabling standard optional attributes #1383

Conversation

JGoldstone
Copy link
Contributor

These changes bring to OpenEXR new standard optional attributes that were discussed in the SMPTE RIS [Rapid Industry Solutions] for OSVP [on-set virtual production] working group through about late February or early March 2023. This set represents baseline attributes likely shared by both traditional plate-based non-real-time VFX, and in-camera or on-set real-time VFX.

Additionally, some useful attributes from the SMPTE ACES Container File Layout standard, SMPTE ST 2065-4:2023, have been included as well. At the time ST 2065-4 was defined OpenEXR was close to unmaintained -- this was in fact the genesis of the Academy Software Foundation -- and so attributes defined in ST 2065-4:2013 never appeared in the reference implementation. The ones included here are those judged by the PR author to have passed the test of time.

Things that could use some assistance / scrutiny in this PR:
camdkit defines an areal attribute named ActiveSensorPhysicalDimensions. There is a little-known, never-seen-by-this-author-in-the-wild attribute in the reference implementation named xDensity, probably dating from when most OpenEXR images originated on film. Along with the dataWindow, aspectRatio and perhaps other attributes (screenWindowWidth?), can the same information represented by camdkit as ActiveSensorPhysicalDimensions be represented by starting to use xDensity?

SMPTE ST 2065-4:2013 suffered from a vague definition of aperture as being T-stop if you knew it, otherwise f-number, which meant the artist had no way to tell whether they were getting the one or the other. camdkit defines separate TNumber and FNumber attributes to address this ambiguity. But the OpenEXR reference Implementation has never been ambiguous: in the reference implementation aperture has always clearly meant f-number. Thus this PR leaves aperture alone and adds tStop.

It would be good to have lens manufacturers review those definitions.

There was no carrier of focal length in the reference implementation. This PR adds three types of focal length: nominal (what would appear on a camera report), pinhole (what a minimalist CGI model would use) and effective (what a lens designer uses when employing the classic thick lens model). Again, it would be good to have lens manufacturer review here. In particular, if we introduce the three types of focal length, the comment on the definition of f-number should be clear as to which of the three types of focal length is being divided by the diameter of the lens entrance pupil.

In the existing reference implementation's definition of focus distance, there is no explicit mention that when the lens is in its hyperlocal setting, the focus distance is positive infinity. Should a note be added to this effect?

The PR adds an entrance pupil offset (measured from the image plane). Given two entrance pupil offsets, the larger one is closer to the subject. The entrance pupil is not a physical structure, and it can for some lenses in some configurations be negative, representing the entrance pupil being behind the image plane.

This PR does not add an attribute allowing the measured sensitivity of the camera to be contrasted with the rated sensitivity of the camera, but probably should, before it is merged.

It would be good to understand (from lens manufacturers and from the makers of software that reconstructs lens configuration from a sequence of images) how to deal with anamorphic lenses. Full handling of anamorphic lenses may require carrying two entrance pupils; if so, should there be the proposed entrance pupil offset and an additional attribute, or should the proposed entrance pupil offset attribute be valid only for flat lenses, with two new entrance pupil offsets being defined for anamorphic lenses?

The PR author would welcome suggestions on how to better format or organize the above issues.

…ibutes

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
…w unit test invocations to use same format as previous ones

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
@JGoldstone
Copy link
Contributor Author

I added a framelines attribute with the value being a JSON-encoded string. Note that the top-level key is for the scheme (e.g. ASC FDL) and its value is what's defined by the schema owner. This would allow both camera-vendor-provided frameline info and ASC-provided info to co-exist (though it becomes the responsibility of any application changing one scheme's content to ensure all other schemes' contents are consistently changed).

More trivially the prior additions by this PR to the unit tests for standard attributes lacked a space before the argument list to the assertion, as was the case for all existing tests. This amendment to the PR makes the new ones consistent with the old.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naturally I have some fussy questions for you :)

src/lib/OpenEXR/ImfStandardAttributes.h Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
@meshula
Copy link
Contributor

meshula commented Apr 18, 2023

I didn't spot an attribute for hyperfocal distance. If we are defining focus distance of infinity being at the hyperfocal distance, then don't we need to know what the hyperfocal distance is in order to know what is in focus?

To put it another way, if I have a magical lens with a hyperfocal distance of 30m, how would I prepare this data? would I scale the focus distance at 30m to inf, and 15m to 0.5inf?

Shouldn't the focus distance be a distance in meters, and the hyperfocal distance a value to be used in conjunction?

I can't understand how to use a distance measurement where infinity is defined as at the hyperfocal distance.

@meshula
Copy link
Contributor

meshula commented Apr 18, 2023

If we are going to record measure/actual sensitivity, do we not also need carry other aspects related to exposure such as gain settings, or sensor eV, or any ND filters betwixt aperture and sensor?

@meshula
Copy link
Contributor

meshula commented Apr 18, 2023

ISO in particular is an odd one as well, as I can find no scientific basis for the mapping between a supposed ISO rating of a digital sensor, and a film ISO setting. Is this value not in the realm of feelings and inaccessible secret sauce calculations? That's another argument for recording the eV and gain where possible. density was carried in original EXR as a nod to density readings from scanners.

As such, I don't think we should repurpose density for unrelated usages such as data window size calculations. I think we should relegate ISO and density to the analog world and move on.

@meshula
Copy link
Contributor

meshula commented Apr 18, 2023

finally I think we should measure sensor aperture as such, an x-magnitude in mm and a y-magnitude in mm. Anything else, with aspect ratios and so on gets confusing fast when we need to account for sensor size versus sensor aperture mask. I think sensor size, and sensor aperture should be separately recorded.

I realize many of my notes should have litigated in the committee, but I'm late to this party, and this is my first opportunity.

@lgritz
Copy link
Contributor

lgritz commented Apr 18, 2023

I would love to have a dialog in a future openexr TSC meeting (or openimageio TSC? -- it has recently submitted a proposal to join ASWF) about the right way deal with this expansion of reserved metadata names. In particular, a lot of these correspond to things from Exif and that previously/currently would have passed through with "Exif:..." prefix. So you wonder, should it auto-translate those names on the way in and out, etc.

…ke the relationship between aperture and tStop less ambiguous.

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
…escriptions of entrancePupilOffset and shutterAngle

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 22, 2023 via email

…on of non-default rounding

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
src/lib/OpenEXR/ImfStandardAttributes.h Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
src/lib/OpenEXR/ImfStandardAttributes.h Outdated Show resolved Hide resolved
…mpelling real-world use case

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments modulo the glamorous but not vital question about stamped edge numbers are all addressed.

My take is that this is ready to land.

@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 26, 2023 via email

…as per Cooke Optics

Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
@meshula
Copy link
Contributor

meshula commented Apr 26, 2023

Hi Joseph! Could you file those five issues individually, using the Issues tab at the top of this page? At that point, I can land this, and then further work can be piecemeal, on an attribute by attribute basis, or in aggregate, as more information arrives.

@meshula
Copy link
Contributor

meshula commented Apr 26, 2023

Also, I created a new label "camdkit" that we can use to easily filter issues to find them again.

@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 26, 2023 via email

@meshula
Copy link
Contributor

meshula commented Apr 26, 2023

Great! let's proceed with the conversation in issues & subsequent PRs as they may arise.

@meshula meshula merged commit 2e24872 into AcademySoftwareFoundation:main Apr 26, 2023
@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 26, 2023 via email

@JGoldstone JGoldstone deleted the feature_add_attrs_from_camdkit branch April 27, 2023 13:15
@revisionfx
Copy link

Here's my notes on spatial dimensions:

Theoretically film density would be related to transmission of light (exposure) but as it's documented as X (and in reference to Y computed from PAR/AR) , also in OIIO Google says (I don't use it), I would speculate it was initially related to adding synthetic film grain for CG renders on some basis type of thing??? - perhaps there is someone from ILM then who has a better historical explanation. I remember the old internal compositing app at ILM did not have a concept of project dimension... so it needed this sort of workaround.

Density as a spatial attribute is a slippery slope - but there is a correlation between the way camera set guide/frame lines in sensor dimension (which is in photosite count - note: some cameras now do over-sampling e.g. 5.9K out from 8K sensor - the physical crop sensor dimensionis a window in mm that corresponds to recorded image (pixels) window, and camera vendors tend to use values that generates "good numbers - divide into integers"), proxy resolution, and display scaling (in website and app dev, an OS level display setup or injecting a resizer for large format LED displays when engine cannot sustain resolution at a given frame rate)... related concepts is seen in some pixel shaders kit for support of infinite resolution without explicit references to Width and Height - Anyhow, this value in practice is derivable from physical dimension (is sometimes called pixel shift) corresponding to rendered resolution to sensor cropped window dimension... so I think is redundant if you have XY physical dimensions or vice-versa. SO it would be an option to rebrand Density to mean "pixel shift" (not everyone agrees on that expression either) and assume that value X image window XY gives you the sensor crop physical dimension, where Y is a PAR (for anamorphic...) that defaults to 1.0. Extra redundancy, this is not a normalization to 0 to 1 as in UV map or -1 to 1 as in screen space. Someone will then complain that a random noise field generator does not have explicit W * H resolution so in EXR as not really used for direct in-camera capture these days, which is fair, so it would be appropriate to use pixel physical size rather than a dimension window. Then as you crop to a particular aspect ratio that value does not need to be changed. Note some applications (e.g. Nuke) distinguishes between format and DoD... the later would be for example a blur expanding the image boundary without changing the effective spatial domain (format) where say tracking points, shape masks... are in reference to. Once again, recording 8K on a 35 mm sensor (4320 height) and in post (EXR) converting to 2K (1080 height) via image scaling would require scaling the FKA xdensity, so that equivalent in camera of generating a 2K with the same 8K sensor surface (4:4 oversampling) would result for both EXR creation paths then to have the same physical unit in the end so lens data for example make sense.

Further note, now ray-tracing partially equivalent would be a number of samples, and even that is unclear with biased renderer now relying on denoising...

Pierre

@cary-ilm cary-ilm added the v3.2.0 label Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants