OIIO Transcode: Simplify Extract OIIO transcode settings

[MustafaJafar]

Changelog Description

This PR aims to ease and simplify the settings of ExtractOIIOTranscode plugin.

List of TODOs

• Simplify the settings
• Tweak the code to work with the new settings
• Add Better Tool Tips
• Add setting conversion

Additional info

This PR was not requested by clients or community.
The motivation came from documentation when this PR started initially as renaming the input fields.
but it evolved from continues discussions and testing to improve the user experience of the settings.

Therefore, we no longer need to explain why the display and view settings are shown if the transcode type is colorspace.

This PR is not backward compatible as it changed the settings hierarchy a little bit.
But, conversion logic is implemented so we can copy old settings to the new settings without an issue.

For reference, I was testing BigRoy's PR Collect required data for local and farm render jobs to have a 'colorspace' value specified #75

Demo

Testing notes:

1. Add your settings to the plugin.
2. Verify it works as usual

[MustafaJafar]

Here's a snippet from a run test on my side.

DEBUG: Looking for matching profile for: hosts: "houdini" | product_types: "render" | product_names: "render_beauty" | task_names: "cfx" | task_types: "CFX"
DEBUG: Profile selected: {'product_types': ['render'], 'hosts': ['houdini'], 'task_types': [], 'task_names': [], 'product_names': [], 'delete_original': True, 'outputs': [{'name': 'render', 'extension': 'jpeg', 'transcoding_type': 'use_colorspace', 'use_colorspace': {'colorspace': 'Output - sRGB'}, 'use_display_view': {'display': '', 'view': ''}, 'oiiotool_args': {'additional_command_args': []}, 'tags': [], 'custom_tags': []}]}

[iLLiCiTiT]

NOTE: If it's about changing labels and adding conditional enum, then please change just labels and add conditional enums without changing attribute names, otherwise we would have to implement conversion of settings (PITA).

[MustafaJafar]

NOTE: If it's about changing labels and adding conditional enum, then please change just labels and add conditional enums without changing attribute names, otherwise we would have to implement conversion of settings (PITA).

Can I add conditional enum without creating a new settings class ?
Specifically, I mean display and view settings.

[jakubjezek001]

NOTE: If it's about changing labels and adding conditional enum, then please change just labels and add conditional enums without changing attribute names, otherwise we would have to implement conversion of settings (PITA).

I agree that it might be PITA but this could be implemented with conversion.py settings overrides (example here ayon-core\server\settings\conversion.py).

[jakubjezek001]

Consider making the colorspace section toggleable. For more details, check out the ColorspaceConfigurationModel in our codebase nuke-addon-server-settings-common. This approach would better suit situations where we need to clearly indicate that adding a value to a field will override an upstream value. By using a toggle, we can make it clear to the user that they are about to override an existing colorspaceData.

[BigRoy]

This approach would better suit situations where we need to clearly indicate that adding a value to a field will override an upstream value. By using a toggle, we can make it clear to the user that they are about to override an existing colorspaceData.

I'm not sure what you mean. It's not overriding the colorspace 'metadata' of the image - it's literally transcoding it from the source colorspace to the destination colorspace? Yes, it's hence also marking it as the destination colorspace? (But is there really a case where it shouldn't if it's transcoded?)

I may be missing your point.

[jakubjezek001]

(But is there really a case where it shouldn't if it's transcoded?)

Yes, since you can use addinional oiiotool args, you could also use preset for resizing or other reposition methods.

[BigRoy]

(But is there really a case where it shouldn't if it's transcoded?)

Yes, since you can use addinional oiiotool args, you could also use preset for resizing or other reposition methods.

Ah yes, we have that actually.

But still - then the logic that specificying no colorspace value means "do not color convert" is totally sensible and when specifying a colorspace value will transcode it to that and mark that as the resulting file's colorspace.

[jakubjezek001]

But still - then the logic that specificying no colorspace value means "do not color convert" is totally sensible and when specifying a colorspace value will transcode it to that and mark that as the resulting file's colorspace.

If I was a user, I would rather prefer the explicit way, but this is just a suggestion. You can of course keep it implicit and add it into documentation.

[BigRoy]

If I was a user, I would rather prefer the explicit way, but this is just a suggestion. You can of course keep it implicit and add it into documentation.

Explicit as in - it's a separate option in the enum?
With "user" you mean the "admin" maintaining the settings?

Like:

1. no color conversion
2. convert to target colorspace
   • colorspace: str (required target colorspace; must be a valid value)
3. apply display/view from host
4. apply display/view transform
   • display: str (required target display; must be a valid value)
   • view: str (required target view; must be a valid value)

[MustafaJafar]

There should be implemented conversion of overrides for new settings structure. Let me know if you don't know how to do it.

I'd appreciate your help. I don't know indeed how to implement the conversion.

[MustafaJafar]

@iLLiCiTiT I managed to add the settings conversion.

2024-09-20.00-27-47.mp4

[MustafaJafar]

@iLLiCiTiT @jakubjezek001 I think this PR is ready.

[iLLiCiTiT]

LGTM

[robin-ynput]

I gave it a try with previous version and adjusted UI settings, and I can confirm it works perfectly. The new UI makes way more sense indeed, so thanks for that 💪 .

As mentioned, the only caveat I found is that I had to re-create my Extract OIIO Transcodesettings when I uploaded the new addons.

Before:

After:

That's probably out of the scope of this review, but I think this really needs to be communicated properly on roll-out (new major ? non backward compatible flag ?).

[MustafaJafar]

That's probably out of the scope of this review, but I think this really needs to be communicated properly on roll-out (new major ? non backward compatible flag ?).

Thank you for pointing that out.
I gave it another test on my side and currently it copies the exact list + doing settings conversion.

in this video some tests (sorry no voice) where I copy from older version to the current version and they all have the same result.
However, it won't work when copying settings from the current to the older version.

2024-10-02.10-37-18.mp4

[MustafaJafar]

Let's merge this PR and if there any issue, we can fix them later.