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

LUT mapping #100

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

LUT mapping #100

wants to merge 2 commits into from

Conversation

sundermann
Copy link
Contributor

This PR aims at solving the color accuracy issues seen with the LG capture APIs. Essentially, this is like HyperHDR's HDR tone mapping feature but integrated into hyperion-webos itself. The data format is also the same and as such lut tables from HyperHDR can be used as is. However, opposed to HyperHDR the tone mapping is always active because we are seeing color inaccuraries even on SDR content.

I am providing my LUT file that I calculated using machine learning by showing pregenerated color table images and capturing them. Then the network learned given an input capture color to predict the input color. The predictions were carried out for the full 8-bit color space and saved into a LUT file. The generated LUT is not perfect for many reasons possibly the most prominent one being that multiple input colors match the same captured color. Yet still, without LUT mapping colors are quite off. For instance, solid red captures as [228, 80, 52], green as [110, 255, 100] and blue as [44, 0, 184] which is quite far off the real value.

I am marking this as WIP because I haven't tested configuration reloading.

LUT table

Copy link
Member

@Informatic Informatic left a comment

Choose a reason for hiding this comment

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

Just some small nitpicks

src/unicapture.c Outdated Show resolved Hide resolved
src/unicapture.c Outdated Show resolved Hide resolved
@sundermann sundermann force-pushed the lut-mapping branch 4 times, most recently from 07d5108 to 51cca3c Compare February 24, 2023 19:11
src/main.c Outdated Show resolved Hide resolved
@Informatic Informatic marked this pull request as ready for review March 11, 2023 11:44
@TBSniller
Copy link
Collaborator

@sundermann: Do you mind to change the config line to @Informatic's latest start/stop improvement in #105?

Also wanted to ask if you think it's possible to add lut tables to this git repository. Maybe in root under a new "luts" folder. In this case we could easily ship them in PicCap.

@tuxuser
Copy link
Member

tuxuser commented Mar 11, 2023

Would it break the LUT functionality in HyperHDR, as it also ships a LUT ?
Edit: Nevermind, only one LUT correction should be enabled at given time.

Two options:

  1. Make sure users are aware that only one method should be used, not both.
  2. Remove the LUT stuff from HyperHDR entirely?! e.g. Remove LUT table itself + Remove the HTTP calls into HyperHDR RPC API to toggle content mode (SDR/HDR)

@sundermann
Copy link
Contributor Author

@sundermann: Do you mind to change the config line to @Informatic's latest start/stop improvement in #105?

Rebase?

Also wanted to ask if you think it's possible to add lut tables to this git repository. Maybe in root under a new "luts" folder. In this case we could easily ship them in PicCap.

One LUT table is roughly 50mb. I don't think we want to ship different LUT tables by default. Also right now I have only calculated a decent LUT table for o22.

Edit: Nevermind, only one LUT correction should be enabled at given time.

No, multiple LUTs can work together. Consider the hyperion-webos more like a LUT that corrects for the device specific tone mapping errors. After those have been corrected HyperHDR can apply its own LUT table for HDR content.

@tuxuser
Copy link
Member

tuxuser commented Mar 12, 2023

Rebase?

Rebased

One LUT table is roughly 50mb. I don't think we want to ship different LUT tables by default. Also right now I have only calculated a decent LUT table for o22.

In that case, just have the functionality ready for the advanced users that are able to create their own LUT, for their respective model, and edit the config.json by hand.
Do not ship anything additional within piccap.

No, multiple LUTs can work together. Consider the hyperion-webos more like a LUT that corrects for the device specific tone mapping errors. After those have been corrected HyperHDR can apply its own LUT table for HDR content.

Thanks for clearing that up.

@sundermann
Copy link
Contributor Author

Do we need anything else here to merge this?

@Dak0r
Copy link

Dak0r commented Oct 30, 2023

Hi @sundermann, the LUT seems to be offline, would it be possible to re-upload it? I'd love to give this branch a try 👍

Do I understand correctly, that this entirely removes the need for using a LUT with SDR content, but a LUT will still be needed in HyperHDR to map HDR to SDR?

I built a version of hyperion-webos that uses HyperHDRs Json api to allow using different LUTs for sdr and hdr content, and I'm wondering if it's worth to create a PR, if this one already removes the need for it :)

Edit: Something worth noting is that @TBSniller noticed that the captured images are dependent on the chosen image mode on the TV. I suppose as long the creators & the users settings are not completely different, the results with a general LUT such as yours, should still be better than without one at all

Edit2: PR for the approach I mentioned above: #120

@jclsn
Copy link

jclsn commented Feb 11, 2024

@sundermann Can you please reupload the LUT you generated? The links is dead and I would like to give it a try

Ah wait, I was assuming the LUT was generated for Piccap, which is probably not correct.

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.

6 participants