GNOME Bugzilla – Bug 335093
Support for animated images
Last modified: 2010-05-02 14:37:42 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
http://librarian.launchpad.net/1785060/movie.gif and http://www.007.org.uk/~vtrouilliez/tmp/ are examples.
Related gnomesupport.org forum topic: http://gnomesupport.org/forums/viewtopic.php?t=12521
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?
*** Bug 540786 has been marked as a duplicate of this bug. ***
Are there plans to implement this yet?
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?
*** Bug 555650 has been marked as a duplicate of this bug. ***
EOG should animate. Play/Pause buttons, etc., are not necessary.
a patch is here : http://raoulito.info/eog_animagifs/
agree with comment # 8
(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?
Created attachment 124012 [details] [review] patch for eog 2.24.1 and 2.24.2 Sorry I should have uploaded the patch on bugzilla :)
Sorry for not being precise. A patch against trunk is what could be most useful.
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))
(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 ?
(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
Created attachment 124204 [details] [review] patch against eog trunk 4902
> 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.
Created attachment 124217 [details] [review] patch against eog trunck 4902 modified : eog_image_is_animation (EogImage *img)
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.
(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 ?
Hmm, I don't know exactly how bugzilla works. Anyway, at least I can mark them as obsolete, doing that.
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.
(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.
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.
(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).
Created attachment 128718 [details] [review] patch against trunck 4997
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.
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).
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.
Created attachment 131091 [details] [review] updated patch, needs work
(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.
(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.
Created attachment 131478 [details] [review] new API, gtk-doc comments, fix interpolation stuff
Created attachment 131623 [details] [review] updated patch added EogImage class member : void (* next_frame) (EogImage *img, gint delay)
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.
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.
What remains to be done to get this patch merged?
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.
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.
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.
Good news :-) Yes, thanks to all the testers who helped making a better patch !
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.
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"...
Review of attachment 146012 [details] [review]: Thanks, committed as commit 53d23233729079bf5e9c00eb9b50d173ab81fb9e.
*** Bug 617353 has been marked as a duplicate of this bug. ***