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

Collated Feedback from the Studio Config #67

Open
KelSolaar opened this issue Aug 10, 2022 · 12 comments
Open

Collated Feedback from the Studio Config #67

KelSolaar opened this issue Aug 10, 2022 · 12 comments
Labels
configs for separation purposes

Comments

@KelSolaar
Copy link
Contributor

KelSolaar commented Aug 10, 2022

@flord

  • There is no Arri input space in the studio config? That is almost the only one we need!
  • There is no linear sRGB colorspace? I understand it's the same as linear Rec.709, but I would expect sRGB to be mentioned somewhere to avoid any confusion. linear Rec.709/sRGB perhaps...
  • In Nuke, when loading a .mov file, it seems to be hard-coded to use the Gamma2.2 space. With either config, Nuke throws an error because that space does not exist. How are we going to deal with that? Tell Foundry to stop hard coding spaces?
  • In Nuke, there is only one menu for displays and display spaces. This makes the raw space being repeated multiple times, making the list of spaces even longer. I guess we also need to ask Foundry to fix that.

image

@MrLixm

Hello, great to see this discussion. A thing I'm wondering is why the unique identifier of the colourspace is also the pretty name used for interfaces? Shouldn't decoupling the UI and the logic a better idea to allow for more flexible configs that can be updated at any moment ?

SImple naive example :

  - !<ColorSpace>
    identifier: displaysrgb
    name: True-PRO sRGB Display
    family: Display
    equalitygroup: ""
    bitdepth: 32f
    description: Convert CIE XYZ (D65 white) to sRGB (piecewise EOTF)

Where anyone could just change the name and it wouldn't break anything in any software.

This could then open a lot of other discussions (identifiers formats, identifiers conventions, ...) but just wanna know your thought about this ? Cheers. Liam.

@anderslanglands

I understand that it may be too late but it would be really nice if compact, unique identifiers could be split from the “display name”, especially if the display name is forced to have extra context to be used in isolation.

For instance, some applications will bake the colour space name into file names (substance does this when exporting) and with the current ACES configs this results in some absolutely horrific file names. Plus all those spaces are just bugs waiting to happen.

As someone who likes to think he knows a little about colour, but doesn’t know OCIO very well, I find the current naming in the ACES configs extremely confusing - eg which sRGB is actually sRGB? Or are they both the same thing? (I still don’t know the answer to this).

@KelSolaar

  • Rec. 709/Rec. 1886 for display names vs Rec.709/Rec.1886 for colorspaces.
  • Ensure that colorspaces version is updated when we update the generator.

@scoopxyz

  • Just a small comment to include a link to the spreadsheets that are used as the "ground truth" for naming, etc. in the README.md if you can :)

@nick-shaw

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

Bartłomiej Styczeń

