-
Notifications
You must be signed in to change notification settings - Fork 616
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
Add selected SMPTE camdkit or camdkit-enabling standard optional attributes #1383
Conversation
…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>
I added a 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. |
There was a problem hiding this 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 :)
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. |
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? |
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. |
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. |
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>
No, we are not ‘omitting’ owner. My mistake was believing that I had added ‘owner’ to the standard optional attributes. In fact, it was already there, vague definition and all.
By the way there is a similarly vague definition of ‘creator’ in the ACES Container spec:

Obviously I don’t want to bring creator over.
ARRI’s proprietary OpenEXR metadata contains several other attributes, the names of which reflect the believe that *eventually* they would make their way into the reference implementation:

At some point we could open an Issue to discuss how many of ARRI_ACES_CONTAINER_ATTRIBUTE_STRING_EMP attributes deserved to be carried over into the OpenEXR reference implementation .The documentation we used to supply with the old ARRIMetadataBridge software explains the ‘interim.<xxx>.' prefix: it was designed so that if you found a file with both foo and interim.foo, you knew to go with the attribute named without the prefix.
But that’s for someday. As things stand, we don’t touch ‘owner’ in this round.
… On Apr 20, 2023, at 23:53, Nick Porcino ***@***.***> wrote:
@meshula commented on this pull request.
In src/lib/OpenEXR/ImfStandardAttributes.h <#1383 (comment)>:
> @@ -167,12 +468,18 @@ IMF_STD_ATTRIBUTE_DEF (focus, Focus, float)
//
// owner -- name of the owner of the image
//
+// If present, the value should contain only printable characters
Per tsc conversation, are we omitting "owner"?
—
Reply to this email directly, view it on GitHub <#1383 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIVB7ILD5JLVCKMEWPCPRLXCIADXANCNFSM6AAAAAAXBZQLD4>.
You are receiving this because you authored the thread.
|
…on of non-default rounding Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
…mpelling real-world use case Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
There was a problem hiding this 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.
First, I agree that we have something that can be merged into main. I’m just new enough at this that I don’t know how to tie off any loose ends. I did click on ‘Resolve conversation’ for the KeyKode thread.
Second, I went over the PR and my notes and identified the following five issues to file so we don’t lose them.
Figure out a use case that justifies the addition of timecodeRate that both the OpenEXR TSC and SMPTE RIS can agree is illustrative.
Convey the information described in camdkit's ActiveSensorPhysicalDimensions.
Ensure that there are no conflicts between the discussion of coordinate systems in SMPTE ST 2065-4 ("ACES Container File Format") and the discussion in the Technical Introduction to OpenEXR (section "Projection, camera coordinate system and screen window" and the illustration immediately above it). Make it clear what *part* of the camera we mean when we say the camera is 'located at the origin'. If it's the entrance pupil, that will be lots of fun, since the entrance pupil can be all over the place, even behind the image plane. Does that mean that in that case, the camera's location is considered to be behind its own image plane?
When we get feedback from Cooke Optics' engineers, refine the description of aperture to indicate whether the lens focal length that is the numerator of the ration of focal length to diameter of lens entrance pupil is a nominal focal length, an effective focal length or a pinhole focal length. Make a similar refinement for tStop.
Clarify how entrance pupil is handled for anamorphic lenses. With an anamorphic lens there will be two entrance pupil positions. I would recommend adding a second entrance pupil offset and adding text to the comment for plain-old entrancePupilOffset saying which entrance pupil is being described in the anamorphic case.
Are there others that I should file as issues? Should any of the above NOT be filed?
Third, if we are all agreed what we have can be merged, then can someone who has the proper permissions actually do the merge into main?
Thank you
… On Apr 25, 2023, at 21:22, Nick Porcino ***@***.***> wrote:
@meshula approved this pull request.
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.
—
Reply to this email directly, view it on GitHub <#1383 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIVB7O2RVB7CJUL6TNTNNLXDB2GRANCNFSM6AAAAAAXBZQLD4>.
You are receiving this because you authored the thread.
|
…as per Cooke Optics Signed-off-by: Joseph Goldstone <jgoldstone@arri.com>
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. |
Also, I created a new label "camdkit" that we can use to easily filter issues to find them again. |
Of course. I just wanted your agreement that they were legit and distinct before doing so.
BTW I just made one last change, not in the code but in the comments. Ian Sheret of Cooke Optics got back to me and said that, when one is trying for just slightly better than the normal definition of f-number as focal length over entrance pupil diameter, it’s effective focal length that should be on top, specifically. And not nominal, and not pinhole. I hope the clarification didn’t introduce any grammatical errors. Anyway, I pushed that up maybe 5 minutes ago.
… On Apr 26, 2023, at 12:53, Nick Porcino ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub <#1383 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIVB7JQZY3RZC276XIT7ADXDFHI5ANCNFSM6AAAAAAXBZQLD4>.
You are receiving this because you authored the thread.
|
Great! let's proceed with the conversation in issues & subsequent PRs as they may arise. |
Four issues of the file filed — the fifth was resolved as part of this PR. Thanks for the merge.
… On Apr 26, 2023, at 13:54, Nick Porcino ***@***.***> wrote:
Great! let's proceed with the conversation in issues & subsequent PRs as they may arise.
—
Reply to this email directly, view it on GitHub <#1383 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIVB7PUFTG2UW2OTEFEZF3XDFOLXANCNFSM6AAAAAAXBZQLD4>.
You are receiving this because you authored the thread.
|
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 |
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 namedxDensity
, probably dating from when most OpenEXR images originated on film. Along with thedataWindow
,aspectRatio
and perhaps other attributes (screenWindowWidth
?), can the same information represented by camdkit asActiveSensorPhysicalDimensions
be represented by starting to usexDensity
?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 separateTNumber
andFNumber
attributes to address this ambiguity. But the OpenEXR reference Implementation has never been ambiguous: in the reference implementationaperture
has always clearly meant f-number. Thus this PR leavesaperture
alone and addstStop
.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.