RFC: Pure Python Decoder objects #1938

wiredfool · 2016-05-31T08:58:01Z

File format decoders are currently complicated to put together.

There's no registry, we're just doing a getattr on the image.core object. This PR adds a registry, then falls back to the current method.
The decoders are all in C, a fast but unsafe language. Python is significantly slower, but safe. This is a tradeoff that we should be able to make. (in either direction)
Currently, if we do implement a decoder in python, the load phase is stuffed into the open phase. This breaks the convention of open being a relatively quick operation and by deferring the heavy work to the load phase. As an added bonus, parsing errors here will no longer lead to "cannot identify image file" errors.

This pr builds on the Python fd changes from #1934, but the underlying concept doesn't need to rely on that. It's still likely that the interface chosen there will change, requiring changes here.

Changes proposed in this pull request:

Add Image.register_decoder and Image.register_encoder to map decoder name -> a callable that returns a de/encoder.
Adds the ImageFile.PyDecoder class that implements the interface used by the C layer decoder object. (Undone, an encoder object)
Refactors the DDS image Plugin to use the new registry and decoder class.

wiredfool · 2016-06-30T11:57:01Z

Looking at merging this soon after 3.3

homm · 2016-06-30T11:58:39Z

Don't forget to rebase.

wiredfool · 2016-06-30T11:59:00Z

yeah. and docs.

aclark4life · 2016-10-03T11:39:39Z

I'm going to assume since rebasing and documentation are still needed that this one will have to wait for 3.5.

wiredfool · 2016-10-03T11:42:04Z

Yeah, the case study got converted to C. So, either we ship it as an example, but with no actual use case, or ... I'm not sure. I'd like to have a set of example code in here somewhere.

wiredfool · 2017-02-21T13:50:43Z

Now includes #2384, and a re-implementation of the Msp Decoder in python.

homm · 2017-02-21T14:32:43Z

PIL/Image.py

+    :param encoder: A callable(mode, args) that returns an
+                    ImageFile.PyEncoder object
+
+    .. versionadded:: 3.3.0


hugovk

About docs/example/DdsImagePlugin.py

Is it the same as the one in PIL? Can we instead refer to the live file? A copy in docs will be unmaintained and may at some point stop working or be a misleading example.

hugovk · 2017-02-23T07:04:53Z

PIL/Image.py

+def register_decoder(name, decoder):
+    """
+    Registers an image decoder.  This function should not be
+    used in application code.


If this function shouldn't be used in application code, should it begin with an underscore?

Scroll up. This is the same as register_extension/save/open/mime/foo. It's not for application code, it's for plugin developers.

hugovk · 2017-02-23T07:05:01Z

PIL/Image.py

+def register_encoder(name, encoder):
+    """
+    Registers an image encoder.  This function should not be
+    used in application code.


If this function shouldn't be used in application code, should it begin with an underscore?

hugovk · 2017-02-23T07:05:20Z

PIL/Image.py

@@ -433,6 +440,11 @@ def _getencoder(mode, encoder_name, args, extra=()):
        args = (args,)

    try:
+        encoder = ENCODERS[encoder_name]
+        return encoder(mode, *args + extra)


This line isn't covered by tests.

Yes, we don't have a pure python encoder yet.

hugovk · 2017-02-23T07:05:36Z

PIL/Image.py

