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

Reminder: Simplify HDR Decoder #1480

Closed
johannesvollmer opened this issue May 22, 2021 · 5 comments
Closed

Reminder: Simplify HDR Decoder #1480

johannesvollmer opened this issue May 22, 2021 · 5 comments

Comments

@johannesvollmer
Copy link
Contributor

F32 color support was further improved, allowing us to hopefully simplify the HDR decoder.

See #1478 and maybe #1473

Draft

The HDR decoder currently uses an adapter to simulate f32 storage. This issue is a reminder to consider removing the adapter in favor of a simple RGB32F buffer.

@johannesvollmer johannesvollmer changed the title Simplify HDR Decoder Reminder: Simplify HDR Decoder Jun 3, 2021
@johannesvollmer
Copy link
Contributor Author

johannesvollmer commented Oct 21, 2021

There seems to be a very minimal HDR encoder, which is not used anywhere yet. There seems to be no comment about whether it could work. Maybe the only barrier preventing it from being used was the missing f32 support back then?

Now that f32 support is here, is it a matter of enabling the encoder, or will it have to be actually implemented?

@johannesvollmer
Copy link
Contributor Author

The approach for the decoder would be simply to change the default color type to Rgb32F instead of Rgb8 simply to maintain the accuracy. What do you think?

@emble
Copy link

emble commented Apr 8, 2022

Yes, please, do that. All my votes for that! I just stumbled about that issue.

My project is hdr/exr import, image = "0.24.1"
Invoking Reader::decode()
Result:

  • the brand-new OpenExr decoder works great (many thanks to all involved)
  • the Hdr decoder, as it currently is, renders itself unusable, on the first impression. One could call it broken. It should return f32, not u8.

Workaround for me was to invoke the Hdr decoder directly and call decoder.read_image_hdr(), while passing the other formats to decode()

Again, please remove the bad wrapper code that breaks the good code.

@johannesvollmer
Copy link
Contributor Author

johannesvollmer commented Apr 9, 2022

I've opened #1599 for this, but got stuck

@wrightwriter
Copy link

wrightwriter commented Nov 14, 2022

Btw, the solution @emble mentioned would look like this (if anyone's searching for it):

        let mut r = BufReader::new(File::open(file_name).unwrap());
        let decoder = image::codecs::hdr::HdrDecoder::new(r).unwrap();
        let pixels = decoder.read_image_hdr().unwrap();

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

No branches or pull requests

4 participants