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

Using seek to skip more than one frame with FliImageFile only shows the pixels changed that frame #2649

Closed
adambiser opened this issue Aug 4, 2017 · 9 comments · Fixed by #2650
Labels
Bug Any unexpected behavior, until confirmed feature.

Comments

@adambiser
Copy link

What did you do?

I opened a FLI file and used .seek(50) on the image before creating a PhotoImage to display it on a tix label.

What did you expect to happen?

I expected to see the complete image.

What actually happened?

I only saw the part of the image that had changed for that particular frame. The rest of the image is black.

What versions of Pillow and Python are you using?

Python 3.6.2 on Windows 7 x64
Pillow: 4.2.1

I did find that if I hack in a call to self.load() in FliImageFile's _seek() method, the frame displays fully. I don't know if this is the best way to fix the issue.

import PIL as pil
from PIL import Image,ImageTk,FliImagePlugin
import tkinter.tix as tix

class FliImageFile(FliImagePlugin.FliImageFile):
    def _seek(self, frame):
        FliImagePlugin.FliImageFile._seek(self, frame)
        # ensure that this frame is loaded
        self.load()

def createlabel(root, filename):
    label = tix.Label(root)
    label.original = Image.open(filename)
    label.original.seek(50) # Go to frame 50.
    label.photoimage = ImageTk.PhotoImage(label.original) # keep a reference!
    label.config(image=label.photoimage)
    return label

def main():
    root = tix.Tk()
    label1 = createlabel(root, 'a.fli')
    label1.pack()
    # Hack to replace PIL's FliImageFile with one that loads image data at
    # the end of each internal _seek() call.
    Image.OPEN[FliImagePlugin.FliImageFile.format] = (FliImageFile, Image.OPEN[FliImagePlugin.FliImageFile.format][1])
    label2 = createlabel(root, 'a.fli')
    label2.pack()
    root.mainloop()

main()

Using a.fli found at https://samples.libav.org/fli-flc/

Top image is what Pillow displays as-is. The bottom image uses my hack that loads the image at the end of _seek.

fli-bug

@hugovk
Copy link
Member

hugovk commented Aug 4, 2017

Minimal example to reproduce:

>>> from PIL import Image
>>> im = Image.open("a.fli")
>>> im.seek(50)
>>> im.show()

image

This works:

>>> im2 = Image.open("a.fli")
>>> im2.load()
<PixelAccess object at 0x10f6d5c30>
>>> im2.seek(50)
>>> im2.show()

image


load isn't usually done as part of open, docs:

PIL.Image.open(fp, mode='r')

Opens and identifies the given image file.

This is a lazy operation; this function identifies the file, but the file remains open and the actual image data is not read from the file until you try to process the data (or call the load() method). See new().

...

Image.load()

Allocates storage for the image and loads the pixel data. In normal cases, you don’t need to call this method, since the Image class automatically loads an opened image when it is accessed for the first time. This method will close the file associated with the image.

And some other plugins don't load after seek, but GIF does.

@wiredfool
Copy link
Member

@hugovk It doesn't though, look at the differences in the file. The shoe is not complete in the one that you posted. I think this may need to do the same thing that GIF does because of delta compression, in that a seek to n+1 is going to require all of the 0..n frames to be loaded first.

@hugovk
Copy link
Member

hugovk commented Aug 4, 2017

Here's frame 50 exported with gimp:

a fli-frame50

@adambiser
Copy link
Author

adambiser commented Aug 4, 2017

@hugovk, @wiredfool : (Referring to the images in the second post) The shoe is incomplete and the item in the center of the conveyor belt shouldn't be there at all in frame 50 (or actually, I guess it should be on the far left by that frame).

Isn't GIF actually loading the previous frame's self.tile at the top of _seek instead loading the new frame's self.tile information at the end of _seek? I tried doing something similar here and was a frame behind.

@hugovk hugovk added the Bug Any unexpected behavior, until confirmed feature. label Aug 4, 2017
@wiredfool
Copy link
Member

@adambiser Yeah, I think the gif plugin is a frame behind, to preserve the semantics of 'lazy' loading. At the end of the seek(N) call, you're going to have the image reflect frame n-1, and calling load on it will load frame N. IIRC there's even some seek-forward/back silliness there as well.

@adambiser
Copy link
Author

@wiredfool True. FliImageFile probably just needs that bit of code at the top of its _seek method, too.

    else:
        # ensure that the previous frame was loaded
        if not self.im:
            self.load()

@adambiser
Copy link
Author

Why did this get closed from #2650?
#2650 says that it does not fix this.

@hugovk
Copy link
Member

hugovk commented Aug 16, 2017

Reopening.

GitHub automatically closed it because it misunderstood "Doesn't fix #2649" as "fix #2649"!

@radarhere
Copy link
Member

I have created PR #3478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Any unexpected behavior, until confirmed feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants