Skip to content

Add getter functions for settings to build ColorGrading object #8699

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

Merged
merged 5 commits into from
May 8, 2025

Conversation

dkasuga
Copy link
Contributor

@dkasuga dkasuga commented May 5, 2025

Currently, ColorGrading is created using a builder API, but there's no way to retrieve the settings that were used to build it after the fact. As the most simple and robust solution, this change make the ColorGrading object storing its creation parameters internally and providing their getter functions.

This change also involves changing changing the interface of the existing ToneMapper setter of the builder so that it takes tone mapper as shared_ptr instead of the raw pointer. For JNI function impl, to avoid the risk of the double free between C++ codes and JVM GC, the ToneMapper instance is just copied and passed.

FIXES=[364901658]

Copy link
Contributor

@poweifeng poweifeng left a comment

Choose a reason for hiding this comment

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

minor comments

@@ -51,6 +51,8 @@ class FColorGrading : public ColorGrading {
uint32_t mDimension;
bool mIsOneDimensional;
bool mIsLDR;

void initializeSettings(Settings& settings, const Builder& builder) noexcept;
Copy link
Contributor

Choose a reason for hiding this comment

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

static method?

nit: can you make settings the second param? Typically the "output" is at the end of the list of parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion, and I now incorporated that to my change.

@dkasuga dkasuga force-pushed the dk/colorgrading-properties-getter branch from 2971fd3 to 1fef472 Compare May 8, 2025 14:53
@dkasuga dkasuga merged commit f10d226 into main May 8, 2025
16 checks passed
@dkasuga dkasuga deleted the dk/colorgrading-properties-getter branch May 8, 2025 15:43
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.

2 participants