Add support for 16- and 32-bit Photoshop file reads

[EmilDohne]

Description

As mentioned in #4205, #3900 and #3910 (reply in thread) OpenImageIO previously did not support reading of 16- and 32-bit files beyond their merged image data section. I have now implemented this by:

1. Adding support for reading layer info from the "Lr16" and "Lr32" Tagged Blocks as well as adding additional checks to the regular layer info section
2. Implemented the Zip and ZipPrediction compression codec which is required to read these files as that is photoshops preferred compression codec for those bitdepths

Due to the way the Zip data is stored (per channel but not per scanline) the current implementation reads the whole channel into a buffer which is then memcpy'd from by calls to read_native_scanline(). The buffer gets cleared on deletion of the PSDInput object

Checklist:

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite
  (adding new test cases if necessary).
• If I added or modified a C++ API call, I have also amended the
  corresponding Python bindings (and if altering ImageBufAlgo functions, also
  exposed the new functionality as oiiotool options).
• My code follows the prevailing code style of this project. If I haven't
  already run clang-format before submitting, I definitely will look at the CI
  test that runs clang-format and fix anything that it highlights as being
  nonconforming.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: EmilDohne / name: EmilDohne (4744f3f, 91770b7, 3f73dbb, 23493fc, 7d47e2d, 6a5d11a, e8d9d67, 09db12f, 8698155, 3f6daf9, 0cc0fed, cc36f2e, 2fb616d, 439cea3, 522379b, 816bef7, 03df1ff, a22a7a7)
• ✅ login: lgritz / name: Larry Gritz (1ada685)

[EmilDohne]

In regards to tests, I can confirm all of them do pass but im a bit unsure whether psd even gets run in the test cases (from the tests I found). The closest one would be imageinout_test but since there is no output for PSD it doesnt get run from what I gather?

Would love to get some more insight on this to see if adding alternative test cases is necessary

[lgritz]

I'm really happy to see somebody tackling this, thanks so much for taking on the task.

In CI, we're seeing build failures. They look simple, probably things that may have been only warnings for you, but we make all warnings into failures for CI. Just click on the "details" links in the failed CI tests and you'll see the errors, they should be easy to fix.

Apologies for the awkward CI delays -- it's GitHub policy that a first-time contributor to a repo does not get their submission run in CI until an admin of the project hits a button to authorize it. Once you get a PR accepted into the projects, subsequent PRs you submit will run CI automatically when you push, without needing to wait for me or another admin to authorize it individually.

Anyway, as for the testing, we do have PSD tests in the testsuite. Admittedly, it is pretty basic testing for PSD, and could stand improvement.

It would be very helpful if your PR could include tests for the new functionality, which would consist of the following:

• Adding small PSD files that use the newly supported modes to the src/ subdirectory in that test.
• Or, if certain tests are only possible with files so large that we don't want them to be part of every OpenImageIO checkout, they could perhaps be added to the sister project OpenImageIO-images, where you'll see a number of other psd files used for these tests.
• Modify the run.py file to actually do something that exercises the features you want to test. For example, given that we don't write PSD files, a test might be to read a layer from one of your sample PSD files, write it as a TIFF, and then compare that TIFF file to a "reference" copy (that you've inspected to convince yourself is correct) stored in the ref/ subdirectory.
• Modify the ref/out.txt file to reflect any changed console output that you expect from the added tests.

There are lots of directories in testsuite/ to inspect to see examples of how this is done, and also I'm happy to help out and make suggestions.

[EmilDohne]

Hi @lgritz, im very happy to be able to put my rather obscure knowledge of the photoshop file format to use somewhere. I'll have a look at those error messages but as you said they do seem rather trivial. While the tests do have 16- and 32- bit files I will go ahead and expand on those with some more modern files (as the ones in the test suite are from Photoshop CS5 from what I can tell).

I'll likely be updating this pull request a couple of times until the errors are all gone for the code and test cases and I will let you know once I am ready for review again :)

[EmilDohne]

The CI build errors seem to have been fixed and I also addressed some test regressions due to an incorrect assumption I made. This also gave me some further insight into why, even though there are 16- and 32-bit test files, these were not picking up any errors.

This is due to them having a single locked layer which photoshop will store in the merged image data section to save space and it will never go to the Lr16 or Lr32 tagged block.

These test cases should address that and ive updated the out.txt to reflect these new test cases. I will check if any other errors get raised but I believe this should now work

[EmilDohne]

Apologies for the back and forth, I forgot to add the files to run.py but was also able to remove some more clang-tidy errors :)

[EmilDohne]

@lgritz I saw there was some mismatched sha-1's for the 16 and 32-bit files which is to be expected and ive corrected that but im unsure about the errors raises by psd-colormodes as from what I gather its only checking if the files' diff is as expected. However im not versed enough with the way the tests are run to confirm why this could be happening. Do you maybe have some more insight on this?

