python-pillow / Pillow

Python Imaging Library (Fork)
https://python-pillow.org
Other
12.06k stars 2.21k forks source link

Is this MPO file decoded correctly? #8052

Closed bigcat88 closed 2 months ago

bigcat88 commented 3 months ago

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?

Test image can be found here: https://github.com/comfyanonymous/ComfyUI/issues/3416#issuecomment-2097406107

bigcat88 commented 3 months ago

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.

liusida commented 3 months ago

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 https://github.com/comfyanonymous/ComfyUI/issues/3416#issuecomment-2097406107

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

bigcat88 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

radarhere commented 3 months ago

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

liusida commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

shawnington commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 https://github.com/python-pillow/Pillow/issues/8036 is resolved, then I expect both the signal and the gain map to be required.