raw: fixes and improvements to thumbnail functionality

[antond-weta]

Description

Fixes and improvements to the thumbnail functionality to the raw image reader.

Adds optional hints raw:thumbnail_index and raw:thumbnail_sort to make it easier to select a specific thumbnail.

Tests

Yes, some basic unit-tests for every raw image available in oiio-images.

Checklist:

• I have read the guidelines on contributions and code review procedures.
• I have read the Policy on AI Coding Assistants
  and if I used AI coding assistants, I have an Assisted-by: TOOL / MODEL
  line in the pull request description above.
• I have updated the documentation if my PR adds features or changes
  behavior.
• I am sure that this PR's changes are tested in the testsuite.
• I have run and passed the testsuite in CI before submitting the
  PR, by pushing the changes to my fork and seeing that the automated CI
  passed there. (Exceptions: If most tests pass and you can't figure out why
  the remaining ones fail, it's ok to submit the PR and ask for help. Or if
  any failures seem entirely unrelated to your change; sometimes things break
  on the GitHub runners.)
• My code follows the prevailing code style of this project and I
  fixed any problems reported by the clang-format CI test.
• If I added or modified a public C++ API call, I have also amended the
  corresponding Python bindings. If altering ImageBufAlgo functions, I also
  exposed the new functionality as oiiotool options.

[lgritz]

The "hobbled" test failure is exactly what was fixed by #5339, not related to your code.

[lgritz]

I'm just throwing out ideas here for consideration/discussion. Not ready to be insistent on anything yet.

I'm wondering if these should get elevated out of the "raw" namespace so that readers of other formats that support multiple thumbnails (are there any?) can also honor them?

I also wonder if thumbnail_sort should be 3-valued: 0 means leave it in file order, -1 sorts small-to-large, 1 sorts large-to-small. (Or the other way around? Which is intuitive?) Which should be the default behavior?

It may be interesting to compare to (and maybe harmonize with?) MIP-maps. We generally expect MIPmaps to start with the full-rez image and go in successively lower rez. Should thumbnails be the same? Should negative thumbnail_index mean "count back from the end" so that index=-1 means "lowest res thumbnail"? (Is that just a bit too clever?)

I sort of regret the current API of get_thumbnail, which assumes that there is a single one. If there are multiple, I can see two kinds of use cases: (1) wanting to be able to select by index (if you want the highest, lowest, or need to iterate through all of them); (2) having a target resolution you're shooting for, and wanting to retrieve the one that is closest to that.

I think that changing the call signature of get_thumbnail probably needs to wait for OIIO 3.0, and we'll have to rely on attribute hints like you've done here, until then. But I believe that if we're going to add these hints, we should think really carefully about what the ideal interface is.

[antond-weta]

Can do! I would need to rely on your guidance here, I have never worked with MIP-maps.

[lgritz]

I'm not sure what to do here, I was just throwing out suggestions. The MIP-map angle is really just speculation.

Let's start from the basics:

• If there are multiple thumbnails, how should they be presented via the API? High to low? Low to high? Or whatever random order they appear in the file? Should there be an option to control the order, or is there one canonical way that all users will want, and we should just impose that and not need a control?
• What is the right (i.e. most useful) way for users to specify which of several thumbnails they want? Will they want a numerical index? Will they want to say (somehow) "biggest"/"smallest"? Will they want to supply a size and have it return the thumbnail that is closest?
• Am I overthinking this?

As for the MIP-maps, I was just noting the similarity between thumbnails of different sizes, and how MIP-maps are also about a file that contains several versions of an image, at successively reduced resolution. That's similar enough to wonder if they should be the same mechanism (like, should we never have made "thumbnails" and just made them look like MIPmaps?). On the other hand, there are many places where we make the assumption that MIPmap levels all have the same data type, channels, and color space, and that is NOT the case with raw files and their thumbnails, so maybe they are not a good analogy after all?

[antond-weta]

Currently it is impossible to know the number of thumbnails available in an image, right? We only have one optional thumbnail per sub-image in ImageInput, and has_thumbnail/get_thumbnail in ImageBuf. Ideally we should be able to query the number of thumbnails, and their sizes in order to pick the best one for each particular use case.

One example - a GUI app showing a gallery view, we may want to fetch the thumbnail closest to the current displayed thumbnail size for each image, refetch all thumbnails if the displayed size changes.
Another example - a GUI for a raw converter, where we want to allow user to visually select the crop region of the image to process - we may want to always use the biggest thumbnail for that.

I'm happy to make the suggested change for "oiio:thumbnail_sort" to hold the desired sort order. Perhaps we could also put the number of thumbnails and their resolution to the ImageSpec extra_attributes? Or would it be too kludgy?

[jinhgkim]

Perhaps we could also put the number of thumbnails and their resolution to the ImageSpec extra_attributes? Or would it be too kludgy?

Just want to note that, as a user, I would love to see the number of thumbnails and their resolutions in the ImageSpec so that I can easily access them with Python. I think it would be quite handy. For example, from an image-classification point of view, I often query an image's thumbnail and feed that into the model, since many models internally preprocess the image down to 224px or something in that range.

[kenmcgaugh]

One problem with adding the thumbnail resolutions to the ImageSpec is that you would have to ask libraw to read the each thumbnail into memory to query this information. This would defeat the purpose of using the thumbnails as a quick preview of the image, especially when the raw files are kept on network storage.

