Re: [Gegl-developer] Choosing the operation input format based the connected nodes output format

[Dov Grobgeld <dov grobgeld gmail com>]

Based on this discussion I made the following changes to ppm-load.c based on ff-load.c. Is it ok to commit? 

I also changed gaussian blur so that it negotiates the format. Unfortunately I seem to have introduced some bug that broke its functionality that I have yet to solve. But that's for some other night.

diff --git a/operations/external/ppm-load.c b/operations/external/ppm-load.c

index 82041e2..d518eab 100644

--- a/operations/external/ppm-load.c

+++ b/operations/external/ppm-load.c

@@ -54,11 +54,11 @@ typedef struct {

   gsize      channels; 

   gsize      bpc;        /* bytes per channel */

   guchar    *data;

-} pnm_struct;

+} Priv;

 

 static gboolean

 ppm_load_read_header(FILE       *fp,

-                     pnm_struct *img)

+                     Priv       *img)

 {

   /* PPM Headers Variable Declaration */

   gchar *ptr;

@@ -160,8 +160,65 @@ ppm_load_read_header(FILE       *fp,

 }

 

 static void

+init (GeglChantO *o)

+{

+  Priv       *p = (Priv*)o->chant_data;

+

+  if (p==NULL)

+    {

+      p = g_new0 (Priv, 1);

+      o->chant_data = (void*) p;

+    }

+}

+

+static void

+prepare (GeglOperation *operation)

+{

+  GeglChantO *o = GEGL_CHANT_PROPERTIES (operation);

+  Priv *p = (Priv*)o->chant_data;

+  FILE *fp;

+

+  if (p == NULL)

+    {

+      init (o);

+      p = (Priv*)o->chant_data;

+    }

+

+  g_assert (o->chant_data != NULL);

+

+  fp = (!strcmp (o->path, "-") ? stdin : fopen (o->path,"rb") );

+  if (!ppm_load_read_header (fp, p))

+    goto out;

+

+  if (p->bpc == 1)

+    {

+      if (p->channels == 3)

+        gegl_operation_set_format (operation, "output",

+                                   babl_format ("R'G'B' u8"));

+      else

+        gegl_operation_set_format (operation, "output",

+                                   babl_format ("Y u8"));

+    }

+  else if (p->bpc == 2)

+    {

+      if (p->channels == 3)

+        gegl_operation_set_format (operation, "output",

+                                   babl_format ("R'G'B' u16"));

+      else

+        gegl_operation_set_format (operation, "output",

+                                   babl_format ("Y' u16"));

+    }

+  else

+    g_warning ("%s: Programmer stupidity error", G_STRLOC);

+

+ out:

+  if (stdin != fp)

+    fclose (fp);

+}

+

+static void

 ppm_load_read_image(FILE       *fp,

-                    pnm_struct *img)

+                    Priv *img)

 {

   guint   i;

 

@@ -219,44 +276,12 @@ get_bounding_box (GeglOperation *operation)

 {

   GeglChantO   *o = GEGL_CHANT_PROPERTIES (operation);

   GeglRectangle result = {0,0,0,0};

-  pnm_struct    img;

-  FILE         *fp;

-

-  fp = (!strcmp (o->path, "-") ? stdin : fopen (o->path,"rb") );

-

-  if (!fp)

-    return result;

-

-  if (!ppm_load_read_header (fp, &img))

-    goto out;

-

-  if (img.bpc == 1)

-    {

-      if (img.channels == 3)

-        gegl_operation_set_format (operation, "output",

-                                   babl_format ("R'G'B' u8"));

-      else

-        gegl_operation_set_format (operation, "output",

-                                   babl_format ("Y' u8"));

-    }

-  else if (img.bpc == 2)

-    {

-      if (img.channels == 3)

-        gegl_operation_set_format (operation, "output",

-                                   babl_format ("R'G'B' u16"));

-      else

-        gegl_operation_set_format (operation, "output",

-                                   babl_format ("Y' u16"));

-    }

-  else

-    g_warning ("%s: Programmer stupidity error", G_STRLOC);

+  Priv   *p = (Priv*)o->chant_data;

 

-  result.width = img.width;

-  result.height = img.height;

+  g_assert (o->chant_data != NULL);

 

- out:

-  if (stdin != fp)

-    fclose (fp);

+  result.width = p->width;

+  result.height = p->height;

 

   return result;

 }

@@ -269,16 +294,19 @@ process (GeglOperation       *operation,

 {

   GeglChantO   *o = GEGL_CHANT_PROPERTIES (operation);

   FILE         *fp;

-  pnm_struct    img;

   GeglRectangle rect = {0,0,0,0};

   gboolean      ret = FALSE;

+  Priv   *p = (Priv*)o->chant_data;

+  const Babl         *format = gegl_operation_get_format (operation, "output");

+

+  g_assert (o->chant_data != NULL);

 

   fp = (!strcmp (o->path, "-") ? stdin : fopen (o->path,"rb"));

 

   if (!fp)

     return FALSE;

 

-  if (!ppm_load_read_header (fp, &img))

+  if (!ppm_load_read_header (fp, p))

     goto out;

 

   /* Allocating Array Size */

@@ -287,63 +315,24 @@ process (GeglOperation       *operation,

    * error signalled by returning FALSE isn't properly acted upon. Therefore

    * g_malloc() is used here which aborts if the requested memory size can't be

    * allocated causing a controlled crash. */

-  img.data = "" g_malloc (img.numsamples * img.bpc);

+  p->data = "" g_malloc (p->numsamples * p->bpc);

 

   /* No-op without g_try_malloc(), see above. */

-  if (! img.data)

+  if (! p->data)

     {

-      g_warning ("Couldn't allocate %" G_GSIZE_FORMAT " bytes, giving up.", ((gsize)img.numsamples * img.bpc));

+      g_warning ("Couldn't allocate %" G_GSIZE_FORMAT " bytes, giving up.", ((gsize)p->numsamples * p->bpc));

       goto out;

     }

 

-  rect.height = img.height;

-  rect.width = img.width;

+  rect.height = p->height;

+  rect.width = p->width;

 

-  if (img.bpc == 1)

-    {

-      if (img.channels == 3)

-        gegl_buffer_get (output, &rect, 1.0, babl_format ("R'G'B' u8"), img.data,

-                         GEGL_AUTO_ROWSTRIDE, GEGL_ABYSS_NONE);

-      else

-        gegl_buffer_get (output, &rect, 1.0, babl_format ("Y' u8"), img.data,

-                         GEGL_AUTO_ROWSTRIDE, GEGL_ABYSS_NONE);

-    }

-  else if (img.bpc == 2)

-    {

-      if (img.channels == 3)

-        gegl_buffer_get (output, &rect, 1.0, babl_format ("R'G'B' u16"), img.data,

-                         GEGL_AUTO_ROWSTRIDE, GEGL_ABYSS_NONE);

-      else

-        gegl_buffer_get (output, &rect, 1.0, babl_format ("Y' u16"), img.data,

-                         GEGL_AUTO_ROWSTRIDE, GEGL_ABYSS_NONE);

-    }

-  else

-    g_warning ("%s: Programmer stupidity error", G_STRLOC);

-

-  ppm_load_read_image (fp, &img);

+  ppm_load_read_image (fp, p);

 

-  if (img.bpc == 1)

-    {

-      if (img.channels == 3)

-        gegl_buffer_set (output, &rect, 0, babl_format ("R'G'B' u8"), img.data,

-                         GEGL_AUTO_ROWSTRIDE);

-      else

-        gegl_buffer_set (output, &rect, 0, babl_format ("Y' u8"), img.data,

+  gegl_buffer_set (output, &rect, 0, format, p->data,

                    GEGL_AUTO_ROWSTRIDE);

-    }

-  else if (img.bpc == 2)

-    {

-      if (img.channels == 3)

-        gegl_buffer_set (output, &rect, 0, babl_format ("R'G'B' u16"), img.data,

-                         GEGL_AUTO_ROWSTRIDE);

-      else

-        gegl_buffer_set (output, &rect, 0, babl_format ("Y' u16"), img.data,

-                         GEGL_AUTO_ROWSTRIDE);

-    }

-  else

-    g_warning ("%s: Programmer stupidity error", G_STRLOC);

-

-  g_free (img.data);

+  g_free (p->data);

+  p->data = "">

 

   ret = TRUE;

 

@@ -354,6 +343,24 @@ process (GeglOperation       *operation,

   return ret;

 }

 

+static void

+finalize (GObject *object)

+{

+  GeglChantO *o = GEGL_CHANT_PROPERTIES (object);

+

+  if (o->chant_data)

+    {

+      Priv *p = (Priv*)o->chant_data;

+      if (p->data)

+        g_free(p->data);

+

+      g_free (o->chant_data);

+      o->chant_data = NULL;

+    }

+

+  G_OBJECT_CLASS (gegl_chant_parent_class)->finalize (object);

+}

+

 static GeglRectangle

 get_cached_region (GeglOperation       *operation,

                    const GeglRectangle *roi)

@@ -370,9 +377,11 @@ gegl_chant_class_init (GeglChantClass *klass)

   operation_class = GEGL_OPERATION_CLASS (klass);

   source_class    = GEGL_OPERATION_SOURCE_CLASS (klass);

 

+  G_OBJECT_CLASS (klass)->finalize = finalize;

   source_class->process = process;

   operation_class->get_bounding_box = get_bounding_box;

   operation_class->get_cached_region = get_cached_region;

+  operation_class->prepare = prepare;

 

   gegl_operation_class_set_keys (operation_class,

                                  "name"        , "gegl:ppm-load",

On Thu, May 30, 2013 at 6:20 PM, Daniel Sabo <danielsabo gmail com> wrote:

I am in the midst of changing some of the details of this, right now prepare is called much more often than it needs to be. What you can rely on is that changing a property of an attached operation will cause it's node to emit the "invalidated" signal, which forces prepare() to be called on all nodes before the next process().

On Thu, May 30, 2013 at 4:56 AM, Dov Grobgeld <dov grobgeld gmail com> wrote:

I had a look at the operations/external file loaders and it seems that most of them lack format extraction in the prepare() function. Am I correct to assume that the proper place for reading the headers should be in the prepare function? But what I wonder is how this works with respect to setting the "path" after the network has been built? When exactly is prepare() called?

Regards,

Dov

On Thu, May 30, 2013 at 11:02 AM, Dov Grobgeld <dov grobgeld gmail com> wrote:

Thanks. If the behavior of ppm-load is considered buggy, then it all makes sense. I'll use the output format if it is set, or otherwise I will fallback to RGBA within prepare.

Regards,
Dov

On Thu, May 30, 2013 at 10:54 AM, Daniel Sabo <danielsabo gmail com> wrote:

My first instinct would be to say that ppm-load not setting it's output format is a bug, and that it should try to sniff the file in prepare() inorder to set the correct format. If you do format switching in gaussian-blur's process() then you'll also spoil the format detection of whatever is connected to gaussian's output.

As an alternative to that, I would be inclined to process ppm-load to a buffer then pass that buffer to a second graph.

Finally (and please don't use this option), the reason you get that error is because the parent implementation of process() in gegl/operation/gegl-operation-filter.c calls gegl_operation_context_get_target for you, it is possible to override this and still use the rest of the filter class (see operations/common/over.c).

On Wed, May 29, 2013 at 8:58 PM, Dov Grobgeld <dov grobgeld gmail com> wrote:

Hi all,

Yesterday night I started playing around with converting the gaussian-blur operation to only work on the channels that are part of the input pad. My goal was to make the operation "format transparent", in the sense that the output format should be equal to the input format. But after several failures, I realized that I was stuck.

The problem is as follows. I want to set my input format based on the output format of the previous node. I'm building the following network:

gegl:ppm-load → gegl:gaussian-blur → gegl:npy-save

and I feed the ppm-load with a "Y u8" image. Now, the problem is that if I in the gaussian-blur prepare function call: 

gegl_operation_get_source_format(operation, "input")

it returns NULL. Why is that? Am I correct to assume that the prepare calls are done before any processing has taken place? If so the ppm-load still doesn't know what format it will set in its output pad.

So instead I asked about the format in process(). But then I get the following warning:

(test-npy.py:2383): GEGL-gegl-operation-context.c-WARNING **: no format for gegl:gaussian-blur 0x95bb9b0 presuming RGBA float

I still haven't figured out exactly why this happens, but I would still like to know if anyone knows what I'm doing wrong, and whether dynamically choosing the input format based on the previous output format is even possible?

Thanks!

Dov

_______________________________________________
gegl-developer-list mailing list
gegl-developer-list gnome org
https://mail.gnome.org/mailman/listinfo/gegl-developer-list