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

Is this MPO file decoded correctly? #8052

Closed
bigcat88 opened this issue May 11, 2024 · 16 comments · Fixed by #8056
Closed

Is this MPO file decoded correctly? #8052

bigcat88 opened this issue May 11, 2024 · 16 comments · Fixed by #8056

Comments

@bigcat88
Copy link
Contributor

bigcat88 commented May 11, 2024

What did you do?

from PIL import Image

im = Image.open("328384445-d4972d5b-3409-4d5f-a107-ccb8c5dc0177.jpg")  # MPO file
im.show()

What did you expect to happen?

Correctly decode second image

image

What actually happened?

image

What are your OS, Python and Pillow versions?

  • OS: macOS/Linux
  • Python: 3.10/3.11
  • Pillow: 10.2.0 (edited) (was 10.3.0, my mistake)

Test image can be found here: comfyanonymous/ComfyUI#3416 (comment)

@bigcat88
Copy link
Contributor Author

bigcat88 commented May 11, 2024

Sorry my bad, I tested it on Pillow 10.2

on Pillow 10.3 there is no error when decoding, just second image looks wrong.

Updated issue to reflect this. Sorry for mess.

@bigcat88 bigcat88 changed the title Could not load MPO file without LOAD_TRUNCATED_IMAGES. Does this MPO file decoded correctly? May 11, 2024
@liusida
Copy link

liusida commented May 12, 2024

Hi, @bigcat88 , 10.3.0 correctly extract the second frame with a different resolution. However, the second frame is not actually a frame, it's a gain map according to this UltraHDR format.
A JPG file should only have one frame. And people naturally expect the second frame would in the same resolution as the first one. That's the problem we have in comfyanonymous/ComfyUI#3416 (comment)

So, supporting UltraHDR is a good way to go, I guess. #8036

@bigcat88
Copy link
Contributor Author

10.3.0 correctly extract the second frame with a different resolution

No it does not, imho.

Pillow should either reject a gain map and show only primary image, or it should decode second frame correctly, even with lower resolution.

@liusida
Copy link

liusida commented May 12, 2024

btw, the tomato photo I provided eariler is not very representitive, since the gain map is all 1.0s (completely white). I didn't understand this was related to the UltraHDR format earlier.

Here is another photo I took that has something in the gain map:

1000014283

we can see the gain map is smaller comparing to the original photo.
image

@bigcat88
Copy link
Contributor Author

btw, the tomato photo I provided eariler is not very representitive, since the gain map is all 1.0s (completely white).

please look at my first message.
It is not white.

@shawnington
Copy link

perhaps gain maps need their own type returned from .get_bands() or some way to explicitly exclude them from ImageSequence.Iterator

@radarhere radarhere changed the title Does this MPO file decoded correctly? Is this MPO file decoded correctly? May 12, 2024
@radarhere
Copy link
Member

Are there any images with a small file size that could be added to our test suite and distributed under the Pillow license?

@liusida
Copy link

liusida commented May 13, 2024

Are there any images with a small file size that could be added to our test suite and distributed under the Pillow license?

I, as the author of the tomato photo and the trees photo, agree to add these to the Pillow test suite and to adhere to its licensing terms.

I don't have smaller ones, since this is already the 'medium resolution'; the other option is 'full resolution.'

@radarhere
Copy link
Member

Pillow should either reject a gain map and show only primary image, or it should decode second frame correctly, even with lower resolution.

I've created #8056 to treat Ultra HDR images as standard JPEGs, ignoring the gain map. I would consider decoding the second frame correctly to be a matter for #8036

@radarhere
Copy link
Member

Thanks very much for the test image. I've included it in the PR.

@shawnington
Copy link

Pillow should either reject a gain map and show only primary image, or it should decode second frame correctly, even with lower resolution.

I've created #8056 to treat Ultra HDR images as standard JPEGs, ignoring the gain map. I would consider decoding the second frame correctly to be a matter for #8036

Im not familiar with the specs for MPO or Ultra HDR, but I am assuming the smaller gain map is embedded in the exif data of the main photo and is being extracted as a seperate file when opening images? Is there any way to reattach the gain map to the exif of the main image so the data is still there later but it is only presented as a single image? I imagine there are future uses where preserving the data is preferred behavior, but presenting them as seperate image is not.

@radarhere
Copy link
Member

My suggestion will just change the automatic detection of the image format. If a user would like to retain the current behaviour, they could open it directly as an MPO

from PIL import MpoImagePlugin
im = MpoImagePlugin.MpoImageFile('input.jpg')
im.show()

@shawnington
Copy link

shawnington commented May 13, 2024

Gotcha, My quick read suggests MPO is MultiPictureObject format, intended for stereographic images, but the Ultra HDR spec hijacks it using the 2nd image as a gain map.

It seems like this being in xmp meta-data separates one of these UltraHDR images from a true MPO stereoscopic file where both images would be wanted:

<rdf:li rdf:parsetype="Resource,
    <Container: Item
    Item: Semantic="GainMap"
    Item: Mime=" image/jpeg"
    Item: Length="66171" />
</rdf:li>

It seems like users should not have to be familiar with different implementations of the same file specification to be able to get the proper output when opening a file, so a quick parse of the XMP looking for Item: Semantic="GainMap", should differentiate between a true MPO stereoscopic file, and an UltraHDR file, and dictate if standard opening behavior shows one image, or two.

@radarhere
Copy link
Member

In #8056, I'm checking for the "Signal of the format", 'hdrgm:Version'.

If you're trying to say that isn't the best way to check if an image is Ultra HDR or not, I'm not following why. Nor does the spec seem to indicate that an Ultra HDR image will be anything other than a single JPEG without a gain map.

@shawnington
Copy link

Gotcha, I was suggesting that maybe checking both is a good way to guard against improper spec implementation. It seems less likely to me that they forget to label their gain map in the XMP than it is that they specify the hdrgm:Version incorrectly. In any case, they are both in the XMP, and redundancy never hurt anyone.

@radarhere
Copy link
Member

For the moment, unless you've found a malformed image like that in the wild, I'd rather not double check. When it comes to identifying a format, I don't think it's Pillow's overall style.

If/when #8036 is resolved, then I expect both the signal and the gain map to be required.

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

Successfully merging a pull request may close this issue.

4 participants