[antond-weta]

That is only true for the images where libraw does not populate the width and height fields in the thumbnail list, right? We could just read the resolution for the thumbnails where libraw provides it. In case some thumbnail shows zero size, then the user will have to fetch it to get the size.

[kenmcgaugh]

That is correct, and your suggestion is a good one.

[lgritz]

Worth considering:

Libraw 0.20.0 (our current minimum) dates from mid-2020, and 0.21.0 dates from the end of 2022.

It's not unreasonable for OIIO 3.2 (expected to release in September) to bump the minimum to 0.21 if it simplifies things on our end.

Opinions?

[antond-weta]

I wouldn't object this. We have a check like that in 11 places, would be great to get rid of them.

[lgritz]

It depends if you want this backported to 3.1. We can't backport a patch to a release branch if it requires changing the minimum version of a dependency.

But if you intended for this patch to be a 3.2+ feature, I'm happy to let a bump of the minimum LibRaw come along for the ride, if it would simplify this patch or other things you want to do with LibRaw.

[antond-weta]

I'd say let's proceed with the current code. It doesn't add much duplication.
I'm happy to put a separate PR bumping the libraw version for 3.2 if you want.

[lgritz]

Is it to picky to ask that instead of the new test being testsuite/python-thumbnail-raw, that instead it be testsuite/raw-thumbnail ?

The rationale is that it's not REALLY about testing the Python bindings (as all the "python-*" tests are), it's about testing the raw reader, it just happens to be the case that testing this facet of the reader is conveniently done with a small python script.

[antond-weta]

I can move the tests to raw-thumbnail, I assume I would also need to rewrite them in C++? Should I do the same in the jpeg PR?

[lgritz]

I can move the tests to raw-thumbnail, I assume I would also need to rewrite them in C++? Should I do the same in the jpeg PR?

No C++ is needed, I just want you to change the name of the test.
There's nothing wrong with using python for a test. It's just that all the test directory names that start with "python-*" are tests of the python bindings themselves, not tests that happen to use python.

[lgritz]

Note that there is also another thumbnail-related PR in #5236 from @jinhgkim.
Let's make sure both authors look over both PRs and make sure we are harmonized on overall strategy of how to deal with multiple thumbnails in a file. (Or at least make sure we aren't simultaneously working on two strategies that are in conflict.)

[antond-weta]

I have moved the unit tests to raw-thumbnail, expanded "raw:thumbnail_sort" to hold both sort orders.

[jinhgkim]

Since we have renamed this folder, I think it no longer belongs here. Can we move it down to where raw test is?

[jinhgkim]

Suggestion: It would be nice to rename test_raw_thumbnail.py to extractthumb.py following what testsuite/targa-thumbnail does.

[lgritz]

🤷 It's just a script within a test, it's not like it's exposed anywhere that users will see.

[jinhgkim]

Suggestion: instead of clamping, could we reject invalid inputs (negative values) early? Right now -1 means "let libraw choose" but -2 silently becomes the first entry, which I don't think anyone would guess. I think clamping positive values is fine, since the intent is at least unambiguous there

[kenmcgaugh]

Good spot. That is my mistake and is a holdover from an earlier iteration. The positive clamp is intended to allow for selecting the largest thumbnail without having to know how many thumbnails are in the file, when used with sorting.

[jinhgkim]

Just curious, would twidth * theight be a better sort key, or was tlength chosen because the dimensions aren't always populated in thumbs_list?

[lgritz]

We should presume thumbnails (reduced resolution versions of the main image) will all have the same aspect ratio. I think that you can sort by width, by height, or width*height and will still get the same ordering.

[kenmcgaugh]

@jinhgkim, yes the twidth and theight are not populated by libraw for some thumbnail types. Internally libraw uses tlength to find the largest thumbnail when the older interface is used, and is therefore always available. (I actually want the smallest thumbnail for my use-case).

[antond-weta]

I believe that I have addressed all issues raised which are within the scope of this PR.
One thing that I haven't done is changing the hint prefix from raw to oiio. It is an easy change and I don't really oppose it.
Do we still want that? Are there any other input plugins supporting multiple thumbnails per sub-image? How realistic is that we will implement this functionality there before a proper thumbnail API?

[jinhgkim]

This is optional, but we could simplify the branching like so:

Suggested change

	if (m_thumb_index == -1) {
	// Use the default from LibRaw, which will always be the largest one
	width = m_processor->imgdata.thumbnail.twidth;
	height = m_processor->imgdata.thumbnail.theight;
	} else {
	// clamp the thumbnail index
	if (m_thumb_index < 0) {
	errorfmt("Invalid thumbnail index ({})", m_thumb_index);
	return false;
	} else if (m_thumb_index >= thumb_count) {
	m_thumb_index = thumb_count - 1;
	}
	if (m_thumb_index < -1) {
	errorfmt("Invalid thumbnail index ({})", m_thumb_index);
	return false;
	}
	if (m_thumb_index == -1) {
	// Use the default from LibRaw, which will always be the largest one
	width = m_processor->imgdata.thumbnail.twidth;
	height = m_processor->imgdata.thumbnail.theight;
	} else {
	m_thumb_index = std::min(m_thumb_index, thumb_count - 1);
	}