[BUG] Unsettable delay for animated GIFs w/proposed fix

[GreyHak]

trafficstars

Describe the bug I am trying to reencode an animated GIF from an IOMemReader to an IOVecOutput. All the code for copying the images and creating the output works except that I cannot set the delay (FPS, FramesPerSecond) in the output. I've reviewed the code, and cannot see how I'm supposed to be able to set the delay without a change to the OIIO library.

Let me walk you through the library code's logic. In gif.h the routine GifWriteLzwImage encodes the output using the argument delay. GifWriteLzwImage is called from gif.h's GifWriteFrame routine which also gets delay from an argument. GifWriteFrame is called by gifoutput.cpp's GIFOutput::finish_subimage which passes GIFOutput::m_delay. GIFOutput::m_delay only gets set in a single place, GIFOutput::open(const std::string&, int, const ImageSpec*). This open routine is where I believe the issue to be. m_delay is set using the attribute "FramesPerSecond" in m_spec. You'll notice that open gets an ImageSpec as an argument. The passed in ImageSpec is only ever used to initialize the m_subimagespecs array. The m_subimagespecs array is never used for anything, and can be removed. It's my belief that m_spec, which is used to create m_delay, is never initializable from outside the library. From outside the library you're able to control the specs argument, but that's never actually used. So I believe the fix is to use specs to initialize m_spec, and then get the "FramesPerSecond" attribute.

To Reproduce Steps to reproduce the behavior:

1. Run code to take https://media.tenor.com/QWAo2echlVEAAAAC/cute.gif and reencode the animated GIF with the same 0.1 second delay. That's a "FramesPerSecond" attribute of 100/10.

Expected behavior It should be possible to use the OpenImageIO library to write an animated GIF with a non-default delay between frames.

Evidence No matter what I do, the animated GIFs I write with OpenImageIO always encode with the default 1 seconds delay. Hopefully my code review is evidence enough, but I'd appreciate any alternate way of viewing the fix. Maybe there's another way to set m_delay or m_spec that I'm unaware of.

Platform information:

• OIIO branch/version: master branch commit 1449b465000bf7c83e983cf870ca7b1f2966d446
• OS: CentOS Stream 9
• C++ compiler: gcc version 11.3.1 20221121
• Any non-default build flags when you build OIIO: None

IF YOU ALREADY HAVE A CODE FIX: Here's the fix that I've tested. If there's an alternate solution, please let me know. Thank you.

diff --git a/src/gif.imageio/gifoutput.cpp b/src/gif.imageio/gifoutput.cpp
index e033f221..14a8c0e4 100644
--- a/src/gif.imageio/gifoutput.cpp
+++ b/src/gif.imageio/gifoutput.cpp
@@ -83,7 +83,6 @@ private:
     int m_subimage;  // Current subimage index
     int m_nsubimages;
     bool m_pending_write;                    // Do we have an image buffered?
-    std::vector<ImageSpec> m_subimagespecs;  // Saved subimage specs
     GifWriter<Filesystem::IOProxy> m_gifwriter;
     std::vector<uint8_t> m_canvas;  // Image canvas, accumulating output
     int m_delay;
@@ -154,7 +153,9 @@ GIFOutput::open(const std::string& name, int subimages, const ImageSpec* specs)
     m_filename   = name;
     m_subimage   = 0;
     m_nsubimages = subimages;
-    m_subimagespecs.assign(specs, specs + subimages);
+    if (specs != nullptr) {
+        m_spec = *specs;
+    }
     float fps = m_spec.get_float_attribute("FramesPerSecond", 1.0f);
     m_delay   = (fps == 0.0f ? 0 : (int)(100.0f / fps));

[ankmachine]

hi @lgritz Will take pick up for dev days next week.