Looking at this now I'm confused about why it didn't already work, while setting the format in get_bounding_box isn't quite right it should always have gotten called at some point before the final prepare.
Hold on to this patch for a couple days. I'm going to make a branch for some major changes to the processing code soon and I'd rather do this after so we don't end up fixing things twice.On Mon, Jun 3, 2013 at 12:54 PM, Dov Grobgeld <dov.grobgeld@xxxxxxxxx> wrote:
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.cindex 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 gbooleanppm_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 voidppm_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 GeglRectangleget_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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> 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,
DovOn Thu, May 30, 2013 at 10:54 AM, Daniel Sabo <danielsabo@xxxxxxxxx> 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@xxxxxxxxx> 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-saveand 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@xxxxxxxxx
https://mail.gnome.org/mailman/listinfo/gegl-developer-list
_______________________________________________ gegl-developer-list mailing list gegl-developer-list@xxxxxxxxx https://mail.gnome.org/mailman/listinfo/gegl-developer-list
