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

Support gamma from users' input in the demo app #181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DichenZhang1
Copy link
Collaborator

No description provided.

@@ -372,6 +374,7 @@ class UltraHdrAppInput {
size_t mMapDimensionScaleFactor;
int mMapCompressQuality;
bool mUseMultiChannelGainMap;
float mGamma;
Copy link
Contributor

Choose a reason for hiding this comment

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

const float mGainMapGamma;
this keeps in sync with other constructor arg declarations

@@ -137,7 +136,7 @@ class JpegR {
public:
JpegR();

JpegR(size_t, int, bool);
JpegR(size_t, int, bool, float);
Copy link
Contributor

Choose a reason for hiding this comment

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

all constructors can be merged with
JpegR(size_t mapDimensionScaleFactor = kMapDimensionScaleFactorDefault,
int mapCompressQuality = kMapCompressQualityDefault,
bool useMultiChannelGainMap = kUseMultiChannelGainMapDefault,
float gainMapGamma = kGainMapGammaDefault);

@@ -332,6 +331,12 @@ class JpegR {

bool isUsingMultiChannelGainMap() { return this->mUseMultiChannelGainMap; }

void setGainMapGamma(float gamma) {
Copy link
Contributor

Choose a reason for hiding this comment

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

are these getter setter functions required, as these are only controlled from constructor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think yes. User may change the configurations on the fly since we don't have a m_sailed-like flag at jpegr.cpp layer.

Besides, it's not convenient to set this parameters only from constructor. E.g. if user only wants to set gamma while use the default values for the other parameters, they'll need to also provide the other parameter values in the constructor.

snprintf(status.detail, sizeof status.detail, "received nullptr for uhdr codec instance");
return status;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

error checks for gamma range are required.
check required if context has moved to running state from init (m_sailed).

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.

None yet

2 participants