AcademySoftwareFoundation / openfx

OpenFX effects API
Other
419 stars 122 forks source link

Misc fixes to colourspace extension #171

Closed john-paulsmith closed 1 month ago

john-paulsmith commented 1 month ago

Some more uses of the misnamed property values have been corrected, and the docs around output clip colourspace have been improved.

Guido-assim commented 1 month ago

The documentation for kOfxImageClipPropColourspace might still be somewhat confusing. Maybe it should be more clear that the host will query this property through kOfxImageEffectActionGetOutputColourspace and that the plugin may set it on the output clip for its own internal purposes but that the host will not check the property on the output clip.

barretpj commented 1 month ago

The documentation for kOfxImageClipPropColourspace might still be somewhat confusing. Maybe it should be more clear that the host will query this property through kOfxImageEffectActionGetOutputColourspace and that the plugin may set it on the output clip for its own internal purposes but that the host will not check the property on the output clip.

Just been discussing that with J-P and Gary. I'd say no to both of those. In my opinion the host should be the only thing that sets this property on a clip. The kOfxImageEffectActionGetOutputColourspace call returns the plugin's requested output colour space in the args, not as a property of the clip itself.

john-paulsmith commented 1 month ago

I am wondering if you would want kOfxColourspaceRoleAcesInterchangeIsBasic instead of kOfxColourspaceAcesInterchangeIsBasic?

Thanks @Guido-assim, I've fixed that now.

Guido-assim commented 1 month ago

@john-paulsmith I just noticed that in the file ofx-native-v1.5_aces-v1.3_ocio-v2.3.h the line: / @file ofx-native-v1.5_aces-v1.3_ocio-v2.3.h changed back to: / @file ofxColourspaceList.h This had been changed in: https://github.com/AcademySoftwareFoundation/openfx/commit/78a927ea2b2b65ad129e407f197f1a7a1e05a4dc Maybe this needs to be changed in the script that generates ofx-native-v1.5_aces-v1.3_ocio-v2.3.h.