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.

Sign up for GitHub

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

Jump to bottom

Saved .mpo are not compatible with Fujifilm W3 cameras #6720

Closed
Franky4242 opened this issue Nov 6, 2022 · 39 comments
Closed

Saved .mpo are not compatible with Fujifilm W3 cameras #6720

Franky4242 opened this issue Nov 6, 2022 · 39 comments
Labels
Awaiting OP Action Stale

Comments

@Franky4242
Copy link

Franky4242 commented Nov 6, 2022 •
edited by radarhere

What did you do?

When loading and then saving an .mpo image from fujifilm camera, the saved image breaks when viewed with a Fujifilm w3 camera (with autostereoscopic screen). There is a loading error displayed in the camera. 3D mpo loading in Pillow is ok it is possible to get left and right images but writing does not seem to be fully compatible.
My script to do this :

im = Image.open(mpo_input_filename)
im.save(mpo_output_filename, save_all=True)

What did you expect to happen?

saved image to open properly in Fujifilm camera viewer

What actually happened?

a loading error

What are your OS, Python and Pillow versions?

  • OS: Windows 11
  • Python: 3.11
  • Pillow: 9.3

Image file is provided with a link as Github does not support upload of .mpo files : http://www.christian-roux.ch/mpo/63460_pbz98_3D_MPO_70pc.mpo
It is just an example as I have not found any .mpo working after a im.save(mpo_output_filename, save_all=True). I have tried many. NB : the program Stereo Photo Maker is able to generate valid .mpo for Fujifilm cameras.
@radarhere
Copy link
Member

Hi. Without having access to a Fujifilm camera, the only way to verify a fix for this is to check with you, so this will involve trial and error.

My first attempt will be changing the MP Image Type from Undefined to Multi-frame Disparity, like your original image -
mpimagetype.mpo.zip. Does that work?

@Franky4242
Copy link
Author

Franky4242 commented Nov 6, 2022 via email •
edited by radarhere

@Franky4242
Copy link
Author

Franky4242 commented Nov 9, 2022 via email •
edited by radarhere

@Franky4242
Copy link
Author

Franky4242 commented Nov 9, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

Here's my next attempt then

  • One image just has the MPF Version, as you suggested
  • Another has the MP Image Type as 'Multi-frame Disparity', the MPF Version and the MP Individual Image Number
  • Another has the MP Image Type as 'Multi-frame Disparity', the MPF Version, the MP Individual Image Number and an EXIF block

attempt2.zip

@radarhere radarhere mentioned this issue Nov 12, 2022
Merged
@github-actions github-actions bot added the Stale label Nov 19, 2022
@github-actions
Copy link

Closing this issue as no feedback has been received.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 19, 2022
@radarhere
Copy link
Member

#6735 has been merged, adding the MP Format Version. That may or may not resolve this issue, but without further feedback, there is no way to know.

@Franky4242
Copy link
Author

Franky4242 commented Nov 19, 2022 via email •
edited by radarhere

@radarhere radarhere reopened this Nov 20, 2022
@radarhere
Copy link
Member

To be clear, there's the "Offset of Next IFD", and there's the "Individual image Data Offset". I gather you're asking about the "Individual image Data Offset".

From https://web.archive.org/web/20190227081740/http://www.cipa.jp/std/documents/e/DC-007_E.pdf,

5.2.3.3.3. Individual Image Data Offset
This field specifies the data offset to the beginning (i.e. SOI marker) of an Individual Image. The field is specified as a LONG value, and the byte order depends on MP Endian. This offset is specified relative to the address of the MP Endian field in the MP Header (see §5.2), unless the image is a First Individual Image, in which case the value of the offset shall be NULL(00000000.H).

Pillow is just writing out each image, getting the position afterwards, and calculating how far that is since the end of the last image. "relative to the address of the MP Endian field" is where 28 comes from.

@Franky4242
Copy link
Author

Franky4242 commented Nov 25, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

At the moment, you've provided one image that works, presumably directly from the camera.

You say that Stereo Photo Maker can generate a valid image. Could you share a valid image generated by Stereo Photo Maker? Having a second point of reference could help, because we could look for common elements between your two working images.

