QMovie non-anim: use QImageReader::imageCount but not nextImageDelay

Since 3f4d6279c4b0d04422efff478a5e2fb36259dbaa (khansen 2005) QMovie
calls QImageReader::read() until QImageReader::canRead() returns false.
That's apparently to support animated image formats in which the
frame count is not known at the beginnning (i.e. not specified in
the image format's metadata). But non-animated multi-frame formats
are expected to return valid values from QImageReader::imageCount();
and those also tend to keep returning true from canRead() regardless
of how many frames have been read (the interpretation of canRead()
is "does the file have the expected format?" rather than "are we
about to read too many frames?"). So, when a multi-frame image is abused
as an animation, QMovie was able to keep reading the same frame
repeatedly and pretend that the frame sequence goes on forever.
It also tended to read frames as fast as they could be decoded,
because nextImageDelay() is not usually provided, because multi-frame
image formats don't specify a frame rate in their metadata.

So now we change QMovie's behavior for image formats where
QImageIOHandler::supportsOption(Animation) returns false:
trust imageCount(), but not do not trust nextImageDelay().

But to actually jump to the next frame in this case, we also need to
call QImageReader::jumpToNextImage().

Altogether, this makes QMovie support "flipbook" animation for
multi-frame image formats, such as tiff and pdf.

Added "read frame x of c" logging in qt.gui.imageio category.

For testing, we use a pre-existing multi-frame Obj_N2_Internal_Mem.ico
file, to avoid depending on the tiff plugin.

[ChangeLog][QtGui][QMovie] QMovie now handles non-animated multi-frame
image formats (such as tiff): QImageIOHandler::imageCount() is observed,
and the default frame rate is 1 FPS.

Pick-to: 6.5 6.6 6.7
Fixes: QTBUG-117429
Change-Id: I6dad2a684e12c78a68288402e223a59427bf649e
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>

4 files changed, 60 insertions, 5 deletions

diff --git a/src/gui/image/qmovie.cpp b/src/gui/image/qmovie.cpp
index aa2e088c62..f5fdc2a0ea 100644
--- a/src/gui/image/qmovie.cpp
+++ b/src/gui/image/qmovie.cpp
@@ -148,6 +148,7 @@
 #include "qlist.h"
 #include "qbuffer.h"
 #include "qdir.h"
+#include "qloggingcategory.h"
 #include "private/qobject_p.h"
 #include "private/qproperty_p.h"
 
@@ -155,6 +156,8 @@
 
 QT_BEGIN_NAMESPACE
 
+Q_DECLARE_LOGGING_CATEGORY(lcImageIo)
+
 class QFrameInfo
 {
 public:
@@ -305,6 +308,19 @@ QFrameInfo QMoviePrivate::infoForFrame(int frameNumber)
         return QFrameInfo(); // Invalid
     }
 
+    // For an animated image format, the tradition is that QMovie calls read()
+    // until canRead() == false, because the number of frames may not be known
+    // in advance; but if we're abusing a multi-frame format as an animation,
+    // canRead() may remain true, and we need to stop after reading the maximum
+    // number of frames that the image provides.
+    const bool supportsAnimation = reader->supportsOption(QImageIOHandler::Animation);
+    const int stopAtFrame = supportsAnimation ? -1 : frameCount();
+
+    // For an animated image format, QImageIOHandler::nextImageDelay() should
+    // provide the time to wait until showing the next frame; but multi-frame
+    // formats are not expected to provide this value, so use 1000 ms by default.
+    const int nextFrameDelay = supportsAnimation ? reader->nextImageDelay() : 1000;
+
     if (cacheMode == QMovie::CacheNone) {
         if (frameNumber != currentFrameNumber+1) {
             // Non-sequential frame access
@@ -334,8 +350,12 @@ QFrameInfo QMoviePrivate::infoForFrame(int frameNumber)
                 }
             }
         }
-        if (reader->canRead()) {
+        qCDebug(lcImageIo, "CacheNone: read frame %d of %d", frameNumber, stopAtFrame);
+        if (stopAtFrame > 0 ? (frameNumber < stopAtFrame) : reader->canRead()) {
             // reader says we can read. Attempt to actually read image
+            // But if it's a non-animated multi-frame format and we know the frame count, stop there.
+            if (stopAtFrame > 0)
+                reader->jumpToImage(frameNumber);
             QImage anImage = reader->read();
             if (anImage.isNull()) {
                 // Reading image failed.
@@ -343,7 +363,7 @@ QFrameInfo QMoviePrivate::infoForFrame(int frameNumber)
             }
             if (frameNumber > greatestFrameNumber)
                 greatestFrameNumber = frameNumber;
-            return QFrameInfo(QPixmap::fromImage(std::move(anImage)), reader->nextImageDelay());
+            return QFrameInfo(QPixmap::fromImage(std::move(anImage)), nextFrameDelay);
         } else if (frameNumber != 0) {
             // We've read all frames now. Return an end marker
             haveReadAll = true;
@@ -359,15 +379,19 @@ QFrameInfo QMoviePrivate::infoForFrame(int frameNumber)
     if (frameNumber > greatestFrameNumber) {
         // Frame hasn't been read from file yet. Try to do it
         for (int i = greatestFrameNumber + 1; i <= frameNumber; ++i) {
-            if (reader->canRead()) {
+            qCDebug(lcImageIo, "CacheAll: read frame %d of %d", frameNumber, stopAtFrame);
+            if (stopAtFrame > 0 ? (frameNumber < stopAtFrame) : reader->canRead()) {
                 // reader says we can read. Attempt to actually read image
+                // But if it's a non-animated multi-frame format and we know the frame count, stop there.
+                if (stopAtFrame > 0)
+                    reader->jumpToImage(frameNumber);
                 QImage anImage = reader->read();
                 if (anImage.isNull()) {
                     // Reading image failed.
                     return QFrameInfo(); // Invalid
                 }
                 greatestFrameNumber = i;
-                QFrameInfo info(QPixmap::fromImage(std::move(anImage)), reader->nextImageDelay());
+                QFrameInfo info(QPixmap::fromImage(std::move(anImage)), nextFrameDelay);
                 // Cache it!
                 frameMap.insert(i, info);
                 if (i == frameNumber) {
diff --git a/tests/auto/gui/image/qmovie/CMakeLists.txt b/tests/auto/gui/image/qmovie/CMakeLists.txt
index e370ff22fe..52e0a347c4 100644
--- a/tests/auto/gui/image/qmovie/CMakeLists.txt
+++ b/tests/auto/gui/image/qmovie/CMakeLists.txt
@@ -14,7 +14,7 @@ endif()
 # Collect test data
 file(GLOB_RECURSE test_data_glob
     RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
-    animations/*)
+    animations/* multiframe/*)
 list(APPEND test_data ${test_data_glob})
 
 qt_internal_add_test(tst_qmovie
@@ -31,6 +31,7 @@ set(resources_resource_files
     "animations/comicsecard.gif"
     "animations/corrupt.gif"
     "animations/trolltech.gif"
+    "multiframe/Obj_N2_Internal_Mem.ico"
 )
 
 qt_internal_add_resource(tst_qmovie "resources"
diff --git a/tests/auto/gui/image/qmovie/multiframe/Obj_N2_Internal_Mem.ico b/tests/auto/gui/image/qmovie/multiframe/Obj_N2_Internal_Mem.ico
new file mode 100644
index 0000000000..8da119efdd
--- /dev/null
+++ b/tests/auto/gui/image/qmovie/multiframe/Obj_N2_Internal_Mem.ico
diff --git a/tests/auto/gui/image/qmovie/tst_qmovie.cpp b/tests/auto/gui/image/qmovie/tst_qmovie.cpp
index faa0bee3b9..4e30fe95d4 100644
--- a/tests/auto/gui/image/qmovie/tst_qmovie.cpp
+++ b/tests/auto/gui/image/qmovie/tst_qmovie.cpp
@@ -43,6 +43,9 @@ private slots:
     void emptyMovie();
     void bindings();
     void automatedBindings();
+#ifndef QT_NO_ICO
+    void multiFrameImage();
+#endif
 };
 
 // Testing get/set functions
@@ -259,5 +262,32 @@ void tst_QMovie::automatedBindings()
     }
 }
 
+#ifndef QT_NO_ICO
+/*! \internal
+    Test behavior of QMovie with image formats that are multi-frame,
+    but not normally intended as animation formats (such as tiff and ico).
+*/
+void tst_QMovie::multiFrameImage()
+{
+    QMovie movie(QFINDTESTDATA("multiframe/Obj_N2_Internal_Mem.ico"));
+    const int expectedFrameCount = 9;
+
+    QVERIFY(movie.isValid());
+    QCOMPARE(movie.frameCount(), expectedFrameCount);
+    movie.setSpeed(1000); // speed up the test: play at 10 FPS (1000% of normal)
+    QElapsedTimer playTimer;
+    QSignalSpy frameChangedSpy(&movie, &QMovie::frameChanged);
+    QSignalSpy errorSpy(&movie, &QMovie::error);
+    QSignalSpy finishedSpy(&movie, &QMovie::finished);
+    playTimer.start();
+    movie.start();
+    QTRY_COMPARE(finishedSpy.size(), 1);
+    QCOMPARE_GE(playTimer.elapsed(), 100 * expectedFrameCount);
+    QCOMPARE(movie.nextFrameDelay(), 100);
+    QCOMPARE(errorSpy.size(), 0);
+    QCOMPARE(frameChangedSpy.size(), expectedFrameCount);
+}
+#endif
+
 QTEST_MAIN(tst_QMovie)
 #include "tst_qmovie.moc"