Skip to content

Conversation

@barretpj
Copy link
Contributor

Example now includes 3 plugins, to exercise Basic, Core and Full styles.

@revisionfx
Copy link
Contributor

Thanks

Is there a difference between?:
ROLE(kOfxColourspaceRoleColorTiming, "Color Timing role (Arbitrary scene log)"),
ROLE(kOfxColourspaceRoleCompositingLog, "Compositing Log role (Arbitrary scene log)"),
Is there a difference between?:
ROLE(kOfxColourspaceRoleMattePaint, "Matte Paint role"),
ROLE(kOfxColourspaceRoleTexturePaint, "Texture Paint role (Typically sRGB)"),

@barretpj
Copy link
Contributor Author

Is there a difference between?: ROLE(kOfxColourspaceRoleColorTiming, "Color Timing role (Arbitrary scene log)"), ROLE(kOfxColourspaceRoleCompositingLog, "Compositing Log role (Arbitrary scene log)"), Is there a difference between?: ROLE(kOfxColourspaceRoleMattePaint, "Matte Paint role"), ROLE(kOfxColourspaceRoleTexturePaint, "Texture Paint role (Typically sRGB)"),

Not relevant to this PR. These are the spaces/roles define in the OCIO config we use.

@Guido-assim
Copy link
Contributor

These plugins are very nice to check the behavior of the host :-).
Maybe a hypothetical case, but if the plugin specifies a basic color space as an output color space then the host doesn't know what the exact color space is that is used. The host might need to convert the output of the plugin to another color space for display or other reasons.

@john-paulsmith
Copy link
Contributor

Maybe a hypothetical case, but if the plugin specifies a basic color space as an output color space then the host doesn't know what the exact color space is that is used. The host might need to convert the output of the plugin to another color space for display or other reasons.

Not specifically relevant to this PR, but my recommendations would be:

  1. Plug-ins should use cross-referencing for the output clip unless absolutely necessary.
  2. When using the basic style, plug-ins should not perform colourspace transformations to anything other than ofx_raw (e.g. motion vectors).
  3. If the plug-in does set a basic colourspace on the output clip, the host should interpret that in the same way it would if it was supplying images to the plug-in using that basic colourspace.

@garyo
Copy link
Contributor

garyo commented Feb 25, 2025

Good, CI is passing again now. To fix the DCO check problem, see here https://github.com/AcademySoftwareFoundation/openfx/pull/178/checks and make sure to always sign commits with -s .

To make sure I never forget, I do this:

git config --global commit.template ~/.gitmessage

And ensure Signed-off-by: Your Name <[email protected]> is included in that template (I put it after two blank lines).

barretpj and others added 2 commits February 27, 2025 14:52
@barretpj barretpj force-pushed the colourspace-example-updates branch from a3745a7 to f31404b Compare February 27, 2025 14:57
@barretpj
Copy link
Contributor Author

OK that eventually worked. I really struggle to get my head around this workflow.

@garyo
Copy link
Contributor

garyo commented Feb 27, 2025

Thanks for sticking with it, Phil! The signoff is just to make sure we are legally protected; sorry it's a pain.

@garyo
Copy link
Contributor

garyo commented Feb 27, 2025

@john-paulsmith do you approve this PR?

Copy link
Contributor

@Guido-assim Guido-assim left a comment

Choose a reason for hiding this comment

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

I am not sure why the comment about kOfxImageClipPropPreferredColourspaces in getOutputColourspace is dropped? The use of kOfxImageClipPropPreferredColourspaces in kOfxImageEffectActionGetOutputColourspace is described in the ofxColour.h file.

@barretpj
Copy link
Contributor Author

I am not sure why the comment about kOfxImageClipPropPreferredColourspaces in getOutputColourspace is dropped? The use of kOfxImageClipPropPreferredColourspaces in kOfxImageEffectActionGetOutputColourspace is described in the ofxColour.h file.

Thanks, fixed.

Copy link
Contributor

@Guido-assim Guido-assim left a comment

Choose a reason for hiding this comment

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

Looks good to me. Very useful to test the behavior of a host.

@barretpj barretpj merged commit a85b1c5 into AcademySoftwareFoundation:main Mar 3, 2025
17 checks passed
@barretpj barretpj deleted the colourspace-example-updates branch March 3, 2025 19:49
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.

5 participants