Heres an excerpt of the errors

Comparing "pattern2-8-rgb.psd.tif" and "ref/out.txt"
idiff ERROR: Could not read ref/out.txt:
	Invalid image file "ref/out.txt": OpenImageIO could not find a format reader for "ref/out.txt". Is it a file format that OpenImageIO doesn't know about?
Comparing "pattern2-8-rgb.psd.tif" and "ref/pattern2-cmyk.tif"
200 x 150, 3 channels
  Mean error = 0.0429666
  RMS error = 0.0829628
  Peak SNR = 21.6223
  Max error  = 0.337255 @ (53, 51, B)  values are 0.5019608, 0, 0.5019608 vs 0.3803922, 0, 0.83921576
  16406 pixels (54.7%) over 0.032
  18432 pixels (61.4%) over 0.016
FAILURE
Comparing "pattern2-8-rgb.psd.tif" and "ref/pattern2.tif"
PASS
Comparing "pattern2-16-rgb.psd.tif" and "ref/out.txt"
idiff ERROR: Could not read ref/out.txt:
	Invalid image file "ref/out.txt": OpenImageIO could not find a format reader for "ref/out.txt". Is it a file format that OpenImageIO doesn't know about?
Comparing "pattern2-16-rgb.psd.tif" and "ref/pattern2-cmyk.tif"
200 x 150, 3 channels
  Mean error = 0.0430666
  RMS error = 0.083697
  Peak SNR = 21.5458
  Max error  = 0.341161 @ (53, 51, B)  values are 0.49805447, 0, 0.49805447 vs 0.3803922, 0, 0.83921576
  16406 pixels (54.7%) over 0.032
  18432 pixels (61.4%) over 0.016
FAILURE
Comparing "pattern2-16-rgb.psd.tif" and "ref/pattern2.tif"
PASS
Comparing "pattern2-8-cmyk.psd.tif" and "ref/out.txt"
idiff ERROR: Could not read ref/out.txt:
	Invalid image file "ref/out.txt": OpenImageIO could not find a format reader for "ref/out.txt". Is it a file format that OpenImageIO doesn't know about?
Comparing "pattern2-8-cmyk.psd.tif" and "ref/pattern2-cmyk.tif"
PASS

[lgritz]

As an aside, I think you are typing "clang-tidy" when you mean "clang-format".

[lgritz]

This might not be the best way to organize the PSD tests.

Sometimes the test is (like this) "iinfo all the things" and we know that the hash will ensure that we're reading it just as we always had. This is an approach we make sometimes and it works well when we are VERY sure that things are behaving correctly and will never change, and also it can be attractive because it's a much smaller amount of saved reference material than an image.

But when things do change the hash, it's hard to diagnose why, or if it's still "correct".

An alternate approach is to read from the PSD, and write the file back. Since we don't write PSD, it could be another file type, such as TIFF. Then actually do an image diff of the output image against a saved reference image. Can be a bigger thing to store, but it has the advantage of being easy to visually inspect.

If you want to just wait a short time, I can try to locally download what you have and see if I can quickly rearrange the test to show you what I mean, and then push it back to your tree to complete the PR?

[EmilDohne]

@lgritz Thank you for the update and the tips, I do think that is likely a better way of organizing the tests and I would very much welcome your improvement!

[lgritz]

OK, I will take a quick stab at it and share what I have.

[lgritz]

Pro tip for next time: it can get really messy when you make patches from your fork's "master". It's usually easier to make a new branch name and submit a PR from there, against our master.

[lgritz]

I pushed an update for the psd-colormodes test. What was happening there is that we didn't have reference images for 16 bits -- it was reading 16 bit inputs, but since it wasn't getting the full 16 bits, it was previously successfully matching against 8 bit references! Now that it's really 16 bits, it no longer matched the 8 bit reference output. So I added 16 bit reference output.

[lgritz]

Let's see what happens now in the latest CI run, I can also touch up testsuite/psd if I need to.

[lgritz]

I think the remaining failures are either spurious or unrelated to your changes.

I believe we're ready to merge this, if it's okay with you, @EmilDohne

[lgritz]

LGTM, thanks for this contribution to the project!

[EmilDohne]

Pro tip for next time: it can get really messy when you make patches from your fork's "master". It's usually easier to make a new branch name and submit a PR from there, against our master.

Thanks, I'll definitely keep that in mind and open an appropriate branch next time around!

I think the remaining failures are either spurious or unrelated to your changes.

I believe we're ready to merge this, if it's okay with you, @EmilDohne

Feel free to merge it, thanks for taking a look and figuring it out so quickly. Glad I was able to contribute :)