Passing duration or fps has no effect on the output GIF using PillowPlugin

[ahuang11]

import imageio.v3 as iio
import matplotlib.pyplot as plt

gif_path = "test.gif"
frames_path = "{i}.jpg"

n = 20
plt.figure(figsize=(4,4))
for x in range(n):
    plt.scatter(x/n, x/n)
    plt.xlim(0, 1)
    plt.ylim(0, 1)
    plt.savefig(f"{x}.jpg")

frames = np.stack(
    [iio.imread(f"{i}.jpg") for i in range(n)],
    axis=0
)

iio.imwrite(gif_path, frames, fps=1, loop=0)

I've tried duration=1000 too

[FirefoxMetzger]

Interesting find. Here is a small test snippet that I created to see if I can replicate this issue:

import imageio.v3 as iio

# v3 imread + PillowPlugin will read all frames in a GIF by default
frames = iio.imread("imageio:newtonscradle.gif")
iio.imwrite("test.gif", frames, duration=1000, loop=0)

which (on imageio v2.12.2) should produce the following GIF where each frame is shown for 1s:

@ahuang11 Could you share the code you have tried with duration? I can take a look and see if I can spot the problem.

For reference, GIF supports frames at a rate up to 100 Hz (though most tools like browsers cap out at 50 Hz) and duration is specified in ms. Hence a command like duration=1 (which is 1/1000 or 1000 Hz) won't have any visual effect and it should indeed be duration=1000.

[ahuang11]

Oh, perhaps I got the duration mixed up when I tested in code (I thought duration=1 is 1 second, but I typed it correctly in this issue.

However, fps is not working for me, unless I have that mixed up too.

[FirefoxMetzger]

No, I think you got that one right. The old pillow plugin added fps on top of pillow's native duration. I didn't port that addition into the new PillowPlugin because it is redundant. It is also more limiting because duration allows an iterable with one duration per frame whereas something like fps=[1, 1/50, 1/25, 1/25] seems a bit odd.

That said, I see how fps seems like the more intuitive keyword compared to duration so we could reintroduce it as a thin wrapper around (and coexistant with) duration. (Subject to resolving the above odd edge case) What do you think?

[ahuang11]

Oh, I honestly think duration is more useful too, but since it was listed in the Pillow docs, I thought it should have worked. Perhaps if fps is specified, could raise an error instead of supporting it since zen of python says there should only be one obvious way to do something?

[FirefoxMetzger]

but since it was listed in the Pillow docs

Yes, it is part of the docs for the legacy plugin since we still have it here for backwards compatibility. You can force using it by setting plugin="GIF-PIL". The docs for PillowPlugin shouldn't mention it and the other docs will get removed once we remove the old plugin (probably when we officially step into v3.0)

Perhaps if fps is specified, could raise an error instead of supporting it

I like this idea; lets do it :) Would you be interested in contributing a PR for it?

[ahuang11]

Sure, can you point me to where it would go ideally? Under imopen if plugin is pillow?

[FirefoxMetzger]

Of course I can. It would live in PillowPlugin.write.

The way it works under the hood is that imopen selects the plugin, instantiates it, and returns it (a typical factory). imwrite calls imopen internally to figure out the plugin to use and then calls plugin.write to do the actual writing.

[ahuang11]

Thanks! I'll take a deeper look into it tomorrow

[ahuang11]

Made the PR #871. Let me know if there's anything I should change!