@Thomas Mansencal
so I finally got to this, and all seems to go well until I specify a BuiltinTransform Style in the CSV, I tried with these:
ACEScc_to_ACES2065-1
ACEScct_to_ACES2065-1
ACEScg_to_ACES2065-1
ACESproxy10i_to_ACES2065-1
And they all crash the generator with
INFO:opencolorio_config_aces.config.cg.generate.config:Creating a "Colorspace" transform for "ACEScc_to_ACES2065-1" style...
Traceback (most recent call last):
File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/studio/generate/config.py", line 274, in
config, data = generate_config_studio(
File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/studio/generate/config.py", line 228, in generate_config_studio
_config, data = generate_config_cg(
File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/cg/generate/config.py", line 997, in generate_config_cg
colorspace = style_to_colorspace(**kwargs)
File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/cg/generate/config.py", line 415, in style_to_colorspace
[beautify_alias(signature["name"])] + signature["aliases"]
KeyError: 'name'
Before I delve into the source code, just wanted to ask if it’s something you recognise and maybe the problem is as trivial as error in the BuiltinTransform Style name

After some debugging and slight modification to the code and csv, I got a config, but not really sure about the correctness of it.
The problem I mentioned above is due to a signature["name'] never being set. I have no clue where that name should come from so I just fed it style name.
Later on there was a problem with the config version:
PyOpenColorIO.Exception: Error building YAML: Only config version 2.1 (or higher) can have BuiltinTransform style 'ACES-LMT - ACES 1.3 Reference Gamut Compression'.
It seems like the generation starts with the proper 2.1 version, but then it is changed midway to 2.0 in
config/cd/generate/config.py:
1085: data.profile_version = VersionData(2, 0)
If that was intended, can I just remove that line or will it mess up the further logic? (edited)

Sidenote: Dockerfile seems to be a bit out of date, it’s based on old ci-ocio that uses python 3.7 and one of the requirements requires python >= 3.8 (numpy 1.22 if I remember correctly).
I’ve used FROM aswf/ci-ocio:2022 and it built fine, but maybe that can cause some issues with the generator?

@mdecaria

Hi, congratulations on the beta config release, just have some general questions/feedback on the Studio Config

  • I'm curious about the reason behind no Linear P3-D60? Why have P3-D60 Display and D60 sims if the linear equivalent isn't available to help with those workflows?
  • How are the aliases' names chosen? Some seem a little inconsistent/missing.
    srgb_texture would be nice to have on sRGB - Texture
    Should ST-2084 - Curve have the st2084_crv alias for consistency with the display name?
    Why does Linear Rec.2020 get a combined alias with no underscore but other spaces don't?
  • For categories labels - ACEScg is a working-space, Linear Rec.709 is also one; although not typical, Linear P3-D65 could be labelled as one?
  • As the DCDM ODTs are now omitted ( sadly ), what's the best re-implementation for backwards compatibility sake besides relying on the previous LUTs?

@remia

I looked quickly at the studio config, nothing really important to report or probably already known:

  • The config generates the following warning: "[OpenColorIO Warning]: file_rules: defines a default rule using color-space 'ACES2065-1' that does not match the default role 'sRGB - Texture'.",
  • The '/' family separator is used in some colorspaces which cause additional menu hierarchies in Nuke but seems to be handled correctly with the newer app helpers so just need to update the UI,
  • NamedTransform for ST2084 curve encoding attribute is "log", might be "hdr-video" but I guess the former also makes sense strictly speaking,
  • I was a bit mislead by the OCIO version in the config name, it seems to be the one used by the generator but not the one to be used by the end-user. As in if it was the case I would expect the OCIO 2.1.2 studio config to include the RGC and have a 2.0.4 version without it,
  • Testing on ociodisplay exposed a crash on Linux when there is no Looks in the OCIO config and you hover the Looks override menu (more of a note for OCIO dev to look at).

Great work everyone!

@zachlewis

My quickie beef -- can we get rid of the " - Display" part of the Display names? Gets to be quite a bit to type.

I understand that we want to keep " - Display" in the Display Color Spaces names, and that's fine. But maybe instead of using Shared Views for all the ACES-based views, we can use plain-ol regular views.

Derek Flood

I see that the Canon CLFs have been updated for C-Log3 and its corresponding linear.
#71

Is it planned to also support the C-Log2? Or is C-Log2 depreciated/unsupported?

Just seeking education/understanding, thanks!

@KelSolaar KelSolaar changed the title Collated Feedback from the Studio Release Collated Feedback from the Studio Config Aug 10, 2022
@MrLixm
Copy link

MrLixm commented Aug 10, 2022

Hello, thanks for porting this here.
Perhaps me and Anders comments should be ported to the OpenColorIO repository?

@KelSolaar
Copy link
Contributor Author

@MrLixm : We touched on a similar topic during the last meeting: https://wiki.aswf.io/display/OCIO/2022-08-02

We will get back to it soon.

Note that I ported the new slugification code from Colour to generate aliases automatically, maybe this could be used.

Cheers,

Thomas

@nick-shaw
Copy link

nick-shaw commented Aug 11, 2022

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

@nick-shaw
Copy link

nick-shaw commented Aug 11, 2022

Correction. It's not alphabetically before it in terms of name, but it does come before in the Reference config. Is that ordered based on CTL file name? If they were in alphabetical order, "ACES 1.3 Reference Gamut Compression" would come first, and become the default. Problem solved.

@KelSolaar
Copy link
Contributor Author

Hey @nick-shaw,

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

Great question with a simple answer, whilst the current VFX Reference Platform mandates OCIO 2.1 as the version for 2022. Many applications, e.g. Nuke, are still on the 2.0 version of the library which does not have the ACES 1.3 Reference Gamut Compression transform. So rather than having a config that cannot be loaded in those applications, we decided to only leave it in the Reference config.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

It is because of the Ordering value it has in the spreadsheet and its position within it:

image

We should simply change the Ordering value of the Blue Light Artifact Fix to 315 and that would push it down.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

This is a good one, and we lost that when converting the Colorspace from 1.2 to a Look. There is unfortunately no API for that: https://opencolorio.readthedocs.io/en/latest/api/look.html. @doug-walker and @hodoulp for VIS.

@nigelhadley-foundry
Copy link

Hello @KelSolaar @nick-shaw

Nuke 14 (latest main) is at OCIO 2.1
The CG and the Studio configs are being added now (ie today!) so Nuke will have 2.1 OCIO and ACES 1.3 for the upcoming 14.0 Beta

@KelSolaar
Copy link
Contributor Author

This is awesome @nigelhadley-foundry! Just to be sure, you picked the v0.3.1 release right? v0.3.0 had some incorrect directions set.

@KelSolaar
Copy link
Contributor Author

Self-Reminder: Ensure that colorspaces version is updated when we update the generator.

@scoopxyz
Copy link
Contributor

Just a small comment to include a link to the spreadsheets that are used as the "ground truth" for naming, etc. in the README.md if you can :)

@KelSolaar
Copy link
Contributor Author

Thank you good point @scoopxyz !

@remia
Copy link

remia commented Sep 8, 2022

I looked quickly at the studio config, nothing really important to report or probably already known:

  • The config generates the following warning: "[OpenColorIO Warning]: file_rules: defines a default rule using color-space 'ACES2065-1' that does not match the default role 'sRGB - Texture'.",
  • The '/' family separator is used in some colorspaces which cause additional menu hierarchies in Nuke but seems to be handled correctly with the newer app helpers so just need to update the UI,
  • NamedTransform for ST2084 curve encoding attribute is "log", might be "hdr-video" but I guess the former also makes sense strictly speaking,
  • I was a bit mislead by the OCIO version in the config name, it seems to be the one used by the generator but not the one to be used by the end-user. As in if it was the case I would expect the OCIO 2.1.2 studio config to include the RGC and have a 2.0.4 version without it,
  • Testing on ociodisplay exposed a crash on Linux when there is no Looks in the OCIO config and you hover the Looks override menu (more of a note for OCIO dev to look at).

Great work everyone!

@zachlewis
Copy link

My quickie beef -- can we get rid of the " - Display" part of the Display names? Gets to be quite a bit to type.

I understand that we want to keep " - Display" in the Display Color Spaces names, and that's fine. But maybe instead of using Shared Views for all the ACES-based views, we can use plain-ol regular views.

@carolalynn carolalynn added the configs for separation purposes label Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configs for separation purposes
Projects
None yet
Development

No branches or pull requests

8 participants