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

Fix hardcoded color range in tonemapx LUTs #470

Closed
wants to merge 1 commit into from

Conversation

nyanmisaka
Copy link
Member

Changes

  • Fix hardcoded color range in tonemapx LUTs
  • Fix typo and clean up code to be consistent with GPGPU filters

Issues

  • Obviously different colors between sw tonemapx and GPGPU filters (in maxRGB)

@nyanmisaka nyanmisaka requested a review from a team October 1, 2024 22:56
@gnattu
Copy link
Member

gnattu commented Oct 1, 2024

I have a local version to make more changes to keep GPU/CPU implementations consistent to replace this because our GPU implementation side also has some inconsistencies abut range and peak handling that is different from the software one. But I noticed some design differences in yours so maybe we need to discuss further.

@nyanmisaka nyanmisaka marked this pull request as draft October 1, 2024 23:06
+
+ for (i = 0; i < 32768; i++) {
+ double v1 = (i - 2048.0f) / 28672.0f;
+ double v1 = (i - s->lut_off) / 32767.0f;
Copy link
Member

Choose a reason for hiding this comment

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

Why it is not divided by limited range 28672.0f when the input is in limited range now? In my implementation this is changed with the lut_off.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because doing so gives incorrect colors based on testing, too bright and loses too much detail.

off
fine

Copy link
Member

Choose a reason for hiding this comment

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

This is due to still incorrect range handling though, as the output rgb color after yuv2rgb is always in full range this limited range lut should not be used on those colors. I'm going to push my version soon.

@nyanmisaka nyanmisaka closed this Oct 5, 2024
@nyanmisaka nyanmisaka deleted the fix-hardcoded-luts branch October 5, 2024 10:58
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