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

Allow for larger excursions of min, max content boost #215

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

ram-mohan
Copy link
Contributor

Current implementation clips min/max content boost to a smaller range. Allow for larger excursions for better hdr intent representation.

Also, fixed round factor before encoding gainmap coefficient

Test: ./ultrahdr_unit_test

@DichenZhang1
Copy link
Collaborator

We may not be able to merge this since this may introduce other concerns (e.g. performance drag, introducing error if the input is compressed due to compression artifacts). Besides, existing components e.g. Skia is using the old logic, and moving to this logic may lead to changes outside this library. We can hold this for now.

@DichenZhang1
Copy link
Collaborator

DichenZhang1 commented Aug 9, 2024

@ram-mohan @harishdm we will take this change, and some modifications need to be done.

We can introduce an encode mode parameter, "SPEED" mode and "QUALITY" mode, the former to be default. Under "QUALITY" mode we do this. This can happen in API-1, 2, 3 (no API-0 because our internal tone mapping already considered this)

I suggest in this change we start with a macro definition of the encode mode parameter, and making it configurable through the command line app in a follow-up change.

Thank you for your help!

@DichenZhang1
Copy link
Collaborator

Besides, we may also try out #236, which has similar effects for API-1 in my local tests but it's much easier.

@ram-mohan
Copy link
Contributor Author

@ram-mohan @harishdm we will take this change, and some modifications need to be done.

We can introduce an encode mode parameter, "SPEED" mode and "QUALITY" mode, the former to be default. Under "QUALITY" mode we do this. This can happen in API-1, 2, 3 (no API-0 because our internal tone mapping already considered this)

I suggest in this change we start with a macro definition of the encode mode parameter, and making it configurable through the command line app in a follow-up change.

Thank you for your help!

Dichen, I have made the necessary changes and pushed a patchset.

@ram-mohan
Copy link
Contributor Author

Besides, we may also try out #236, which has similar effects for API-1 in my local tests but it's much easier.

This should improve quality but solution is not universal. Even though the lower limit is extended from 0 to -6.64, to some extent it is clamped and the upper limit is still clamped. Range of content boost is indicative of dynamic range of recovery of hdr intent from sdr intent. Clamping to a predefined value can work in most cases, but there can be scenarios where this can cause limitations.

@DichenZhang1
Copy link
Collaborator

Thank you for the latest change. My point is if #236 can give us similar quality improvement as this one, we can consider taking #236. Or we may take both.

Current implementation clips min/max content boost to a smaller range.
Allow for larger excursions for better hdr intent representation.

This is not enabled by default and is enabled in best quality encoding
preset.

Test: ./ultrahdr_app <options> -D 1
Test: ./ultrahdr_unit_test
@DichenZhang1 DichenZhang1 merged commit 04788ce into google:main Aug 19, 2024
1 check passed
@ram-mohan ram-mohan deleted the gain branch August 19, 2024 19:24
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