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

Update the ColourSpace example #170

Conversation

Guido-assim
Copy link
Contributor

Do not set kOfxImageClipPropPreferredColourspaces for the output clip. Handle kOfxImageEffectActionGetOutputColourspace to set the output colourspace.

Copy link
Contributor

@garyo garyo left a comment

Choose a reason for hiding this comment

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

Tested build on Mac; builds OK. I don't have a colourspace-ready host to test against though.
The changes (adding getOutputColourspace and moving the output colourspace selection there) look fine to me at a quick glance, but @john-paulsmith will have to have the final word.

@garyo
Copy link
Contributor

garyo commented Sep 25, 2024

One small but important thing: please add a "DCO" line to your commit message. I suggest doing it by amending the latest commit and force-pushing, like this:

# make sure you're at commit ca59362 before starting this, with no changes, and then:
git commit --amend
# in your editor, add a blank line, and then "Signed-off-by: Guido Veldkamp <[email protected]>", then save.
git push -f # force-push your branch; this will update the PR automatically

In future, just use git commit -s instead of git commit to automatically add the "Signed-off-by" line.

Do not set kOfxImageClipPropPreferredColourspaces for the output clip.
Handle kOfxImageEffectActionGetOutputColourspace to set the output colourspace.

Signed-off-by: Guido Veldkamp <[email protected]>
@Guido-assim Guido-assim force-pushed the ColouSpace-Example-update branch from ca59362 to 4bdf491 Compare September 26, 2024 08:29
@john-paulsmith
Copy link
Contributor

Thanks for improving the example @Guido-assim, I'm going to merge it. This example doesn't do anything with the host-provided preferred output colourspace list, it would be good to include that in a future example.

@john-paulsmith john-paulsmith merged commit a8f10e1 into AcademySoftwareFoundation:main Oct 1, 2024
9 checks passed
@Guido-assim Guido-assim deleted the ColouSpace-Example-update branch October 3, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants