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

Helpful error message on encode/decode #6310

Closed
wants to merge 2 commits into from
Closed

Conversation

boxed
Copy link

@boxed boxed commented May 19, 2022

Changes proposed in this pull request:

  • A nicer error message when encoding/decoding images. The code knows what encoders/decoders are available. It should just say what is valid so the user can fix their problem.

@boxed
Copy link
Author

boxed commented May 19, 2022

Researching more it looks like the entire code paths for DECODERS/ENCODERS/register_encoder/register_decoders and Image.to_bytes is just some unused code that doesn't work anymore? ENCODERS doesn't seem like it can ever be filled with "jpeg" for example, or anything else...

@radarhere
Copy link
Member

#6069 added complete support for register_encoder in Pillow 9.1.0 - https://pillow.readthedocs.io/en/stable/releasenotes/9.1.0.html#added-pyencoder. So that's not code that doesn't work anymore, it's new code.
It is called from BlpImagePlugin

Image.register_encoder("BLP", BLPEncoder)

register_decoder is called from BlpImagePlugin, MspImagePlugin, SgiImagePlugin and PpmImagePlugin.

tobytes() is part of the public API - https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.tobytes - so if you say it is unused, that doesn't make sense to me.

@radarhere
Copy link
Member

So DECODERS and ENCODERS are for the Python codecs.

Within _getencoder, you will see that if it fails to find a Python codec in ENCODERS, it moves on to check the C codecs -

encoder = getattr(core, encoder_name + "_encoder")

Here is a list of the C codecs -

Pillow/src/_imaging.c

Lines 4025 to 4061 in 6bb408c

/* Codecs */
{"bcn_decoder", (PyCFunction)PyImaging_BcnDecoderNew, METH_VARARGS},
{"bit_decoder", (PyCFunction)PyImaging_BitDecoderNew, METH_VARARGS},
{"eps_encoder", (PyCFunction)PyImaging_EpsEncoderNew, METH_VARARGS},
{"fli_decoder", (PyCFunction)PyImaging_FliDecoderNew, METH_VARARGS},
{"gif_decoder", (PyCFunction)PyImaging_GifDecoderNew, METH_VARARGS},
{"gif_encoder", (PyCFunction)PyImaging_GifEncoderNew, METH_VARARGS},
{"hex_decoder", (PyCFunction)PyImaging_HexDecoderNew, METH_VARARGS},
{"hex_encoder", (PyCFunction)PyImaging_EpsEncoderNew, METH_VARARGS}, /* EPS=HEX! */
#ifdef HAVE_LIBJPEG
{"jpeg_decoder", (PyCFunction)PyImaging_JpegDecoderNew, METH_VARARGS},
{"jpeg_encoder", (PyCFunction)PyImaging_JpegEncoderNew, METH_VARARGS},
#endif
#ifdef HAVE_OPENJPEG
{"jpeg2k_decoder", (PyCFunction)PyImaging_Jpeg2KDecoderNew, METH_VARARGS},
{"jpeg2k_encoder", (PyCFunction)PyImaging_Jpeg2KEncoderNew, METH_VARARGS},
#endif
#ifdef HAVE_LIBTIFF
{"libtiff_decoder", (PyCFunction)PyImaging_LibTiffDecoderNew, METH_VARARGS},
{"libtiff_encoder", (PyCFunction)PyImaging_LibTiffEncoderNew, METH_VARARGS},
#endif
{"packbits_decoder", (PyCFunction)PyImaging_PackbitsDecoderNew, METH_VARARGS},
{"pcd_decoder", (PyCFunction)PyImaging_PcdDecoderNew, METH_VARARGS},
{"pcx_decoder", (PyCFunction)PyImaging_PcxDecoderNew, METH_VARARGS},
{"pcx_encoder", (PyCFunction)PyImaging_PcxEncoderNew, METH_VARARGS},
{"raw_decoder", (PyCFunction)PyImaging_RawDecoderNew, METH_VARARGS},
{"raw_encoder", (PyCFunction)PyImaging_RawEncoderNew, METH_VARARGS},
{"sgi_rle_decoder", (PyCFunction)PyImaging_SgiRleDecoderNew, METH_VARARGS},
{"sun_rle_decoder", (PyCFunction)PyImaging_SunRleDecoderNew, METH_VARARGS},
{"tga_rle_decoder", (PyCFunction)PyImaging_TgaRleDecoderNew, METH_VARARGS},
{"tga_rle_encoder", (PyCFunction)PyImaging_TgaRleEncoderNew, METH_VARARGS},
{"xbm_decoder", (PyCFunction)PyImaging_XbmDecoderNew, METH_VARARGS},
{"xbm_encoder", (PyCFunction)PyImaging_XbmEncoderNew, METH_VARARGS},
#ifdef HAVE_LIBZ
{"zip_decoder", (PyCFunction)PyImaging_ZipDecoderNew, METH_VARARGS},
{"zip_encoder", (PyCFunction)PyImaging_ZipEncoderNew, METH_VARARGS},
#endif

So your suggested error message would only report the Python codecs, and ignore the possible C codecs that could be used.

@boxed
Copy link
Author

boxed commented May 19, 2022

Hmm. Yea I see your point. I guess a good error message would then also have to do something like

c_encoders = [x for x in dir(core) if x.endswith('_encoder')]

And join ENCODERS with this c_encoders before producing the error message.

@radarhere
Copy link
Member

This may just be my opinion, but doesn't a list of up to 21 different possible decoders make for a pretty long error message?

@boxed
Copy link
Author

boxed commented May 19, 2022

Why is the length relevant? If it contains information that lets you solve the problem you are happy. If it does not, then you are unhappy.

I gave up on using this API totally because I couldn't figure out a valid encoding name. I used image.save(f, format='jpeg') instead because that worked.

@radarhere
Copy link
Member

I gave up on using this API totally because I couldn't figure out a valid encoding name. I used image.save(f, format='jpeg') instead because that worked.

While I don't know exactly what your goal was, that may have been the actual solution to your problem. The documentation for tobytes() states that

This method returns the raw image data from the internal storage. For compressed image data (e.g. PNG, JPEG) use save(), with a BytesIO parameter for in-memory data.

Also, just to point this out, if I try

from PIL import Image
im = Image.new("RGB", (1, 1))
im.tobytes("jpg")

I get

Traceback (most recent call last):
  File "PIL/Image.py", line 455, in _getencoder
    encoder = getattr(core, encoder_name + "_encoder")
AttributeError: module 'PIL._imaging' has no attribute 'jpg_encoder'. Did you mean: 'jpeg_encoder'?

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "demo.py", line 3, in <module>
    im.tobytes("jpg")
  File "PIL/Image.py", line 768, in tobytes
    e = _getencoder(self.mode, encoder_name, args)
  File "PIL/Image.py", line 457, in _getencoder
    raise OSError(f"encoder {encoder_name} not available") from e
OSError: encoder jpg not available

Which is something of a helpful error message already.

@radarhere
Copy link
Member

I've created #6415 as an alternative to this, adding notes to the documentation on where to find other decoders.

@boxed boxed closed this Jul 4, 2022
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 this pull request may close these issues.

None yet

2 participants