[gimp] Bug 771012 - Improve WebP animation-saving

[Michael Natterer <mitch src gnome org>]

commit 342c60254697fe67d010e69f13ae09504081ca45
Author: Pascal Massimino <pascal massimino gmail com>
Date:   Wed Sep 7 17:12:11 2016 +0200

    Bug 771012 - Improve WebP animation-saving
    
    WebP saving: various improvements in file-webp-save.c
    
    - reorganizes the main loop
    - fixes (temporarily) the bad timestamp by using fixed 100ms delay between frames
    - fixes the progress bar
    - takes care of resource clean-up for some previously unchecked error paths
    - uses 'minimize_size' and 'allow_mixed' options more appropriately
    - only remuxes the final bytestream when ICC profile is present. For
      most common case, we insert the loop-count information during muxer
      creation instead of by remuxing at the end.

 plug-ins/file-webp/file-webp-save.c |   91 ++++++++++++++++++++++-------------
 1 files changed, 57 insertions(+), 34 deletions(-)
---
diff --git a/plug-ins/file-webp/file-webp-save.c b/plug-ins/file-webp/file-webp-save.c
index 02f8b7f..d07e0ca 100644
--- a/plug-ins/file-webp/file-webp-save.c
+++ b/plug-ins/file-webp/file-webp-save.c
@@ -72,7 +72,7 @@ gboolean      save_animation        (const gchar       *filename,
 WebPPreset
 webp_preset_by_name (gchar *name)
 {
-  if( ! strcmp (name, "picture"))
+  if (! strcmp (name, "picture"))
     {
       return WEBP_PRESET_PICTURE;
     }
@@ -249,8 +249,8 @@ save_layer (const gchar    *filename,
       picture.progress_hook = webp_file_progress;
 
       /* Attempt to allocate a buffer of the appropriate size */
-      buffer = (guchar *) g_malloc (w * h * bpp);
-      if(! buffer)
+      buffer = g_try_malloc (w * h * bpp);
+      if (! buffer)
         break;
 
       /* Read the region into the buffer */
@@ -267,6 +267,8 @@ save_layer (const gchar    *filename,
           WebPPictureImportRGBA (&picture, buffer, w * bpp);
         }
 
+      g_free (buffer);
+
       /* Perform the actual encode */
       if (! WebPEncode (&config, &picture))
         {
@@ -361,9 +363,6 @@ save_layer (const gchar    *filename,
   if (outfile)
     fclose (outfile);
 
-  if (buffer)
-    free (buffer);
-
   WebPPictureFree (&picture);
 
   return status;
@@ -378,7 +377,7 @@ save_animation (const gchar    *filename,
                 WebPSaveParams *params,
                 GError        **error)
 {
-  gboolean               status   = FALSE;
+  gboolean               status   = TRUE;
   FILE                  *outfile  = NULL;
   guchar                *buffer   = NULL;
   gint                   w, h;
@@ -389,15 +388,15 @@ save_animation (const gchar    *filename,
   WebPAnimEncoderOptions enc_options;
   WebPData               webp_data;
   int                    frame_timestamp = 0;
-  WebPAnimEncoder       *enc;
-  WebPMux               *mux;
-  WebPMuxAnimParams      anim_params = {0};
+  WebPAnimEncoder       *enc = NULL;
 
   if (nLayers < 1)
     return FALSE;
 
   gimp_image_undo_freeze (image_ID);
 
+  WebPDataInit (&webp_data);
+
   do
     {
       gint loop;
@@ -413,16 +412,24 @@ save_animation (const gchar    *filename,
                        g_file_error_from_errno (errno),
                        _("Unable to open '%s' for writing"),
                        gimp_filename_to_utf8 (filename));
+          status = FALSE;
           break;
         }
 
-      WebPDataInit (&webp_data);
       if (! WebPAnimEncoderOptionsInit (&enc_options))
         {
           g_printerr ("ERROR: verion mismatch\n");
+          status = FALSE;
           break;
         }
 
+      enc_options.anim_params.loop_count = 0;
+      if (! params->loop)
+        enc_options.anim_params.loop_count = 1;
+
+      enc_options.allow_mixed   = params->lossless ? 0 : 1;
+      enc_options.minimize_size = 1;
+
       for (loop = 0; loop < nLayers; loop++)
         {
           GeglBuffer       *geglbuffer;
@@ -456,13 +463,14 @@ save_animation (const gchar    *filename,
               if (! enc)
                 {
                   g_printerr ("ERROR: enc == null\n");
+                  status = FALSE;
                   break;
                 }
             }
 
           /* Attempt to allocate a buffer of the appropriate size */
-          buffer = (guchar *) g_malloc (w * h * bpp);
-          if(! buffer)
+          buffer = g_try_malloc (w * h * bpp);
+          if (! buffer)
             {
               g_printerr ("Buffer error: 'buffer null'\n");
               status = FALSE;
@@ -475,6 +483,7 @@ save_animation (const gchar    *filename,
                             params->quality);
 
           config.lossless      = params->lossless;
+          config.quality       = params->quality;
           config.method        = 6;  /* better quality */
           config.alpha_quality = params->alpha_quality;
           config.exact         = 1;
@@ -489,7 +498,6 @@ save_animation (const gchar    *filename,
           picture.height        = h;
           picture.custom_ptr    = &mw;
           picture.writer        = WebPMemoryWrite;
-          picture.progress_hook = webp_file_progress;
 
           /* Read the region into the buffer */
           gegl_buffer_get (geglbuffer, &extent, 1.0, format, buffer,
@@ -511,65 +519,80 @@ save_animation (const gchar    *filename,
               g_printerr ("ERROR[%d]: %s\n",
                           picture.error_code,
                           webp_error_string (picture.error_code));
+              status = FALSE;
             }
 
           WebPMemoryWriterClear (&mw);
           WebPPictureFree (&picture);
 
           if (buffer)
-            free (buffer);
+            g_free (buffer);
+
+          if (status == FALSE)
+            break;
 
           /* Flush the drawable and detach */
           gegl_buffer_flush (geglbuffer);
           g_object_unref (geglbuffer);
+
+          gimp_progress_update ((loop + 1.0) / nLayers);
+          frame_timestamp += 100;    /* TODO: should extract the real time stamp from layer */
         }
 
+      if (status == FALSE)
+        break;
+
       WebPAnimEncoderAdd (enc, NULL, frame_timestamp, NULL);
 
       if (! WebPAnimEncoderAssemble (enc, &webp_data))
         {
           g_printerr ("ERROR: %s\n",
                       WebPAnimEncoderGetError (enc));
+          status = FALSE;
+          break;
         }
 
-      /* Set animations parameters */
-      mux = WebPMuxCreate (&webp_data, 1);
-
-      anim_params.loop_count = 0;
-      if (params->loop == FALSE)
-        {
-          anim_params.loop_count = 1;
-        }
-
-      WebPMuxSetAnimationParams (mux, &anim_params);
-
-      /* Save ICC data */
+      /* Create a mux object if profile is present */
       profile = gimp_image_get_color_profile (image_ID);
       if (profile)
         {
+          WebPMux      *mux;
           WebPData      chunk;
           const guint8 *icc_data;
           gsize         icc_data_size;
 
+          mux = WebPMuxCreate (&webp_data, 1);
+          if (mux == NULL)
+            {
+               g_printerr ("ERROR: could not extract muxing object\n");
+               status = FALSE;
+               break;
+            }
+
+          /* Save ICC data */
           icc_data = gimp_color_profile_get_icc_profile (profile, &icc_data_size);
           chunk.bytes = icc_data;
           chunk.size  = icc_data_size;
           WebPMuxSetChunk (mux, "ICCP", &chunk, 1);
           g_object_unref (profile);
-        }
 
-      WebPMuxAssemble (mux, &webp_data);
+          WebPDataClear (&webp_data);
+          if (WebPMuxAssemble (mux, &webp_data) != WEBP_MUX_OK)
+            {
+              g_printerr ("ERROR: could not assemble final bytestream\n");
+              status = FALSE;
+              break;
+            }
+        }
 
       webp_anim_file_writer (outfile, webp_data.bytes, webp_data.size);
-
-      WebPDataClear (&webp_data);
-      WebPAnimEncoderDelete (enc);
-
-      status = TRUE;
     }
   while (0);
 
   /* Free any resources */
+  WebPDataClear (&webp_data);
+  WebPAnimEncoderDelete (enc);
+
   if (outfile)
     fclose (outfile);