After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 335093 - Support for animated images
Support for animated images
Status: RESOLVED FIXED
Product: eog
Classification: Core
Component: image viewer
2.14.x
Other All
: Normal enhancement
: ---
Assigned To: EOG Maintainers
EOG Maintainers
: 540786 555650 617353 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-03-19 09:25 UTC by Corey Burger
Modified: 2010-05-02 14:37 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
patch for eog 2.24.1 and 2.24.2 (7.00 KB, patch)
2008-12-05 16:56 UTC, Raoul Berger
none Details | Review
patch against eog trunk 4902 (6.29 KB, patch)
2008-12-08 20:26 UTC, Raoul Berger
none Details | Review
patch against eog trunck 4902 (6.25 KB, patch)
2008-12-08 23:25 UTC, Raoul Berger
none Details | Review
patch against trunck 4997 (6.46 KB, patch)
2009-02-14 14:39 UTC, Raoul Berger
none Details | Review
updated patch, needs work (5.56 KB, patch)
2009-03-21 16:00 UTC, Claudio Saavedra
needs-work Details | Review
new API, gtk-doc comments, fix interpolation stuff (10.56 KB, patch)
2009-03-27 03:07 UTC, Raoul Berger
none Details | Review
updated patch (10.79 KB, patch)
2009-03-29 15:27 UTC, Raoul Berger
none Details | Review
update : fix animation lags (10.57 KB, patch)
2009-05-14 15:35 UTC, Raoul Berger
none Details | Review
removed pause/resume api (8.05 KB, patch)
2009-10-17 13:47 UTC, Raoul Berger
committed Details | Review
fix to apply to the commited patch (492 bytes, patch)
2009-10-22 01:25 UTC, Raoul Berger
committed Details | Review

Description Corey Burger 2006-03-19 09:25:02 UTC
EOG should support animated GIFs. It should display a status bar on the bottom
stating "This image in animated", with a Play or Pause button. Whether or not
EOG should animate those gifs by default, I don't know.

Reported here:
https://launchpad.net/distros/ubuntu/+source/eog/+bug/35545
Comment 2 Sophoklis Goumas 2007-03-15 11:11:52 UTC
Related gnomesupport.org forum topic:
http://gnomesupport.org/forums/viewtopic.php?t=12521
Comment 3 Michael Köchling 2008-04-30 19:24:08 UTC
EOG should adopt any features from gThumb to become a really good Image Viewer. Showing animated .gif Files is one step. Which of these projects is more active?
Comment 4 Baptiste Mille-Mathias 2008-06-29 17:07:41 UTC
*** Bug 540786 has been marked as a duplicate of this bug. ***
Comment 5 haarp 2008-07-07 23:32:09 UTC
Are there plans to implement this yet?
Comment 6 Jay 2008-07-28 00:21:52 UTC
I also think that such basic functions like animating a gif should be enabled by default. Couldn't the code from gThumb be modified to fit?
Comment 7 Claudio Saavedra 2008-10-09 10:24:32 UTC
*** Bug 555650 has been marked as a duplicate of this bug. ***
Comment 8 Jayrome Noyt 2008-10-09 11:59:12 UTC
EOG should animate. Play/Pause buttons, etc., are not necessary.
Comment 9 Raoul Berger 2008-11-25 23:31:42 UTC
a patch is here : http://raoulito.info/eog_animagifs/
Comment 10 antistress 2008-11-26 22:38:42 UTC
agree with comment # 8
Comment 11 Claudio Saavedra 2008-12-05 16:43:29 UTC
(In reply to comment #9)
> a patch is here : http://raoulito.info/eog_animagifs/
> 

Why don't we we get a patch here in bugzilla instead?
Comment 12 Raoul Berger 2008-12-05 16:56:46 UTC
Created attachment 124012 [details] [review]
patch for eog 2.24.1 and 2.24.2

Sorry I should have uploaded the patch on bugzilla :)
Comment 13 Claudio Saavedra 2008-12-05 17:12:54 UTC
Sorry for not being precise. A patch against trunk is what could be most useful.
Comment 14 Raoul Berger 2008-12-05 18:11:37 UTC
Comment on attachment 124012 [details] [review]
patch for eog 2.24.1 and 2.24.2