+
+    .. versionadded:: 4.1.0
+    """
+    ENCODERS[name] = encoder


This line isn't covered by tests.

hugovk · 2017-02-23T07:06:29Z

PIL/ImageFile.py

+
+    @property
+    def handles_eof(self):
+        return self._handles_eof


This line isn't covered by tests.

That property was removed in 4.0 anyway

hugovk · 2017-02-23T07:18:42Z

Tests/test_file_msp.py

@@ -1,8 +1,12 @@
 from helper import unittest, PillowTestCase, hopper

-from PIL import Image, MspImagePlugin
+from PIL import Image, ImageFile, MspImagePlugin


ImageFile imported but unused

Yeah. You imported it to get the LOAD_TRUNCATED_IMAGES variable.

hugovk · 2017-02-23T07:20:17Z

PIL/MspImagePlugin.py

@@ -1,6 +1,5 @@
 #


This file contains tabs and spaces. Let's remove tabs.

hugovk · 2017-02-23T07:21:32Z

docs/handbook/writing-your-own-file-decoder.rst

-attributes. To be able to load the file, the method must also create a list of
-:py:attr:`tile` descriptors. The class must be explicitly registered, via a
-call to the :py:mod:`~PIL.Image` module.
+A image plug-in should contain a format handler derived from the


-> "An image"

hugovk · 2017-02-23T07:22:08Z

docs/handbook/writing-your-own-file-decoder.rst

+:py:attr:`~PIL.Image.Image.size` attributes. To be able to load the
+file, the method must also create a list of :py:attr:`tile`
+descriptors, which contain a decoder name, extents of the tile, and
+any decoder specific data. The format handler class must be explicitly


-> "decoder-specific data"

hugovk · 2017-02-23T07:22:49Z

docs/handbook/writing-your-own-file-decoder.rst

+decode method. File decoders should be registered using
+:py:meth:`PIL.Image.register_decoder`. As in the C implementation of
+the file decoders, there are three stages in the lifetime of a Python
+based file decoder:


-> "Python-based"

wiredfool · 2017-03-11T22:24:13Z

Ok, I think this is tested as much as I can get.

wiredfool added Needs Review Needs Documentation Do Not Merge labels May 31, 2016

wiredfool added this to the 3.3.0 milestone Jun 11, 2016

wiredfool modified the milestones: 3.4.0, 3.3.0 Jun 30, 2016

wiredfool force-pushed the python-decoder branch from 544267b to 32c3fcd Compare June 30, 2016 12:32

wiredfool added Needs Release Notes and removed Do Not Merge labels Jul 5, 2016

wiredfool mentioned this pull request Aug 25, 2016

Move DDS decoders to C #2068

Merged

radarhere added the Needs Rebase label Sep 22, 2016

aclark4life modified the milestones: 3.5.0, 3.4.0 Oct 3, 2016

wiredfool removed Needs Rebase Needs Documentation labels Oct 3, 2016

radarhere added the Needs Rebase label Nov 11, 2016

radarhere modified the milestones: 4.1.0, 4.0.0 Jan 7, 2017

wiredfool force-pushed the python-decoder branch from f76c966 to fb383de Compare February 21, 2017 10:42

wiredfool mentioned this pull request Feb 21, 2017

More MSP tests #2384

Closed

wiredfool removed the Needs Rebase label Feb 21, 2017

homm reviewed Feb 21, 2017

View reviewed changes

hugovk reviewed Feb 23, 2017

View reviewed changes

wiredfool mentioned this pull request Feb 28, 2017

Release Pillow 4.1.0 on April 1, 2017 #2373

Closed

19 tasks

wiredfool and others added 18 commits March 11, 2017 13:50

python decoder docs

be26453

PyDecoder documentation

1c0d1c5

s/lena/hopper/

36a22e2

Moving DdsDecoder python implementation to examples

c5d32d2

Created by forcing Pillow to save with checksum=0

cac235b

Test with bad checksum

e44773c

Test Windows v2 MSP files

98b19a9

Load MSP files to test MSP decoder

38c6771

Remove redundant assert and message

8840f2b

Assert some pixel values

069ad8c

Pure Python MSP Decoder

949932f

Added more additional files

7fe29ec

Py2.7 compatibility

f13b335

Updated versionadded to reflect 6-9 month slip [ci skip]

b668db3

handles_eof was removed in 4.0

c2cd5fe

review lint

38f43c1

Propagage ValueError out, rather than swallowing

40a94d4

UnitTest PyDecoder

d70de6e

wiredfool force-pushed the python-decoder branch from dc976ea to d70de6e Compare March 11, 2017 15:35

wiredfool added 4 commits March 11, 2017 16:30

fixed wording

31c1a65

Trigger set_as_raw error

3828330

Testing empty extents

d718d35

Test encode registry

184d4f8

wiredfool force-pushed the python-decoder branch from d93f01a to 184d4f8 Compare March 11, 2017 21:27

wiredfool merged commit c9f54c9 into python-pillow:master Mar 12, 2017

wiredfool deleted the python-decoder branch October 2, 2017 13:26

radarhere mentioned this pull request Sep 12, 2019

PyEncoder doesn't exist #4059

Closed

radarhere mentioned this pull request Feb 18, 2022

Added PyEncoder and support BLP saving #6069

Merged

radarhere mentioned this pull request Apr 1, 2024

Reading Quite OK Image (QOI) files is slow compared to PNG and qoi #7922

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Pure Python Decoder objects #1938

RFC: Pure Python Decoder objects #1938

wiredfool commented May 31, 2016

wiredfool commented Jun 30, 2016

homm commented Jun 30, 2016

wiredfool commented Jun 30, 2016

aclark4life commented Oct 3, 2016

wiredfool commented Oct 3, 2016

wiredfool commented Feb 21, 2017

homm Feb 21, 2017

wiredfool Feb 21, 2017

hugovk left a comment

hugovk Feb 23, 2017

wiredfool Feb 23, 2017

hugovk Feb 23, 2017

hugovk Feb 23, 2017

wiredfool Feb 23, 2017

hugovk Feb 23, 2017

hugovk Feb 23, 2017

wiredfool Feb 23, 2017

hugovk Feb 23, 2017

wiredfool Feb 23, 2017

hugovk Feb 23, 2017

hugovk Feb 23, 2017

hugovk Feb 23, 2017

hugovk Feb 23, 2017

wiredfool commented Mar 11, 2017

RFC: Pure Python Decoder objects #1938

RFC: Pure Python Decoder objects #1938

Conversation

wiredfool commented May 31, 2016

wiredfool commented Jun 30, 2016

homm commented Jun 30, 2016

wiredfool commented Jun 30, 2016

aclark4life commented Oct 3, 2016

wiredfool commented Oct 3, 2016

wiredfool commented Feb 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugovk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wiredfool commented Mar 11, 2017