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

exr: support 16-bit (half) float export #11001

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented Jan 26, 2022

Also switched to simpler scanline format instead of tiled and added more compression options (assumes >= OpenEXR 2.2, available since Ubuntu 18.04; msys2, macports/homebrew already ship newer).

Also fixes #7965

@TurboGit TurboGit added this to the 4.0 milestone Jan 27, 2022
@TurboGit TurboGit added the feature: enhancement current features to improve label Jan 27, 2022
@TurboGit TurboGit self-requested a review January 27, 2022 09:36
@JLTastet
Copy link

I just tested your PR on macOS Monterrey.

All new combinations of bit depth and compression export successfully and open fine in tev (although the colors are off because tev seems to assume sRGB primaries).

16-bit uncompressed (or with compression settings other than DWA) also opens fine in macOS Preview, and the file size is reduced by ~50% vs 32-bit in the uncompressed case, as expected. Unfortunately, DWA compression leads to files which cannot be opened in Preview. But since it works in tev, this is probably just a limitation of Preview.

DWAA/DWAB compression leads to significant savings of about 5x (16-bit) or 10x (32-bit). Funnily enough, the compressed size is almost exactly the same for 16/32-bit. Is this expected?

Overall, this looks good to me.

@kmilos
Copy link
Contributor Author

kmilos commented Jan 28, 2022

Thanks for testing and reporting the in-depth findings.

the colors are off because tev seems to assume sRGB primaries

Yeah, it might not read the primaries dt stores in the header... It's also not guaranteed dt stores them totally correctly, but that's a different topic/issue to be raised. There's also the question of what transfer curve this tev assumes in addiiton to the primaries? Does it think all EXRs are linear? You might want to set the dt output profile to a linear one as well.

DWAA/DWAB compression leads to significant savings of about 5x (16-bit) or 10x (32-bit). Funnily enough, the compressed size is almost exactly the same for 16/32-bit. Is this expected?

No idea ;) Not an expert on the EXR compression schemes. It is possible that DWA first does a lossy conversion to half and then does the compression, which would make them almost identical. The comment for PXR24 says e.g. the first step is lossy conversion to 24-bit float...

@johnny-bit
Copy link
Member

Hey @kmilos - i just saw this PR and immediatelly remembered a weird bug... Could you take a look at #7965 and whether this PR also fixes that? Or can you check & debug a bit?

@JLTastet
Copy link

Hey @kmilos - i just saw this PR and immediatelly remembered a weird bug... Could you take a look at #7965 and whether this PR also fixes that? Or can you check & debug a bit?

Indeed this PR seems to fix #7965 (or at least improve the situation).

Without it, enabling PIZ compression reduces the size of a 32-bit EXR file from 291.8 MB to 283.3 MB. With it, the same file is compressed down to 231.1 MB.

@johnny-bit
Copy link
Member

I hope it does. Labelling it as a bugfix! If you think it can be added - plz add in opening comment that it fixes the bug?

@johnny-bit johnny-bit added the bugfix pull request fixing a bug label Jan 28, 2022
@kmilos
Copy link
Contributor Author

kmilos commented Jan 28, 2022

As long as the API works, I would consider it a bug fix indeed.

As compression ratio goes, I have noticed that any of the lossless options work poorly on 32-bit floats, but rather well (~40%) on 16-bit half type...

Switching to scanlines might have also been a factor. The relatively small 100x100 tiles might have been inefficient.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@TurboGit TurboGit merged commit 7d02fef into darktable-org:master Feb 11, 2022
@TurboGit TurboGit added documentation-pending a documentation work is required release notes: pending labels Feb 11, 2022
@kmilos kmilos deleted the exr_half branch February 11, 2022 09:33
@elstoc elstoc removed the documentation-pending a documentation work is required label Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug feature: enhancement current features to improve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in the EXR export compression mode in cli
5 participants