Choosing the operation input format based the connected nodes output format
This discussion is connected to the gegl-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.
This is a read-only list on gimpusers.com so this discussion thread is read-only, too.
Choosing the operation input format based the connected nodes output format | Dov Grobgeld | 30 May 03:58 |
Choosing the operation input format based the connected nodes output format | Daniel Sabo | 30 May 07:54 |
Choosing the operation input format based the connected nodes output format | Dov Grobgeld | 30 May 08:02 |
CA++fsGH7zL9fdRSVD4qqwDpF0m... | 04 Jun 07:17 | |
CAPuJpopJtzOQHwbc9yYXQ9onLq... | 04 Jun 07:17 | |
Choosing the operation input format based the connected nodes output format | Dov Grobgeld | 03 Jun 19:54 |
Choosing the operation input format based the connected nodes output format | Daniel Sabo | 04 Jun 01:31 |
Choosing the operation input format based the connected nodes output format | Dov Grobgeld | 04 Jun 05:23 |
Choosing the operation input format based the connected nodes output format | Daniel Sabo | 04 Jun 07:17 |
Choosing the operation input format based the connected nodes output format | Daniel Sabo | 05 Jun 08:17 |
Choosing the operation input format based the connected nodes output format
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
Choosing the operation input format based the connected nodes output format
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 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
Choosing the operation input format based the connected nodes output format
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 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 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
Choosing the operation input format based the connected nodes output format
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 = (guchar*) g_malloc (img.numsamples * img.bpc);
+ p->data = (guchar*) 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 = NULL;
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 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 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,
DovOn Thu, May 30, 2013 at 11:02 AM, Dov Grobgeld 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 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 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
Choosing the operation input format based the connected nodes output format
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.
Thanks, - Daniel
On Mon, Jun 3, 2013 at 12:54 PM, Dov Grobgeld 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.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 = (guchar*) g_malloc (img.numsamples * img.bpc); + p->data = (guchar*) 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 = NULL;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 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 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,
DovOn Thu, May 30, 2013 at 11:02 AM, Dov Grobgeld 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 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 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
Choosing the operation input format based the connected nodes output format
Hi Daniel,
Remember that the whole point of my changes was to set the output format during prepare, as you suggested in an earlier email. Even if the call to get_bounding_box() was done before the final process() (there is no prepare() callback in the current version), it would not fulfill the setting of the output pad format.
Regards, Dov
On Tue, Jun 4, 2013 at 4:31 AM, Daniel Sabo wrote:
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.
Thanks, - Daniel
On Mon, Jun 3, 2013 at 12:54 PM, Dov Grobgeld 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.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 = (guchar*) g_malloc (img.numsamples * img.bpc); + p->data = (guchar*) 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 = NULL;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 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 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,
DovOn Thu, May 30, 2013 at 11:02 AM, Dov Grobgeld 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 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
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
Choosing the operation input format based the connected nodes output format
It calls gegl_operation_set_format in get_bounding_box. That setting is sticky until set_format is called again, so calling get boudning box should have been enough to set the output format. It's the same logic in png-load which I know works.
Choosing the operation input format based the connected nodes output format
This appears to work now in master:
Python 2.7.4 (default, Apr 19 2013, 18:32:33)
[GCC 4.7.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
from gi.repository import Gegl; Gegl.init(0, "") ptn = Gegl.Node()
pnm_node = ptn.create_child("gegl:ppm-load") pgm_node = ptn.create_child("gegl:ppm-load") pnm_node.set_property("path", "test.pnm") pgm_node.set_property("path", "test.pgm") Gegl.graph_dump_outputs(pnm_node)
gegl:ppm-load 0x9860090: output=R'G'B' u8 bounds: 0, 0, 100×100
Gegl.graph_dump_outputs(pgm_node)
gegl:ppm-load 0x98600f8: output=Y' u8 bounds: 0, 0, 100×100