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

Reorder attribute definitions in ImfStandardAttributes.h by functional group #1379

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

JGoldstone
Copy link
Contributor

As discussed in last Thursday's TSC meeting, this is a reordering-only commit in ImfStandardAttributes.h. A good way to verify that is to make a copy of the unmodified file and sort its lines, redirecting the output to file X, and make a copy of the file after this commit and sort its lines, redirecting the output to file Y, and then diff X and Y. Of course both X and Y only contain uncompilable gibberish, but if you look at the output of the diff, the only difference is a few lines of whitespace have been added.

This was my guide to the re-ordering. Attributes in square brackets are actually in ImfHeader.h and not in ImfStandardAttributes.h. Attributes with two asterisks in front of them represent new attributes currently defined in SMPTE's camdkit repository. Otherwise unannotated attributes exist in today's ImfStandardAttributes.h.

[misc]

  • [LineOrder]
  • [Compression]

position and orientation of physical and CG camera at time of capture

  • originalDataWindow
  • worldToCamera
  • worldToNDC
  • [dataWindow]
  • [displayWindow]
  • [pixelAspectRatio]
  • [screenWindowCenter]
  • [screenWindowWidth]
  • xDensity
  • ** something to carry the dimensions of the active sensor area, in physical dimensions
  • longitude
  • latitude
  • altitude

camera ID

  • ** cameraMake
  • ** cameraModel
  • ** cameraSerialNumber
  • ** cameraFirmwareVersion
  • ** cameraUuid
  • ** cameraLabel

camera state

  • ** colorTemperature
  • ** tint
  • ** colorBalance [maybe?]
  • isoSpeed
  • ** exposureIndex [maybe?]
  • expTime
  • ** shutter angle
  • ** captureRate
  • ** timecodeRate

lens ID

  • ** lensMake
  • ** lensModel
  • ** lensSerialNumber
  • ** lensFirmwareVersion

lens state

  • aperture
  • ** deprecate aperture, replace with:
  • ** fNumber
  • ** tStop
  • ** effectiveFocalLength
  • ** nominalFocalLength
  • ** pinholeFocalLength
  • focus

editorial

  • owner
  • comments
  • capDate
  • utcOffset
  • keyCode
  • timeCode
  • framesPerSecond
  • ** imageCounter
  • ** reelName
  • ** FDL [ASC Framing Decision List]

encoded image color characteristics

  • [chromaticities]
  • whiteLuminance
  • adoptedNeutral

anticipated color processing

  • renderingTransform [eventually deprecate and delete]
  • lookTransform [eventually deprecate and delete]

anticipated use in pipeline

  • envmap
  • wrapmodes
  • multiView
  • deepImageState
  • idManifest

camera relationship to a CG environment

  • originalDataView

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 12, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: JGoldstone / name: Joseph Goldstone (0368f5c)

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.

lgtm! @cary-ilm Do we have a CLA in place for Joseph already?

@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 12, 2023 via email

@meshula
Copy link
Contributor

meshula commented Apr 12, 2023

I suspect we have it (hence pinging Cary), and that it predates easycla. So we can shortcut the bot for merging the code with a confirmation, and look at setting you up in easycla separately.

@cary-ilm
Copy link
Member

Either way, somebody from ARRI has to click through the red "NOT COVERED" EasyCLA links and electronically sign the forms. If a previous iteration of the form has been signed, then someone needs to add your name and email to the "approved" list, which I believe is also managed through that link (I've never actually seen that part of the interface because I'm not my org's CLA manager, so I'm in the same boat as you). There should be only one designed ARRI CLA manager, so the "CLA admin at ARRI Munich" should be able to execute for OpenEXR what they did for OCIO.

@cary-ilm
Copy link
Member

In addition to the CLA, the commit also needs a signature, to fix the failing "DCO" check. Do:

git commit -amend -s
git push -f

…l groupings

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

JGoldstone commented Apr 12, 2023 via email

@cary-ilm
Copy link
Member

Yes, that worked, all good, the DCO check has passed, thanks!

You don't regularly need the -amend, it's the -s that you should specify each time.

@JGoldstone
Copy link
Contributor Author

JGoldstone commented Apr 12, 2023 via email

@JGoldstone
Copy link
Contributor Author

My CLA admin in Munich has added me and I've 'repaired' the lack-of-CLA failure above.

Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@cary-ilm cary-ilm merged commit 60611cc into AcademySoftwareFoundation:main Apr 13, 2023
@cary-ilm cary-ilm added the v3.2.0 label Jul 9, 2023
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Aug 9, 2023
As discussed in AcademySoftwareFoundation#1379, these are obsolete. This leaves them in place
but marks them as deprecated.

Signed-off-by: Cary Phillips <cary@ilm.com>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Aug 9, 2023
As discussed in AcademySoftwareFoundation#1379, these are obsolete. This leaves them in place
but marks them as deprecated.

Signed-off-by: Cary Phillips <cary@ilm.com>
cary-ilm added a commit that referenced this pull request Aug 13, 2023
As discussed in #1379, these are obsolete. This leaves them in place
but marks them as deprecated.

Signed-off-by: Cary Phillips <cary@ilm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants