GIF seek performance improvements

[radarhere]

Resolves #6105

1. When checking whether or not a GIF is "animated", meaning that it has more than 1 frame, Pillow gets the current frame, and then seeks to frame 1. If it works, the image is animated, and if it doesn't, it is not.

Pillow/src/PIL/GifImagePlugin.py

Lines 106 to 121 in 54be93c

	@property
	def is_animated(self):
	if self._is_animated is None:
	if self._n_frames is not None:
	self._is_animated = self._n_frames != 1
	else:
	current = self.tell()
	try:
	self.seek(1)
	self._is_animated = True
	except EOFError:
	self._is_animated = False
	self.seek(current)
	return self._is_animated

Except it might be the case that the user has already seeked forward. current might be greater than 1. If so, then Pillow doesn't need to seek anywhere - we already know the image is animated.

This PR uses that knowledge as a minor performance improvement.

2. At the moment, _seek updates the frame position, applies disposal to the previous frame, and then checks to see if there's any more data in the file.

This PR rearranges that order, so if there is no more data, the image can stay as it is.

3. is_animated and n_frames seek forward through the GIF image to determine information about how many frames are present. During those seeks, the actual images are irrelevant. So don't perform image operations like loading or making sure that transparency and disposal are applied

[AnonymouX47]

Just tested it out... great!

Can't wait till it's released 😃.

[AnonymouX47]

n_frames was immediate for a 2188-frame 1354x768 GIF, used to take about 20-30 seconds before... Makes a whole world of difference.
Thanks

[AnonymouX47]

Is there any possibility these improvements will be included in release 9.1.0?

Got a project that depends largely on it.

[radarhere]

I would consider it likely, yes.

[AnonymouX47]

Ok. Thanks.

[hugovk]

Is the performance improvement quantifiable?

[radarhere]

Running tests from #6075 with iss634.gif,

1. If GIF has already seeked past first frame, it is animated - 73cf28c

from PIL import Image
import timeit

def test_is_animated_2():
    img = Image.open("iss634.gif")
    img.seek(41)
    img.is_animated

print(timeit.timeit(stmt=test_is_animated_2, number=25))

Before This Commit: 0.991317542
After This Commit: 0.5407172090000001

2. If next byte ends the GIF, stay on the current frame - 590c616

from PIL import Image
import timeit

def test_seek_2():
    with Image.open("iss634.gif") as im:
        im.seek(41)
        try:
            im.seek(42)
        except EOFError:
            pass

print(timeit.timeit(stmt=test_seek_2, number=25))

Before This Commit: 0.9614665830000001
After This Commit: 0.514499292

3. Do not update images during n_frames or is_animated seeking - f854676

from PIL import Image
import timeit

def test_is_animated_n_frames():
    img = Image.open("iss634.gif")
    img.is_animated
    img.n_frames

print(timeit.timeit(stmt=test_is_animated_n_frames, number=25))

Before This Commit: 0.527664791
After This Commit: 0.134304667

[AnonymouX47]

For what it's worth, here is the large GIF i mentioned earlier.

[FirefoxMetzger]

I looked it over and I don't see any other quick and significant wins. There is one small thing where instead of reading the data of the current block and discarding it, we could seek instead, but this only affects slow file systems, so not sure if it's worth the added complexity:

--- a/src/PIL/GifImagePlugin.py
+++ b/src/PIL/GifImagePlugin.py
@@ -55,11 +55,15 @@ class GifImageFile(ImageFile.ImageFile):

     global_palette = None

-    def data(self):
+    def data(self, read_data=True):
         s = self.fp.read(1)
-        if s and s[0]:
+        if not read_data and s and s[0]:
+            self.fp.seek(s[0], 1)
+            return b""
+        elif s and s[0]:
             return self.fp.read(s[0])
-        return None
+        else:
+            return None

     def _open(self):

@@ -100,7 +104,7 @@ class GifImageFile(ImageFile.ImageFile):
                     self._seek(self.tell() + 1, False)
             except EOFError:
                 self._n_frames = self.tell() + 1
-            self.seek(current)
+            self._seek(current, False)
         return self._n_frames

     @property
@@ -159,7 +163,7 @@ class GifImageFile(ImageFile.ImageFile):
         if self.__offset:
             # backup to last frame
             self.fp.seek(self.__offset)
-            while self.data():
+            while self.data(read_data=False) is not None:
                 pass
             self.__offset = 0

@@ -243,7 +247,7 @@ class GifImageFile(ImageFile.ImageFile):
                         block = self.data()
                         if len(block) >= 3 and block[0] == 1:
                             info["loop"] = i16(block, 1)
-                while self.data():
+                while self.data(read_data=False) is not None:
                     pass

             elif s == b",":

Otherwise the PR looks good to me 👍

---

Note that merging this will close: #6105

[radarhere]

Thanks for the approval, but yeah - testing your diff with runs of 1000 loops, I don't see any consistent improvements, so I'm not personally inclined. You do mention slow filesystems though, and I wouldn't describe my machine like that.

[AnonymouX47]

Just curious, is there anything hindering these changes from being merged? 🤔

[radarhere]

There are no problems that I'm aware of, no. Just waiting for another Pillow core developer to give their seal of approval. Judging from the past habits, I expect this, or a version of this, to be merged by the next release on April 1.

If you need these changes sooner, you can always compile https://github.com/radarhere/Pillow/tree/gif from source yourself.

[AnonymouX47]

Just waiting for another Pillow core developer to give their seal of approval.

Okay, I guessed as much.

If you need these changes sooner, you can always compile https://github.com/radarhere/Pillow/tree/gif from source yourself.

Done that (for myself) since but the issue is... I'm concerned about the users of my project as Pillow is a core dependency and the issue concerned here affects performance and therefore user experience.

For some insight, AnonymouX47/term-image#10 was the cause of these changes.

Thanks.

[hugovk]

There are no problems that I'm aware of, no. Just waiting for another Pillow core developer to give their seal of approval. Judging from the past habits, I expect this, or a version of this, to be merged by the next release on April 1.

🦭 of ✅!

Yep, this will be in 9.1.0 on April 1.