@Franky4242
Copy link
Author

Franky4242 commented Nov 26, 2022 via email •
edited by radarhere

@Franky4242
Copy link
Author

Franky4242 commented Dec 12, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

Congratulations on figuring out a solution.

Shouldn't MPOImageFile.create(left : Image, right : Image) -> MPOImageFile already be achievable through left.save(mpo_output_filename, save_all=True, append_images=[right])?
I similarly don't know what MPOImageFile.fromJPS(im : Image) -> MPOImageFile would do beyond im.save().

As for MPOImageFile.toJPS(self) -> JPEGImageFile, if that is a method to extract JPEGs from an MPO file, then shouldn't this already be achievable through the following?

from PIL import Image, ImageSequence
im = Image.open("63460_pbz98_3D_MPO_70pc.mpo")
for i, frame in enumerate(ImageSequence.Iterator(im)):
    im.save(str(i)+".jpg")

@Franky4242
Copy link
Author

Franky4242 commented Dec 12, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

JPS is a special kind of JPEG. Left and right images are stored in just 1 frame, side by side.

Oh. I did not know this. If you think that is a common operation that users dealing with MPO files would perform, then sure, feel free to create a PR for it.

@Franky4242
Copy link
Author

Franky4242 commented Dec 22, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

If I made this change (you won't want to do this exactly, but it works as a demonstration),

index 2a24eff39..e7a6562f6 100644
--- a/src/libImaging/JpegEncode.c
+++ b/src/libImaging/JpegEncode.c
@@ -210,12 +210,7 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) {
             }
             context->cinfo.smoothing_factor = context->smooth;
             context->cinfo.optimize_coding = (boolean)context->optimize;
