Closed LoopinFool closed 5 years ago
Closing/reopening to include b6b4eb0 in the Travis CI.
Code looks promising, worth including for CI testing against our library of images.
Note that QA 27761 could assist testing.
After inclusion into our daily builds, this PR did not cause any apparent regression against any sample dataset in our curated QA repository. In the absence of failures, merging for integration into a patch release of ome-codecs
.
While investigating the Huffman table problem I noticed a couple of things wrong and missing. The code read in the "pointTransform" value into a variable but never used it. Based on the standard and other code as reference, I added code to properly handle the case where the point transform is non-zero. That feature may almost never be used, but now the codec is able to handle such files.
I also noticed that the existing code used "/ 2" for a couple of the predictor types. I saw that the standard explicitly states that an arithmetic right shift of 1 bit should be used for the differential types since shifting signed values can produce different results than dividing by 2. Specifically, -1 ==> -1 rather than -1 ==> 0. Though it doesn't matter, I also changed the unsigned case to a shift for consistency.
I don't have any test files to fully test either change but did verify that they don't break decoding of my Huffman test images. That makes sense, since pointTransform is zero and they don't use any of those three predictor types. Our internal version of the library now has these changes. I leave it up to you whether you want to incorporate these changes. They should provide a wider range of image file compatibility, but I don't know if any such images actually exist. At least one other library (thorfdbg/libjpeg) has code similar to this - I was using it as a reference after I noticed these mistakes.