bluescan / tacentview

An image and texture viewer for tga, png, apng, exr, dds, pvr, ktx, ktx2, astc, pkm, qoi, gif, hdr, jpg, tif, ico, webp, and bmp files. Uses Dear ImGui, OpenGL, and Tacent. Useful for game devs as it displays information like the presence of an alpha channel and querying specific pixels for their colour.
ISC License
339 stars 33 forks source link

[Bug] Animated images playing at the wrong speed #87

Closed ClangPan closed 1 year ago

ClangPan commented 1 year ago

Animated images (GIF, aPNG or aWEBP) don't play at the correct rate, though the right speed seems to be detected in the UI.

Expected result: ezgif-5-245900e1eb

In Tacentview: Peek 2022-11-05 19-01

Happens on every image I could find. The difference in speed seems slightly inconsistent from image to image, but it's always slower than the original. It's much more noticeable of images with low periods, especially those around the 60Hz range.

ClangPan commented 1 year ago

The culprit seems to be that code snippet right here (image.cpp, line 1435): original

if (FrameDurationPreviewEnabled)
    FrameCurrCountdown = FrameDurationPreview;
else
    FrameCurrCountdown = GetCurrentPic()->Duration;

The problem is that resetting the countdown like this also makes it longer than it should be. I've found that adding the frame duration to the countdown produced a much more faithful result: fixed

if (FrameDurationPreviewEnabled)
    FrameCurrCountdown += FrameDurationPreview;
else
    FrameCurrCountdown += GetCurrentPic()->Duration;

It does have the problem of freezing the program if the preview period is set to 0 and the more minor problem that the image sometimes speeds up dramatically for only 0.25-0.5 seconds when loaded, it plays normally afterwards.

There's also the possibility of changing the 'if' above (line 1405) to a while (in addition to the above change): frame-skip

-if (FrameCurrCountdown <= 0.0f)
+while (FrameCurrCountdown <= 0.0f)

But it creates a frame skipping problem. Both ways seems to create stuttering however, need a second pair of eyes on this.

Original for comparison: 8-cell-simple

bluescan commented 1 year ago

Yeah... not quite sure what's going on... will take a look (but may have to wait till weekend). You are probably right about the reset adding too much -- I suspect it will be because the frame time has already been decremented that frame or something like that. Also, I don't think I ever got vsync working properly on Linux (may need to revisit).

ClangPan commented 1 year ago

The slowdown also happens on Windows

bluescan commented 1 year ago

Had to make a few minutes for this ;) I think your += solution is more or less correct. What you're doing there is equivalent to subtracting the remainer (how much below zero we went when we detected the time was up). There are two further issues/knockons you identified.

1) The 'speedup' when draging the window or loading an image is because the dt value gets really large cuz the update loop uses glfwGetTime() -- and I think it's fair to framerate-limit it. In TacentView.cpp change the two lines below (30fps is a reasonable minimum)

while (!glfwWindowShouldClose(Viewer::Window) && !Viewer::Request_Quit)
{
double currUpdateTime = glfwGetTime();

double elapsed = tMath::tMin(currUpdateTime - lastUpdateTime, 1.0/30.0);   ///// ADD HERE
Viewer::Update(Viewer::Window, elapsed);                                                        ///// MOD HERE

2) Really small values for the frame duration. Easy to repo by just setting the frame duration preview on and choosing a small preview-duration (say 0). For this, it seems to me there's no benefit (internally) in having a duration so small your computer wouldn't be able to display all the frames in the gif/apng/webp/whatever. I mean let's say you're running at 60fps, and you set a duration that requires 1000fps (dur = 1/1000) -- well, we 'could' skip frames, but I think I'd prefer that it just means 'go as fast as you can' but don't skip... faster machines would just be able to support lower durations.. and 0 would always mean one image frame per actual frame.

The clamp in the code below basically (should) do this:

float remainder = -FrameCurrCountdown;
float frameDuration = FrameDurationPreviewEnabled ? FrameDurationPreview : GetCurrentPic()->Duration;
tMath::tiClampMin(frameDuration, remainder);
FrameCurrCountdown = frameDuration - remainder;

This code would replace the 'if-else' where you were poking around. I've explicitly declared the remainder so it's easy to clamp the frameDuration. i.e. frameDuration can't go below the current remainer. If it does, frameDuration == remainder and the countdown gets set to 0... and we'd go at 'full speed' without FrameCurrCountdown getting more and more negative.

I haven't done much testing, but if you're up to it, LMK if you see any side-effects from the changes above.

ClangPan commented 1 year ago

I have tested with multiple images of different speed / formats / dimension and haven't found any side effects yet! It doesn't stutter and they all play at the correct speed (tested frame-by-frame, no delays) Peek 2022-11-08 10-15

Editing the preview period also produces the expected results

bluescan commented 1 year ago

Nice. I'll submit the fix sometime today. Thanks for noticing this and telling me where to look.

As a side note.. the aPNG above looked like it was doing a little skip in Chrome, so I tried to open it and noticed that tacent view wasn't detecting it as an animated PNG at all. I have a fix for this too (in the base tacent library)... and after the fix there's no stutter in TacentView, so the chrome renerer must have a small issue.

As a second note, there is a wierd line at the top not present in the gif. If tacent View is used to save the gif as an apng, it goes away -- so I presume whatever tool was being used to make it also has a small issue... was just double checking it wasn't this viewer. Thx again.

ClangPan commented 1 year ago

Oh if you're talking about the aPNG in my last message, this is just my screen recorder. I haven't tried to make it loop seemlessly or anything. The line seems to also be a bug from it. This isn't an issue on your end or from TacentView don't worry ^^

It's odd that it doesn't detect it as an animated image tho, most aPNGs with the '.png' extension work, that might just be weird formatting from my recorder again. ezgif-2-63909634f1

The GIF converted into a aPNG (.png) with Ezgif.com or imagemagick is detected correctly. No idea what sort of bottom-of-the-barrel format it's using

bluescan commented 1 year ago

Ah thanks!

It wasn't detecting cuz it wasn't looking far enough into the png file for the 'animated' four-CC tag. The file had the tag at about 1100 bytes in, but I was searching only 1024 in. That's now fixed (it looks 2048 bytes in). You will probably need to reconfigure to get that fix as it was in the base tacent library that gets fetched.

For the rest of it, it's now checked-in. CL 256791aa67111976ce6de2418a3e66598ce19006 should have it all working :)