-            if (context->xdpi > 0 && context->ydpi > 0) {
-                context->cinfo.write_JFIF_header = TRUE;
-                context->cinfo.density_unit = 1; /* dots per inch */
-                context->cinfo.X_density = context->xdpi;
-                context->cinfo.Y_density = context->ydpi;
-            }
+            context->cinfo.write_JFIF_header = FALSE;
             switch (context->streamtype) {
                 case 1:
                     /* tables only -- not yet implemented */

then when I run

from PIL import Image
im = Image.open("in.mpo")
im.save("out.mpo", save_all=True)

exiftool no longer lists a "JFIF Version". However, the resulting MPO file isn't something that Preview can read on macOS, so maybe it's not as simple as this - out.mpo.zip

@Franky4242
Copy link
Author

Franky4242 commented Dec 23, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

I'm not convinced that our MPO files shouldn't contain a JFIF header just because your camera doesn't accept that? If you can find something in the specification that says that we shouldn't have it, then sure.

This might be another form of #812.

http://dev.exiv2.org/projects/exiv2/wiki/The_Metadata_in_JPEG_files

In theory, Exif APP1 is recorded immediately after the SOI marker (the marker indicating the beginning of the file). However, this leads to the incompatibility between the Exif and JFIF standards because both of them specify that their particular application segment (APP0 for JFIF, APP1 for Exif) must be the first in the image file. In practice, most JPEG files contain a JFIF marker segment (APP0) that precedes the Exif APP1. This allows older readers to correctly handle the format JFIF segment, while newer readers also decode the following Exif segment, being less strict about requiring it to appear first. This way will not affect the image decoding for most decoders, but poorly designed JFIF or Exif parsers may not recognize the file properly.

@Franky4242
Copy link
Author

Franky4242 commented Dec 24, 2022 via email •
edited by radarhere

@radarhere radarhere mentioned this issue Dec 29, 2022
Closed
@radarhere
Copy link
Member

Looking at the specification, I've come up with a different interpretation - perhaps it is not that we shouldn't have a JFIF header, but that the APP1 and APP2 markers should come first?

I've created PR #6839 for this. See what you think.

@Franky4242
Copy link
Author

Franky4242 commented Dec 29, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

Looking through Pillow's test images, I found 17 images that have the markers not in APPn order - rgb.jpg, no-dpi-in-exif.jpg, xmp_test.jpg, exif-dpi-zerodivision.jpg, flower2.jpg, pil_sample_cmyk.jpg, exif-ifd-offset.jpg, exif-200dpcm.jpg, jpeg_ff00_header.jpg, app13-multiple.jpg, truncated_app14.jpg, photoshop-200dpi-broken.jpg, pil_sample_rgb.jpg, photoshop-200dpi.jpg, invalid-exif.jpg and iptc_roundUp.jpg, iptc.jpg.

@radarhere
Copy link
Member

I noticed that in the specification, it says

Individual Image has the same structure as Exif JPEG data.

So I conclude that yes, this should be EXIF JPEG data, not JFIF.

I don't know if that excludes the possibility of having a APP0 JFIF marker after APP2. As I've said, many of our test images don't feel the need to have the markers be in order, and I can't find much on Google talking about it.

I have two branches - one that allows APP1, APP2 and then APP0, and one that prevents APP0 completely. Would you mind testing both of them with your camera?

@Franky4242
Copy link
Author

Franky4242 commented Dec 30, 2022 via email •
edited by radarhere

@radarhere
Copy link
Member

Ah, you're a Windows user, so building from source is not so simple.

Fortunately, GitHub Actions generates wheels for us. So here are the wheels you will need - Pillow mpo wheels.zip. I don't know if you're 32 or 64-bit, so I've included both.

You should be able to just pip install Pillow-9.4.0.dev0-cp311-cp311-win_amd64.whl

@Franky4242
Copy link
Author

Franky4242 commented Jan 2, 2023 via email •
edited by radarhere

@radarhere
Copy link
Member

If you're having trouble installing the wheels, then instead, here is your image saved with each of those branches - jfif_attempts.zip

@Franky4242
Copy link
Author

Franky4242 commented Jan 3, 2023 via email •
edited by radarhere

@Franky4242
Copy link
Author

Franky4242 commented Jan 6, 2023 via email •
edited by radarhere

@radarhere
Copy link
Member

so I created this missing type in TiffImagePlugin attached

Fyi, no attachment came through.

@Franky4242
Copy link
Author

Franky4242 commented Jan 6, 2023 via email •
edited by radarhere

@radarhere
Copy link
Member

Still nothing. It might not be possible for attachments to work through e-mail replies. If you would like to include it here, you could try doing so through the web interface?

@Franky4242
Copy link
Author

PIL.zip
Here are the zipped files through the web interface

@Franky4242
Copy link
Author

Hello Andrew,
Did you have time to get through the files I've sent to you?

@radarhere
Copy link
Member

If you're having trouble installing the wheels, then instead, here is your image saved with each of those branches - jfif_attempts.zip

I presume that these images did not work with your camera.

@radarhere
Copy link
Member

Looking at your attachments, I'm not seeing any changes in your TiffImagePlugin, but for MpoImagePlugin, you have

  • changed the endianness of the IFD
  • always added the APP1 EXIF segment
  • according to your comments, added excess space to the APP2 marker
  • changed the MP Types to Disparity, and setting Representative Image Flag for one of the images
  • set the Base Viewpoint Number, ConvergenceAngle and BaselineLength.

And you've also said that you exclude JFIF.

The changes are very specific (Pillow doesn't even know what the Convergence Angle of the two images is), and I'm not seeing any evidence that these changes are required by a specification. I congratulate you on figuring out how to get the images working in your camera, but I'm reluctant to say that all MPO images generated by Pillow must adhere to these standards, just for the sake of being read by Fujifilm cameras.

If you would like Pillow users to be able to control the MP Types, or to manually set Base Viewpoint Number, ConvergenceAngle and BaselineLength, sure, those seems like reasonable feature requests. However, changing the defaults and adding specific values to work in a particular situation doesn't seem justified.

Is this not something that you could just add as another plugin to your code, using Image.register_save_all to add the FujifilmMpo format that you can save images in?

@github-actions github-actions bot added the Stale label Mar 16, 2023
@github-actions
Copy link

Closing this issue as no feedback has been received.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Awaiting OP Action Stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@radarhere @Franky4242