From e11e9ff34273f853a82b85401b0606de8be7f61f Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 2 Oct 2023 17:11:55 +0200 Subject: [PATCH] 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. Fixes: QTBUG-117429 Change-Id: I6dad2a684e12c78a68288402e223a59427bf649e Reviewed-by: Qt CI Bot Reviewed-by: Axel Spoerl (cherry picked from commit 7c313f18654d18fe253e1f1c55d6f4d92660888a) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 7fa10ab76eead6731b5d130c7a32836a502902c3) (cherry picked from commit 0f541bbc1bc2837df5362be887ed352ece335584) --- src/gui/image/qmovie.cpp | 32 +++++++++++++++--- tests/auto/gui/image/qmovie/CMakeLists.txt | 3 +- .../qmovie/multiframe/Obj_N2_Internal_Mem.ico | Bin 0 -> 25214 bytes tests/auto/gui/image/qmovie/tst_qmovie.cpp | 30 ++++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 tests/auto/gui/image/qmovie/multiframe/Obj_N2_Internal_Mem.ico diff --git a/src/gui/image/qmovie.cpp b/src/gui/image/qmovie.cpp index 5558f160f3d..56797393544 100644 --- a/src/gui/image/qmovie.cpp +++ b/src/gui/image/qmovie.cpp @@ -149,6 +149,7 @@ #include "qlist.h" #include "qbuffer.h" #include "qdir.h" +#include "qloggingcategory.h" #include "private/qobject_p.h" #include "private/qproperty_p.h" @@ -156,6 +157,8 @@ QT_BEGIN_NAMESPACE +Q_DECLARE_LOGGING_CATEGORY(lcImageIo) + class QFrameInfo { public: @@ -306,6 +309,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 @@ -335,8 +351,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. @@ -344,7 +364,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; @@ -360,15 +380,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 0130ee270b9..140614a3120 100644 --- a/tests/auto/gui/image/qmovie/CMakeLists.txt +++ b/tests/auto/gui/image/qmovie/CMakeLists.txt @@ -8,7 +8,7 @@ # 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 @@ -25,6 +25,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 0000000000000000000000000000000000000000..8da119efddfef10dca2948b298335f21868a66d3 GIT binary patch literal 25214 zcmeI434B!5^~Wz^;tV7q5F(He$q>N+(j+3Hq69Zw>aNwQSoa0xqvC?#nx~?P_OEEQ zZhtMcO8<5FRK?x8ME$!}Tt8~Xt*J^&T`1y0z%cLs{oOZr=FMa#1O)A;e{OQ#-OfGd z-23i&?s;QO57W!^?b{bWVn*~Z=2~Nj8{#oDrO=osNDJa|GbLioCQ)PJ@c@5OfiYJU zDIVbeA!f{(eOx?l{s5gJ1C42D2=J$DV9ay1iYM{Gt*di!@P`}o_O`~1kPHB1jpUN^ z>@#>MPY^gS$n-pmqbEVr+S-aw!1(sx_`X5_e@fCL?OKAM7i$gzrAmlFA?n$HX(1tI zAl~9BozP6h_;2S)j)~z%};*v6W&jQ*>cM*&9>WaYqsBhd$ZF{JDJ^f+szz&@WJM=!wxe?9d(pB?zrR3 z>8GD=rca-4&N=5CbHN1{n2RsI*j#$)rRMU>FE>|Rb(Q(`uYYZ>zy5l2^UXJ#`|rQs zJo@OP=8ZSrFn{~o-_kO!53O&3^)2utv_R!XVW!Nd1BVSwrSvqh$i@SikDTk*w|CFT zFeY>BO8P~k(O5}USwN#t?`Sjq&M*slzHOZ2}PzB9Wez zRW@#dXe3e|M2$;e&P%$OE#PAKB|W3*WgwKE9n79*TSN0~EA-o#BQ?*q6Sng#^Kbrn zHg3sp7s=+*b)30%Tz_2GSe~?AddNe5D5u z9{h;PZLq-xK5K@*-FDmg)ZJ(8@uBNK@W2C~{q(0lZJjuA;&I@Wgy~dORh9JU(PKI5 zj}MLyEen;El|Ji}UteGEUvb41%Xisjm%k#*oXs}d>`=;=IU2OnW~^C>(W6IOUMjD$ z;yZBQK%aHaXC3reyZm4N@|SP2KX`?*djhr}?YQHPqo+)n^5K9118fXOM`WpkMLt}+2qjKO`(EE@)YSUS>W=WnoluwUd34y!%>Z|Vt_p5L@l@-ud z)@LnGj2boS1kh9MXbUGFW5$e`EM5*@zCiK|*iT5N5#ci7@UmsgdO!H!gL{Dcxho@I z?Nd}#w0y6<_PX}H_uk9s2jTKAJb(WDQ)z<*B_$=|xpbd>_PO%TJMZjNXNRB5zwp8f z`|>`{JpcUjS9=`hqL-bPXkrcl+L&ML9HTv-<}cI0-fZomor`&}YfdJR-9j`rBSlgfUTm3*lj~8yOUeLl;9@gpDB#IySKi!iDSAOJ z8|fA+!%X($0R_`+Wzs%9aLa71cnNAh%hFTEOdC}w+zRk8%|We2(j(hT#-oaeh;w3) zj;a2ncqzVOflD426eQELzI(LWG#A64mW5ziLYb=(L8F%y$eieI{f+8|uC1EAXm;MsyI91!U+6Cntbp1`X>+KoWgIIta{bEb8X zz_U-r1sT@4a|`3X)465n(4qF+!u(~=EzDJB?AWnptF5*&6DCZsXO~HnCfT#gzWeTL z4m|L{)EVZ;Babx49($}g;e->+$tRy|rcIk>SR2iG=bfkSWY02}U3Qsay|HJRYp=c5 zo@IXXo8OpQZn?$WcH3>{_SDYnbEg`R2(dpEOTD{j_1eGOxY%ntA)} zx6QlnzH1gPTxb%BL{O>q&-E>^z6I8H3!pvf($#3*$>WL&E8=KC@;JL&F009xUucV! zjT&B6GpaKMyR|7OFr&wgs^~02m6je;Skc$ij2$o_UYk`O?V2$KW$`{mA`=@vqN*m0 zF+h=M#h_R;TEslscT7XYkgy04kCqQED&(|Q2tphYs>1nfzz6q@7TUjPQ8YTJsAzEQ zkaRtx<%41r7DBYJut?;p>IT=Or0m-#nP+hdV}+)qpkmt-PDVbP~H z+>C_<4x>OpYB{JOEdYc{QV4|s!r(Ef3Rpg6Dj56;p|CJ}KU;1olR>T!($XbEeND1F zhy_3=gqnbYeHIkvIKK^of^z9<$t5+5MqJqi1^Lg+!=q&P%_@}YwLB{KI^za)-rkYy5rdg-L&`Kd+)NA zN#`XuY}l|ZIfoyBexdfzLk~UgSHJpI;*?WPxsd{0OwW1Abvpw+MoD#b_4%AR--q{N z&h@|FdFP$ypr1}~=AR3p9lK3zH4)jL9>$sV4T|~b3Vie@Y@1DjJ?aasLym|Bd=H}+->+0%mn>~B>cbtu9 zgEu-a(Qag*vnUxcV#I-*^A~`=60rQ_OD`l)nEL=b#JL}M30=;8>I>|@Zzu0ypaNKJ z$6kCx&IQ-O<5LQ!_$n-24Eub)v9Zy<1MVmMj*bo=z1Sl7zrtQWarW6~-wo~*^b>Wf z%N#w-zVxpTviTMl7h7Ez=LzYJ{7pCA)Zcvb&83%1$tT^^b=O_z(-yDOephh@S#ZV~ zXXvo0rKq?&ve3UD%H}N{7URojocr_zt2dHPOgb{gVoC;N_c^Ed&pr3tUumaX&OP_s zZ_)M5rN2yu<|eD+PrZs**FJOnr8ncg$UpVeQ!QWVuf*5s^dy(sK{BWeebDv=-rql| z#~m|f%vgHml~+E*dpwqYv!ZvI3GVs+K;Be9^Q5*v8TvwSrgJygA-$ex zikJ8}UpyRN$5XyQZJ_?4{;4o`kp4ptJ*4rljCtuj?iGFR7hnAO&woCjG4u?)j{(L5 zDMx=D|G@_zJjNOSN$Ch3Kk@8}ulNb1XO%3qwY9#=tBmxN=t>jff5#nnESx-f@|X0_ zFX@Y?!gybxcS?Xz03Gqgw8J;jySh3$-|@@kI~}jagmkQswEY)dg1>O#!ll&z@959J zL$((3P6mdC*rtN$pSQl|nrlARJTE%Ac)2vy+kxY+xcYCeUcGGJ)xdn~t+$plZ!SUa zkwDLKJ;qvLxF?v49MX5E>gRYlUp%ux{ZDPD@hcg$cA+azFcuz!{&B$A?(jz^b;L_A zz4S;t9)BZShHReUxOgYWfPHrw7p#*B-tSTH)jYJyx`UH)v?txF=4tgS%@d*@=I6pn z4}+6I??Zbh?vo~^h^}UUb>}nJ{ywHXh?BwbbH4cMovZKBZx$VL z$RVfDmvXK@Ya-06ufE!R(M1<6rA*{51+Z#`D0eA6{opRe-k~&k+@k=cN$80$ z=&>XO=qlWyMHetp2n67C74}Z$B3P#HRCI7|0FD6My`dMnPtnFXy?OR@(*y+9O?ndi zKKF(YTcu0hl6As!7kn8Rv6#DM%S`5$Fnk*q%8WI8p;!>KDb3Szaxl#^v(jU>P|R=7 zN)Kq2`ZHVXO=A$H^%Q`q&#k9R!}ku9b~;_62;Wko z-_1;4cB;H`c}9J&IiTc&Yco7sg8SDXy;RqyNN=6IltjVBtHmZcHhX5OK9(_QRGSgd zo=h**oo67v5X9LMHOrJNXPXD@k`5=fj(4JHDTg96HTZkYmJl8Z}}52}(03350u zD~;NSe0DPZmZVsxWQ(0E84|gZa}t5$x^PT-IC@9Q*QBes^ZzS|CrsdXB>g337mKI4 zRQO)fXZ}^3du;2|SOrbQVt}_r-7g0Lq{@;9*a%d7aj{nkWRrS#XGpg4}2-W6|c6wZu}MZpAs$neCIht-DS{$8qU?`;fEhK?Dx!5Pd#OxefC-N;)^eumtTI_+AQlHf5CzU1|5U> zC*%`$zN5H{Y1$%a^ae-=|-!ck5f=$7zAey45=r_pht(uAu!QoB_-0 zyLlnN*A>Q8)|S_g?iPJv1;#4t8qBb&Rn$329Q@*%v18+vmE*dm zp3aRoHja;1c1b^wg(BLrjcIJ$s=BgjT>eTYW#M9{zA9phCo~qvDk`%s($oA?#Ea`{ ziedvIHm9h1%t#Z@#Ue$#s;&}K@}mA!nMJgEY@I0|IVMdu8FJ{QBdakh$1Xi6P}SJj z5HBCrxlxO4xmdimRM@ykQ9n3L5Pn4te9OIZWJMC&7GOoMZfq$w&%Y%kg{KVtC|7TnLLVX9dU^gKqL|rD6em< z<;7*RQU9V8aZ8#q@{7cQH$Aeda$Ie=KtCg*Y0?&(GDSrezq+DgOigudsDt;XK=_Y$ zX8()nN>y0rRy5T1uhnM5xp)SLKsq)VSgi=7sG_01e`XsEGs+IA2CNh-2nxi8O&D8I zH7@R`%das7!U%AkJkSt_Qbq%VrzVVVm_GvY8BYpCOsuhSbZkIo>!fwYm3?k@2z^|u+I3VUhSGuBw#^-JT7hvB+I0#5VbElXORJ} zKw6v#hA#OFAZG|Q87zntWWL`JuewO*0(uAeoqZ8$CIt~wu&N8~;653?zKNIQH4RV*QPyrK-=g!TMO8XV(>H|4YgQS{nN|Kkrzhef6r>ygn zJD)ngo99&N|9sA4$@3}aMte@>bhk$5)HWK+om+J-R9xpp9XI9cxG6vToC+$l2Xc5x zzcuN%C4CX7Cf}V`MF2XIJ@CDx-x}cgZOJ$VnuO=hsR=5qxW5+9sc|FcrHvYGeEs7; zvjxxv48&&PUC#7h-gn=9*HKG)?!T|YRwahMW22;;B~zzP9gEJx+GU}ec$#!A!mC^i zevBIOz1()(iM|N$#n`aS#@}K8{rBGoea%YQcJZym^Q0XKthGBsXnk*ojon7rxJX`U zS~W{q;68L7c0L|;NML*8-*nSW&yaT#uqD7&_Q!YFSxw~*d?AupnPtz1-Ic4uN^CP0 za8J9EzVD;=lKiIwJ7Y_>6Sf<^(b_#I4PCg;H?#iAl`CzS!@$SbX)i$6?W2qKk^eIA&HyH3o0!x7 zi@Y^*%!!v!P`kMB86`h@`gx?EgKg&j z%$YOi1N87`5w8SvJn99kk)y9`o#|3;y#tLybp1Z|oz~`3Ha)T{k}WDUto=urj>3Ga z?qip%@hkhYmq}}4PFN^gv}c}q<^|%r0{a3R18d}HLyc1_f3CjbY~Lh@Y!qdqBe`W? zDf=+l$jOG2zHV*i94+~@xzGKC9T)Vs*GTguKi@R^FTC)=TU<2nPnyQG_8QXhthU2; z<{b*9+TVpWKCxHxxqGp{1)%np&715qW%DX<_NhwKdtj~bkts!sJC@X#W?gs#+8#DR zAF#gp*bptmhHxL!P6YM_%7N8#toJ${d<&#_YCY1N_q*Ty&f4=zM%ibnon4ztc7>g< zu<}(0+2Lw!5KZo+Qu1TZ+Kx@~gY>mGxucSu?PBbmjtB2l@}~3q1=?$Dn1pVP2kypH z@09F(Zy~?qUFZpHnDmzV$S=p*Px~i9cZ@|T|UvJlY#ja5G zX~s76A#CfGvu}EnzPTHC`vD_?RXW;gCP;RUq%N|7*KbYO{w@1Z@s?b!ot?b7eD@Ai z2ie%Oj-+H34+r#rADb)x!w*07fANc7+)G{G#)ep9d>L(aICy#|I|Hj+Bd}?_RW{SI z-`8BO->Q%un`DsOk}038%5&gsmgTFg-Wh$uXHRF_COj{&&d5I8=bK`^=SOM(XJzNl z8oh$LXe>_wCIdQK$YwpSttR`cIsD#6LchDAKCUq;8Jt|*kzMr+gWj1yV_o*`ss}Qp zfVO*`d@-yhQJ^%dk?=q*h>5MC$ zk|~$G%2Rq+W~F6=+FxnT*KaFntwAn3-rRbpccb=~{Oq%}H}e3U8+HU%xu$`m-?u2C zEqXDBjY0l@$ZlV}!*vMDt8liw;kavi`I29Awd$;SRkqi#Oo7J8;fEjY^E-~JgU*2- z(CzjIH^cGm%;wbLT@q7$*vT$=^2r{SIUJV!&R$=2(B9j<2l3GUkG0Hpsc&(ldB8b< zTayW{o|`pm*4g4iDQlN686;DMLP)z=BZ-{ zOYUxsTkyK&=u4_YLSs+-)sCx@Gn_B{Y*1g)+7i~uu$R!;e4RMX#a1t;`Bl8TQ-^Gs z!*S6NXuWjn8*7_%sO!X`>v@*nX|wNI{Wjlw2+NoqR+%tJetoy2{;2WH-t?MvB0NM{ zqYq<9zs0w*_Y?iQtAqMUww&4Vu*~7GfF#>~ zlBv5g3)i)~^uf?s$z1dx=QQaielW*3xaYwvHNQ%xaNFhzYu?cuz&_ueGns>4rqA#7 zg9#mC#JOrZwzXDf)kAXSs)J-!A6Gw--0F9nm!3n{KAAROqxn4~#u^7XdkkH%V#Ph2 zZy!L1Wp!e%4w5-rZg9Tge0&?{mEG5v={hRNJE$#o(FHBxOnd|9 zjD~f@eyx$^j`eEQLBPE9A$p-^`pL+(#^47a#D4f1&hNkFH&ZHq0CwwHtZ#w;_!bDx zH%toF$#0(+eE;HA2R>#<%o>;{m0G_Y^CZRF_^d{LtI7bhR1*(<*CW3@=`X>TJyToA zclKBETS=EzB*UMWjI-0X>Cr5|XEKg2Gb@8{zbER^*@uPE?`R@#;7cII!#?y?2W!9O ldwKlV()L>5(oV_miu6&c;=cV(3cppf@IOxa%2GV1|9@