Review: RLA overhaul#216
Conversation
|
Fixed the bug. Tests pass now. For all of Jeremy's examples, I can "iconvert foo.rla foo-oiio.rla" and have "idiff foo.rla foo-oiio.rla" report identical images. |
|
One more fix -- although I can't find any documentation saying that this should be the case, our previous implementation just dumped float data without RLE compression at all, in order to match other RLA-using software at SPI. I honestly don't know if this is "correct", "incorrect", or "nobody else has ever used float RLA, so who cares." I'm erring on the side of preserving compatibility with what we had before. |
Add unit tests for bit_range_convert.
src/rla.imageio/rlainput.cpp
Outdated
There was a problem hiding this comment.
I get a compile warning here:
[ 70%] /net/homedirs/jeremys/git/oiio.js/src/rla.imageio/rlainput.cpp: In member function 'void OpenImageIO_Arnold::v0_11::RLAInput::preview(std::ostream&)':
/net/homedirs/jeremys/git/oiio.js/src/rla.imageio/rlainput.cpp:131: warning: dereferencing type-punned pointer will break strict-aliasing rules
/net/homedirs/jeremys/git/oiio.js/src/rla.imageio/rlainput.cpp:132: warning: dereferencing type-punned pointer will break strict-aliasing rules
Not sure if this warning matters or not.
There was a problem hiding this comment.
I'll fix the warning before I check in.
|
Looks good to me! (Feel free to commit either with or without the warning fix). |
2 participants
Requirements at work caused us to need more robust handling of non-8-bit RLA files. In the process, I did a lot of simplification, refactoring, and commenting.
I'm very confident in the reader now. Even handles things like 10-bit RLAs embedded in 16 bit containers.
I'm still testing the writer, I think there are a couple subtle bugs that I'm tracking down that leads to occasional LSB errors. I'll update when I have them fixed, but it's going to be very minor, so I think there's no reason not to start the review now for anyone who's interested.