diff -Naur eog-2.24.2/src/eog-image.c src/eog-image.c
--- eog-2.24.2/src/eog-image.c	2008-12-05 19:13:22.000000000 +0100
+++ src/eog-image.c	2008-12-05 19:13:36.000000000 +0100
@@ -94,6 +94,12 @@
 	if (priv->status == EOG_IMAGE_STATUS_LOADING) {
 		eog_image_cancel_load (image);
 	} else {
+	       if (priv->animation != NULL) {
+		        g_object_unref (priv->animation);
+		  	priv->animation_iter = NULL; 
+			priv->animation = NULL;
+		}
+
 		if (priv->image != NULL) {
 			g_object_unref (priv->image);
 			priv->image = NULL;
@@ -240,6 +246,8 @@
 
 	img->priv->file = NULL;
 	img->priv->image = NULL;
+	img->priv->animation = NULL;
+	img->priv->animation_iter = NULL;
 	img->priv->thumbnail = NULL;
 	img->priv->width = -1;
 	img->priv->height = -1;
@@ -1019,9 +1027,18 @@
 		if (priv->image != NULL) {
 			g_object_unref (priv->image);
 		}
-
-		priv->image = gdk_pixbuf_loader_get_pixbuf (loader);
-
+		
+		priv->animation = gdk_pixbuf_loader_get_animation (loader);
+		
+		if (gdk_pixbuf_animation_is_static_image (priv->animation)) {
+		  priv->image = gdk_pixbuf_animation_get_static_image (priv->animation);
+		  priv->animation = NULL;
+		} else { 
+		  priv->animation_iter =
+		    gdk_pixbuf_animation_get_iter (priv->animation,NULL);
+		  priv->image = gdk_pixbuf_animation_iter_get_pixbuf (priv->animation_iter);
+		}
+		
 		if (G_LIKELY (priv->image != NULL)) {
 			g_object_ref (priv->image);
 
@@ -1205,6 +1222,7 @@
 	return image;
 }
 
+
 #ifdef HAVE_LCMS
 cmsHPROFILE
 eog_image_get_profile (EogImage *img)
@@ -1965,3 +1983,66 @@
 
 	return (result != NULL);
 }
+
+
+/* Temporary fix to display animated GIFs */
+gboolean
+eog_image_is_animation (EogImage *img) {
+
+  g_return_val_if_fail (EOG_IS_IMAGE (img), FALSE);
+
+  if (img->priv->animation == NULL)
+    return FALSE;
+  else 
+    return TRUE;
+}
+
+GdkPixbufAnimationIter * 
+eog_image_get_animation_iterator (EogImage *img)
+{
+  GdkPixbufAnimationIter *animation_iter = NULL;
+  
+  g_return_val_if_fail (EOG_IS_IMAGE (img), NULL);
+  
+  g_mutex_lock (img->priv->status_mutex);
+  animation_iter = img->priv->animation_iter;
+  g_mutex_unlock (img->priv->status_mutex);
+  
+  if (animation_iter != NULL) {
+    g_object_ref (animation_iter);
+  }
+  return animation_iter;
+}
+
+gboolean 
+eog_image_iter_advance (EogImage *img) {
+  g_return_val_if_fail (EOG_IS_IMAGE (img), FALSE);
+  g_return_val_if_fail (GDK_IS_PIXBUF_ANIMATION_ITER (img->priv->animation_iter), FALSE);
+
+  return gdk_pixbuf_animation_iter_advance (img->priv->animation_iter, NULL);
+}
+
+void
+eog_image_iter_set_pixbuf (EogImage *img) {
+  EogImagePrivate *priv;
+
+  g_return_if_fail (EOG_IS_IMAGE (img));
+  
+  priv = img->priv;
+
+  g_mutex_lock (priv->status_mutex);
+  g_object_unref (priv->image);
+  priv->image = 
+    gdk_pixbuf_animation_iter_get_pixbuf (priv->animation_iter);
+  g_object_ref (priv->image);
+  g_mutex_unlock (priv->status_mutex);
+  /* keep the transformation over time */
+  if (EOG_IS_TRANSFORM (priv->trans))
+    {
+      GdkPixbuf* transformed = eog_transform_apply (priv->trans, priv->image, NULL);
+      g_object_unref (priv->image);
+      priv->image = transformed;
+      priv->width = gdk_pixbuf_get_width (transformed);
+      priv->height = gdk_pixbuf_get_height (transformed);
+    }
+}
diff -Naur eog-2.24.2/src/eog-image.h src/eog-image.h
--- eog-2.24.2/src/eog-image.h	2008-12-05 19:13:22.000000000 +0100
+++ src/eog-image.h	2008-12-05 19:14:06.000000000 +0100
@@ -195,6 +195,12 @@
 
 gboolean          eog_image_is_supported_mime_type   (const char *mime_type);
 
+/* Temporary fix to display animated GIFs */
+gboolean eog_image_iter_advance (EogImage *img);
+gboolean eog_image_is_animation (EogImage *img);
+GdkPixbufAnimationIter *eog_image_get_animation_iterator (EogImage *img);
+void eog_image_iter_set_pixbuf (EogImage *img);
+
 G_END_DECLS
 
 #endif /* __EOG_IMAGE_H__ */
diff -Naur eog-2.24.2/src/eog-image-private.h src/eog-image-private.h
--- eog-2.24.2/src/eog-image-private.h	2008-12-05 19:13:23.000000000 +0100
+++ src/eog-image-private.h	2008-12-05 19:13:52.000000000 +0100
@@ -27,13 +27,18 @@
 G_BEGIN_DECLS
 
 struct _EogImagePrivate {
-	GFile            *file;
+        GFile            *file;
 
-	EogImageStatus    status;
-	EogImageStatus    prev_status;
+        EogImageStatus    status;
+        EogImageStatus    prev_status;
 	gboolean          is_monitored;
 
 	GdkPixbuf        *image;
+
+       /* Temporary fix to display animated GIFs  */
+        GdkPixbufAnimationIter *animation_iter;
+        GdkPixbufAnimation     *animation;
+ 
 	GdkPixbuf        *thumbnail;
 	
 	gint              width;
diff -Naur eog-2.24.2/src/eog-scroll-view.c src/eog-scroll-view.c
--- eog-2.24.2/src/eog-scroll-view.c	2008-12-05 19:13:23.000000000 +0100
+++ src/eog-scroll-view.c	2008-12-05 19:14:28.000000000 +0100
@@ -1842,11 +1842,43 @@
 	return DOUBLE_EQUAL (view->priv->zoom, MAX_ZOOM_FACTOR);
 }
 
+/* Temporary fix to display animated GIFs */
+typedef struct {
+  EogImage *image;
+  EogScrollView *view;
+} EogImgScrollAssoc;
+
+static gboolean
+load_and_display_next_frame (gpointer data) {
+  
+  EogImgScrollAssoc *assoc = (EogImgScrollAssoc*) data;
+  EogImage *image = assoc->image;
+  
+  if (!GDK_IS_PIXBUF_ANIMATION_ITER (eog_image_get_animation_iterator (image))) {
+       /* another image / sequence has just been set */
+    g_free (data);
+    return FALSE;
+  }
+  
+  if (eog_image_iter_advance (image)) { /* need to refresh */
+    EogScrollView *view;
+    EogScrollViewPrivate *priv;
+    
+    view = assoc->view;
+    priv = view->priv;
+
+    eog_image_iter_set_pixbuf (image); 
+    priv->pixbuf = eog_image_get_pixbuf (image);
+    gtk_widget_queue_draw (GTK_WIDGET (priv->display));
+  }
+  return TRUE;
+}
+
 void
 eog_scroll_view_set_image (EogScrollView *view, EogImage *image)
 {
 	EogScrollViewPrivate *priv;
-
+	
 	g_return_if_fail (EOG_IS_SCROLL_VIEW (view));
 
 	priv = view->priv;
@@ -1867,14 +1899,29 @@
 	priv->progressive_state = PROGRESSIVE_NONE;
 	if (image != NULL) {
 		eog_image_data_ref (image);
-
+		
 		if (priv->pixbuf == NULL) {
 			priv->pixbuf = eog_image_get_pixbuf (image);
 			priv->progressive_state = PROGRESSIVE_NONE;
 			set_zoom_fit (view);
 			check_scrollbar_visibility (view, NULL);
 			gtk_widget_queue_draw (GTK_WIDGET (priv->display));
-
+			
+			/* Temporary fix to display animated GIFs */
+			
+			if (eog_image_is_animation (image))
+			  {
+			    EogImgScrollAssoc * assoc = g_try_malloc (sizeof(EogImgScrollAssoc));
+			    g_return_if_fail (assoc != NULL);
+			    
+			    assoc->image = image;
+			    assoc->view = view;
+			    
+			    /* this timer will update our sequence */
+			    g_timeout_add 
+			      (gdk_pixbuf_animation_iter_get_delay_time (eog_image_get_animation_iterator (image)), 
+			       load_and_display_next_frame, assoc);
+			  }
 		}
 		else if (priv->interp_type != GDK_INTERP_NEAREST &&
 			 !is_unity_zoom (view))
Comment 15 Raoul Berger 2008-12-05 18:15:43 UTC
(In reply to comment #13)
> Sorry for not being precise. A patch against trunk is what could be most
> useful.
> 

Well, I don't have a subversion account actually... maybe I can send a request or you could commit it for me ?
Comment 16 Claudio Saavedra 2008-12-06 12:37:25 UTC
(In reply to comment #15)
> (In reply to comment #13)
> > Sorry for not being precise. A patch against trunk is what could be most
> > useful.
> > 
> 
> Well, I don't have a subversion account actually... maybe I can send a request
> or you could commit it for me ?
> 

Actually, that's not the way to go. I first need a patch against trunk that I can test/review. Then I can commit if it is good enough to go in, otherwise we will work on improve it. You don't need a SVN account, just grab eog from trunk[1], apply your patch, fix whatever you may need to fix, and later generate a new version with "svn diff". When you have it, please attach the patch as a attachment instead of a comment. Thanks a lot!

[1] See the read-only check-out access section here: http://live.gnome.org/Subversion
Comment 17 Raoul Berger 2008-12-08 20:26:44 UTC
Created attachment 124204 [details] [review]
patch against eog trunk 4902
Comment 18 Raoul Berger 2008-12-08 20:28:56 UTC
> Actually, that's not the way to go. I first need a patch against trunk that I
> can test/review. Then I can commit if it is good enough to go in, otherwise we
> will work on improve it. You don't need a SVN account, just grab eog from
> trunk[1], apply your patch, fix whatever you may need to fix, and later
> generate a new version with "svn diff". When you have it, please attach the
> patch as a attachment instead of a comment. Thanks a lot!
> 
> [1] See the read-only check-out access section here:
> http://live.gnome.org/Subversion
> 

Ok :) Now I know what to do in the future.
I sent a patch against trunk and I'm waiting for your review.
Comment 19 Raoul Berger 2008-12-08 23:25:35 UTC
Created attachment 124217 [details] [review]
patch against eog trunck 4902

modified : eog_image_is_animation (EogImage *img)
Comment 20 Tor Lillqvist 2009-01-27 22:38:31 UTC
Retitling as there is nothing GIF-specific in the request or patch. Luo, if the last patch supersedes your earlier ones, please mark them as obsolete.
Comment 21 Raoul Berger 2009-02-04 18:49:40 UTC
(In reply to comment #20)
> Retitling as there is nothing GIF-specific in the request or patch. Luo, if the
> last patch supersedes your earlier ones, please mark them as obsolete.
> 

Yes it supersedes them.
But it seems like I can't change the attachment status to obsolete. Is it because the bug isn't assigned to me or am I missing something ?

Comment 22 Tor Lillqvist 2009-02-05 11:18:26 UTC
Hmm, I don't know exactly how bugzilla works. Anyway, at least I can mark them as obsolete, doing that.
Comment 23 amano 2009-02-07 16:01:58 UTC
What images will be animated with this patch? Gif? APNG? This would be a great step towards GNOME 3.0 if the image viewer could display animated images. 
Comment 24 Raoul Berger 2009-02-11 13:25:27 UTC
(In reply to comment #23)
> What images will be animated with this patch? Gif? APNG? This would be a great
> step towards GNOME 3.0 if the image viewer could display animated images. 
> 

Only GIF files will be animated (the "gdk-pixbuf " library is used). I don't think APNG will be supported anytime soon...  For MNG files, I found this discussion : http://bugzilla.gnome.org/show_bug.cgi?id=71267.
Comment 25 Florian Dorn 2009-02-12 18:35:06 UTC
I haven't really tested your patch, but I noticed, that you do reinstall the timeout after every frame.

gdk_pixbuf_animation_get_iter:
> Each time the image is updated, you should reinstall the timeout with the new, possibly-changed delay time. 
Comment 26 Raoul Berger 2009-02-14 14:35:25 UTC
(In reply to comment #25)
> I haven't really tested your patch, but I noticed, that you do reinstall the
> timeout after every frame.
> 
> gdk_pixbuf_animation_get_iter:
> > Each time the image is updated, you should reinstall the timeout with the new, possibly-changed delay time. 
> 
Thank you for your comment. I keep the same delay during the whole animation, which is wrong. Fixing that.
(Please, someone to mark the old patch as obsolete).
Comment 27 Raoul Berger 2009-02-14 14:39:17 UTC
Created attachment 128718 [details] [review]
patch against trunck 4997
Comment 28 Vadim Peretokin 2009-03-06 20:27:02 UTC
Thank you Luo for looking into this, I hope it'll be in the program soon. Using a non-image viewer such as Firefox for gifs is inconvenient and doesn't make sense.
Comment 29 Vincent Trouilliez 2009-03-20 10:09:30 UTC
I just tested the patch, thanks to someone in the Launchpad/Ubuntu bug report, who built a package with the patch, against Ubuntu 9.04.

Appears to work just fine, animates GIF properly and CPU load is non-existent (as it should).
Comment 30 Claudio Saavedra 2009-03-21 15:58:57 UTC
I had an initial look at the patch now. I have a few questions and comments.

- Why does eog_image_iter_set_pixbuf() need to be public at all? Why eog_image_iter_advance() doesn't automatically set the pixbuf?
- I'm not following well the  /*destroy timer*/ part in the load_and_display_next_frame() function. Can't you destroy the timer when a new image is set instead?
- The coding style is way different from the rest of the eog code. As I feel a bit guilty for not having tested your patch before, I took the freedom to improve this to make it up. I'll attach an updated patch next.
- Please document all the new public API using gtk-doc like comments to make easier to understand your design.

That would be for now, thanks for the patch.

Comment 31 Claudio Saavedra 2009-03-21 16:00:35 UTC
Created attachment 131091 [details] [review]
updated patch, needs work
Comment 32 Raoul Berger 2009-03-22 16:36:08 UTC
(In reply to comment #30)
> I had an initial look at the patch now. I have a few questions and comments.
> 

Claudio, thanks for your comments and your update of the patch. I'll post a better (I hope) patch next week, add some gtk-doc style comments and I'll answer the questions you asked too.

I also want to thank the people who applied the patch and repported some issues (they used FF and EOG to compare, and sometimes patched EOG plays a sequence slightly faster). So... I'm working on this.
Comment 33 Raoul Berger 2009-03-27 03:02:09 UTC
(In reply to comment #30)
Hello,

Claudio, your remarks were usefull to improve the patch.
> - Why does eog_image_iter_set_pixbuf() need to be public at all? Why
> eog_image_iter_advance() doesn't automatically set the pixbuf?

Now eog_image_iter_advance() sets the pixbuf.

> - I'm not following well the  /*destroy timer*/ part in the
> load_and_display_next_frame() function. Can't you destroy the timer when a new
> image is set instead?

I updated the private func free_image_resources() located in eog-scroll-view.c. Now it removes the display_next_frame callback.

I made other few modifications (patch is attached next),

*dev side :
There are 4 public API functions : eog_image_is_animation(), eog_image_pause_animation(), eog_image_continue_animation() and eog_image_is_playing(). 

One new signal : "next-frame" emitted when the animated image advances to the next frame.

Please see the gtk-doc style comments for more details.

So, what do you think everyone ? Is the API correct or should we add / modify some functions or their prototypes ? Let's discuss about this (actually I know I should have asked before coding).

*user side :
For animations, we don't delay interpolation anymore, otherwise this can lead to a weird effect (that is, each frame is quickly displayed with nearest neihbor then the interpolation is done).

I'm waiting for your answers, advices, new patches.
Comment 34 Raoul Berger 2009-03-27 03:07:30 UTC
Created attachment 131478 [details] [review]
new API, gtk-doc comments, fix interpolation stuff
Comment 35 Raoul Berger 2009-03-29 15:27:19 UTC
Created attachment 131623 [details] [review]
updated patch

added EogImage class member : void (* next_frame) (EogImage *img, gint delay)
Comment 36 amano 2009-05-08 15:18:13 UTC
Is this going to be inlcuded for Gnome 2.28? It would be great if it would be included early in the circle. Please don't let this great stuff fall off the radar.
Comment 37 Raoul Berger 2009-05-14 15:35:18 UTC
Created attachment 134649 [details] [review]
update : fix animation lags

This update hopefully fixes animation lags...

Please note :
Zero-delay intermediate frames will be displayed during 100 msec.

According to A. Thyssen (see http://www.imagemagick.org/Usage/anim_basics/#zero), those frames shouldn't be displayed at all. 
According to gdk-pixbuf library devs : "GIFs with delay time 0 are mostly broken, but they just want a default, "not that fast" delay" (see gtk/gdk-pixbuf/io-gif.c). That's why 0ms is replaced by 100ms.
Comment 38 amano 2009-07-08 16:44:40 UTC
What remains to be done to get this patch merged?
Comment 39 Felix Riemann 2009-10-17 12:04:14 UTC
Raoul, can you give us your full name (we need it for git).

If there comes up nothing serious with this patch I'd accept for 2.29.1 (Oct 26).
Even if there might be little timing problems with some pictures, it's still better than the current situation.
Comment 40 Raoul Berger 2009-10-17 13:47:51 UTC
Created attachment 145660 [details] [review]
removed pause/resume api

I updated my bugzilla profile, so you can see my full name ;)

Also, I removed pausing / resuming  api from the last patch. It isn't well coded and not very usefull anyway.
Comment 41 Felix Riemann 2009-10-18 15:55:36 UTC
Thanks a lot, Raoul (& all the testers)! It's in git master now as commit a8133caa982f783c615eec8c8aa7eb6b0e5a9a95.

(In reply to comment #40)
> Also, I removed pausing / resuming  api from the last patch. It isn't well
> coded and not very usefull anyway.

Makes reviewing a bit easier. :-)
If the need comes up for it, pausing can be easily revisited later.
---------------------------------------------------------------------------------
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Comment 42 Raoul Berger 2009-10-21 12:42:01 UTC
Good news :-)
Yes, thanks to all the testers who helped making a better patch !
Comment 43 Raoul Berger 2009-10-21 15:07:36 UTC
Hmmm this is bad, there is a memory leak as soon as we try to apply a transformation...
I'm sorry, commit should be reverted. I'll post a fix asap.
Comment 44 Raoul Berger 2009-10-22 01:25:17 UTC
Created attachment 146012 [details] [review]
fix to apply to the commited patch

Félix, here is a fix for the already commited patch.
I forgot to unref a pixbuf in "eog-scroll-view.c"...
Comment 45 Felix Riemann 2009-10-22 17:18:32 UTC
Review of attachment 146012 [details] [review]:

Thanks, committed as commit 53d23233729079bf5e9c00eb9b50d173ab81fb9e.
Comment 46 Felix Riemann 2010-05-01 09:12:27 UTC
*** Bug 617353 has been marked as a duplicate of this bug. ***