Hi,

First of all - thanks for Entangle, great software !

I'm shooting in raw format, and using Entangle 0.5.1 I found a couple of issues. Using LibRaw 0.14.7 and gphoto2-2.5.1 as support libraries.

Patch 0001: Feature

This patch splits the histogram into the 3 color channels, and displays the result with CAIRO_OPERATOR_ADD. I have also added a grid, feedback on that would be great.

See histogram.jpeg for an example.

Patch 0002: Bug fix

This patch fixes the aperture and shutter speed value parsing in the image status bar.

Patch 0003: Bug fix

This patch uses the camera parameter defaults for generating the representation of the image.

It also adds support for loading the embedded image from raw files used for previews.

Patch 0004: Bug fix

This patch makes sure that only supported file extensions are considered when loading the session.

Patch 0005: Bug fix

The patch adds a fallback for thumbnail loading of non-raw files.


My repository is located at https://github.com/jesperpedersen/entangle

Looking forward for any feedback you have, and issues found with my patches.

Best regards,
 Jesper
>From 5e05fc446d006d1e5ef0f9ec1ca2b79f195ec3f9 Mon Sep 17 00:00:00 2001
From: Jesper Pedersen <jesper.peder...@jboss.org>
Date: Wed, 27 Mar 2013 20:43:16 -0400
Subject: [PATCH 1/5] Histogram: Split into color channels

---
 src/frontend/entangle-image-display.c   |   2 +-
 src/frontend/entangle-image-histogram.c | 102 ++++++++++++++++++++++++++++----
 2 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/src/frontend/entangle-image-display.c b/src/frontend/entangle-image-display.c
index cca4c4a..b551075 100644
--- a/src/frontend/entangle-image-display.c
+++ b/src/frontend/entangle-image-display.c
@@ -82,7 +82,7 @@ static void do_entangle_image_display_render_pixmap(EntangleImageDisplay *displa
 
     pw = gdk_pixbuf_get_width(pixbuf);
     ph = gdk_pixbuf_get_height(pixbuf);
-    priv->pixmap = cairo_image_surface_create(CAIRO_FORMAT_RGB24, pw, ph);
+    priv->pixmap = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, pw, ph);
 
     /* Paint the stack of images - the first one
      * is completely opaque. Others are layers
diff --git a/src/frontend/entangle-image-histogram.c b/src/frontend/entangle-image-histogram.c
index 4034bca..6bfaa48 100644
--- a/src/frontend/entangle-image-histogram.c
+++ b/src/frontend/entangle-image-histogram.c
@@ -31,7 +31,9 @@
     (G_TYPE_INSTANCE_GET_PRIVATE((obj), ENTANGLE_TYPE_IMAGE_HISTOGRAM, EntangleImageHistogramPrivate))
 
 struct _EntangleImageHistogramPrivate {
-    double freq[255];
+    double freq_red[255];
+    double freq_green[255];
+    double freq_blue[255];
     gboolean hasFreq;
     gulong imageNotifyID;
     EntangleImage *image;
@@ -56,7 +58,9 @@ static void do_entangle_pixmap_setup(EntangleImageHistogram *histogram)
 
     if (priv->image)
         pixbuf = entangle_image_get_pixbuf(priv->image);
-    memset(priv->freq, 0, sizeof(priv->freq));
+    memset(priv->freq_red, 0, sizeof(priv->freq_red));
+    memset(priv->freq_green, 0, sizeof(priv->freq_green));
+    memset(priv->freq_blue, 0, sizeof(priv->freq_blue));
 
     if (!pixbuf) {
         priv->hasFreq = FALSE;
@@ -73,8 +77,14 @@ static void do_entangle_pixmap_setup(EntangleImageHistogram *histogram)
     for (y = 0 ; y < h ; y++) {
         guchar *pixel = pixels;
         for (x = 0 ; x < w ; x++) {
-            double level = round(pixel[0] + pixel[1] + pixel[2])/3.0;
-            priv->freq[((int)level) & 0xff]++;
+            double level_red = round(pixel[0]);
+            double level_green = round(pixel[1]);
+            double level_blue = round(pixel[2]);
+
+            priv->freq_red[((int)level_red) & 0xff]++;
+            priv->freq_green[((int)level_green) & 0xff]++;
+            priv->freq_blue[((int)level_blue) & 0xff]++;
+
             pixel+=3;
         }
 
@@ -82,7 +92,9 @@ static void do_entangle_pixmap_setup(EntangleImageHistogram *histogram)
     }
 
     for (x = 0 ; x < 255 ; x++) {
-        priv->freq[x] = DOUBLE_EQUAL(priv->freq[x], 0.0) ? 0.0 : log(priv->freq[x]);
+        priv->freq_red[x] = DOUBLE_EQUAL(priv->freq_red[x], 0.0) ? 0.0 : log(priv->freq_red[x]);
+        priv->freq_green[x] = DOUBLE_EQUAL(priv->freq_green[x], 0.0) ? 0.0 : log(priv->freq_green[x]);
+        priv->freq_blue[x] = DOUBLE_EQUAL(priv->freq_blue[x], 0.0) ? 0.0 : log(priv->freq_blue[x]);
     }
 
     priv->hasFreq = TRUE;
@@ -144,6 +156,23 @@ static void entangle_image_histogram_finalize(GObject *object)
 }
 
 
+static void entangle_image_histogram_draw_line(cairo_t *cr, float from_x, float from_y, float to_x, float to_y)
+{
+    cairo_move_to(cr, from_x, from_y);
+    cairo_line_to(cr, to_x,  to_y);
+}
+
+
+static void entangle_image_histogram_draw_grid(cairo_t *cr, const float width, const float height)
+{
+    for (int k = 1; k < 4; k++) {
+        entangle_image_histogram_draw_line(cr, k/(float)4 * width, 0, k/(float)4 * width, height);
+        cairo_stroke(cr);
+        entangle_image_histogram_draw_line(cr, 0, k/(float)4 * height, width, k/(float)4 * height);
+        cairo_stroke(cr);
+    }
+}
+
 static gboolean entangle_image_histogram_draw(GtkWidget *widget, cairo_t *cr)
 {
     g_return_val_if_fail(ENTANGLE_IS_IMAGE_HISTOGRAM(widget), FALSE);
@@ -157,33 +186,85 @@ static gboolean entangle_image_histogram_draw(GtkWidget *widget, cairo_t *cr)
     ww = gdk_window_get_width(gtk_widget_get_window(widget));
     wh = gdk_window_get_height(gtk_widget_get_window(widget));
 
+    cairo_save(cr);
+
     /* We need to fill the background first */
-    cairo_set_source_rgb(cr, 0.0, 0.0, 0.0);
+    cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 1.0);
     cairo_rectangle(cr, 0, 0, ww, wh);
     cairo_fill(cr);
 
+    /* Draw a grid */
+    cairo_save(cr);
+    cairo_set_line_width(cr, 0.4);
+    cairo_set_source_rgba(cr, 0.4, 0.4, 0.4, 0.2);
+    entangle_image_histogram_draw_grid(cr, ww, wh);
+    cairo_restore(cr); 
+
     if (priv->hasFreq) {
         for (idx = 0 ; idx < 255 ; idx++) {
-            if (priv->freq[idx] > peak)
-                peak = priv->freq[idx];
+            if (priv->freq_red[idx] > peak)
+                peak = priv->freq_red[idx];
+            if (priv->freq_green[idx] > peak)
+                peak = priv->freq_green[idx];
+            if (priv->freq_blue[idx] > peak)
+                peak = priv->freq_blue[idx];
         }
         cairo_set_line_width(cr, 3);
         cairo_set_line_cap(cr, CAIRO_LINE_CAP_ROUND);
-        cairo_set_source_rgba(cr, 0.3, 0.3, 0.3, 1.0);
+        cairo_set_operator(cr, CAIRO_OPERATOR_ADD);
         /* Draw the actual histogram */
+
+        /* Red channel */
+        cairo_save(cr);
+        cairo_set_source_rgba(cr, 1.0, 0.0, 0.0, 0.7);
         cairo_move_to(cr, 0, wh);
 
         for (idx = 0 ; idx < 255 ; idx++) {
             double x = (double)ww * (double)idx / 255.0;
-            double y = (double)(wh - 2) * (double)priv->freq[idx] / peak;
+            double y = (double)(wh - 2) * (double)priv->freq_red[idx] / peak;
 
             cairo_line_to(cr, x, wh - y);
         }
         cairo_line_to(cr, ww, wh);
         cairo_line_to(cr, 0, wh);
         cairo_fill(cr);
+        cairo_restore(cr);
+
+        /* Green channel */
+        cairo_save(cr);
+        cairo_set_source_rgba(cr, 0.0, 1.0, 0.0, 0.7);
+        cairo_move_to(cr, 0, wh);
+
+        for (idx = 0 ; idx < 255 ; idx++) {
+            double x = (double)ww * (double)idx / 255.0;
+            double y = (double)(wh - 2) * (double)priv->freq_green[idx] / peak;
+
+            cairo_line_to(cr, x, wh - y);
+        }
+        cairo_line_to(cr, ww, wh);
+        cairo_line_to(cr, 0, wh);
+        cairo_fill(cr);
+        cairo_restore(cr);
+
+        /* Blue channel */
+        cairo_save(cr);
+        cairo_set_source_rgba(cr, 0.0, 0.0, 1.0, 0.7);
+        cairo_move_to(cr, 0, wh);
+
+        for (idx = 0 ; idx < 255 ; idx++) {
+            double x = (double)ww * (double)idx / 255.0;
+            double y = (double)(wh - 2) * (double)priv->freq_blue[idx] / peak;
+
+            cairo_line_to(cr, x, wh - y);
+        }
+        cairo_line_to(cr, ww, wh);
+        cairo_line_to(cr, 0, wh);
+        cairo_fill(cr);
+        cairo_restore(cr);
     }
 
+    cairo_restore(cr);
+
     return TRUE;
 }
 
@@ -305,7 +386,6 @@ EntangleImage *entangle_image_histogram_get_image(EntangleImageHistogram *histog
     return priv->image;
 }
 
-
 /*
  * Local variables:
  *  c-indent-level: 4
-- 
1.8.1.4

>From ea45a8343f690e89d10c1ec57ba743b6d9b55bc9 Mon Sep 17 00:00:00 2001
From: Jesper Pedersen <jesper.peder...@jboss.org>
Date: Wed, 27 Mar 2013 21:00:46 -0400
Subject: [PATCH 2/5] Fix shutter speed and aperture values

---
 src/frontend/entangle-image-statusbar.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/frontend/entangle-image-statusbar.c b/src/frontend/entangle-image-statusbar.c
index 76aa2a5..7139b19 100644
--- a/src/frontend/entangle-image-statusbar.c
+++ b/src/frontend/entangle-image-statusbar.c
@@ -191,23 +191,27 @@ static void entangle_image_statusbar_update_labels(EntangleImageStatusbar *statu
 
     if (metadata) {
         gint nom, den;
-        glong fnum;
+        gint fnumn, fnumd;
+        gdouble fnum;
         guint isonum;
         gdouble focalnum;
 
         gexiv2_metadata_get_exposure_time(metadata, &nom, &den);
         if (den == 10)
-            shutter = g_strdup_printf("%0.0lf secs", (double)nom/10.0);
+            shutter = g_strdup_printf("%0.1lf secs", (double)nom/10.0);
         else if (nom == 10)
             shutter = g_strdup_printf("1/%0.0lf secs", (double)den/10.0);
         else
-            shutter = g_strdup_printf("%0.0lf/%0.0lf secs", (double)nom/10.0, (double)den/10.0);
+            shutter = g_strdup_printf("%d/%d secs", nom, den);
 
-        fnum = gexiv2_metadata_get_exif_tag_long(metadata, "Exif.Photo.FNumber");
-        if (!fnum)
-            fnum = gexiv2_metadata_get_exif_tag_long(metadata, "Exit.Photo.Aperture");
-        //fnum = gexiv2_metadata_get_fnumber(metadata);
-        aperture = g_strdup_printf("f/%ld", fnum);
+        if (!gexiv2_metadata_get_exif_tag_rational(metadata, "Exif.Photo.FNumber", &fnumn, &fnumd))
+            gexiv2_metadata_get_exif_tag_rational(metadata, "Exif.Photo.Aperture", &fnumn, &fnumd);
+
+        fnum = (double)fnumn/(double)fnumd;
+        if (fnum < 10.0)
+            aperture = g_strdup_printf("f/%1.1f", fnum);
+        else
+            aperture = g_strdup_printf("f/%2.0f", fnum);
 
         isonum = gexiv2_metadata_get_iso_speed(metadata);
         iso = g_strdup_printf("ISO %d", isonum);
-- 
1.8.1.4

>From a3c64f65ef7f596255fdc3733ff3aeb54f249df2 Mon Sep 17 00:00:00 2001
From: Jesper Pedersen <jesper.peder...@jboss.org>
Date: Wed, 27 Mar 2013 21:04:36 -0400
Subject: [PATCH 3/5] Compare with lowercase filenames, use default camera
 parameter for raw images, and use embedded JPG for preview

---
 src/backend/entangle-pixbuf.c | 98 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 91 insertions(+), 7 deletions(-)

diff --git a/src/backend/entangle-pixbuf.c b/src/backend/entangle-pixbuf.c
index c29f098..34f51de 100644
--- a/src/backend/entangle-pixbuf.c
+++ b/src/backend/entangle-pixbuf.c
@@ -102,7 +102,7 @@ static gboolean entangle_pixbuf_is_raw(EntangleImage *image)
         ".crw", ".erf", ".mrw", ".raw", ".rw2", ".raf", NULL
     };
     const char **tmp;
-    const char *filename = entangle_image_get_filename(image);
+    const char *filename = g_utf8_strdown(entangle_image_get_filename(image), -1);
 
     tmp = extlist;
     while (*tmp) {
@@ -137,6 +137,17 @@ static GdkPixbuf *entangle_pixbuf_open_image_master_raw(EntangleImage *image)
         goto cleanup;
     }
 
+    /*
+      Use default camera parameters to display image
+      Real work needs to done by a RAW developer anyway
+    */
+    raw->params.use_camera_wb = 1;
+    raw->params.use_auto_wb = 1;
+    raw->params.use_camera_matrix = 1;
+    raw->params.document_mode = 0;
+    raw->params.user_qual = 0;
+    raw->params.fbdd_noiserd = 1;
+
     ENTANGLE_DEBUG("Open raw %s", entangle_image_get_filename(image));
     if ((ret = libraw_open_file(raw, entangle_image_get_filename(image))) != 0) {
         ENTANGLE_DEBUG("Failed to open raw file: %s",
@@ -174,7 +185,12 @@ static GdkPixbuf *entangle_pixbuf_open_image_master_raw(EntangleImage *image)
                                       img_free, img);
 
  cleanup:
-    libraw_close(raw);
+    if (raw)
+        libraw_recycle(raw);
+
+    if (raw)
+        libraw_close(raw);
+
     return result;
 }
 
@@ -256,6 +272,66 @@ entangle_pixbuf_get_closest_preview(GExiv2PreviewProperties **proplist,
 }
 
 
+static GdkPixbuf *entangle_pixbuf_open_image_preview_raw(EntangleImage *image)
+{
+    GdkPixbuf *result = NULL;
+    GdkPixbufLoader *loader = gdk_pixbuf_loader_new();
+    libraw_data_t *raw = libraw_init(0);
+    libraw_processed_image_t *img = NULL;
+    int ret;
+
+    if (!raw) {
+        ENTANGLE_DEBUG("Failed to initialize libraw");
+        goto cleanup;
+    }
+
+    ENTANGLE_DEBUG("Open preview raw %s", entangle_image_get_filename(image));
+    if ((ret = libraw_open_file(raw, entangle_image_get_filename(image))) != 0) {
+        ENTANGLE_DEBUG("Failed to open preview raw file: %s",
+                       libraw_strerror(ret));
+        goto cleanup;
+    }
+
+    ENTANGLE_DEBUG("Unpack preview raw %s", entangle_image_get_filename(image));
+    if ((ret = libraw_unpack_thumb(raw)) != 0) {
+        ENTANGLE_DEBUG("Failed to unpack preview raw file: %s",
+                       libraw_strerror(ret));
+        goto cleanup;
+    }
+
+    ENTANGLE_DEBUG("Adjust preview raw %s", entangle_image_get_filename(image));
+    if ((ret = libraw_adjust_sizes_info_only(raw)) != 0) {
+        ENTANGLE_DEBUG("Failed to adjust preview raw file: %s",
+                       libraw_strerror(ret));
+        goto cleanup;
+    }
+
+    ENTANGLE_DEBUG("Make preview mem %s", entangle_image_get_filename(image));
+    if ((img = libraw_dcraw_make_mem_thumb(raw, &ret)) == NULL) {
+        ENTANGLE_DEBUG("Failed to extract preview raw file: %s",
+                       libraw_strerror(ret));
+        goto cleanup;
+    }
+
+    gdk_pixbuf_loader_write(loader, img->data, img->data_size, NULL);
+    result = gdk_pixbuf_loader_get_pixbuf(loader);
+
+ cleanup:
+    if (img)
+        libraw_dcraw_clear_mem(img);
+
+    if (raw)
+        libraw_recycle(raw);
+
+    if (raw)
+        libraw_close(raw);
+
+    gdk_pixbuf_loader_close(loader, NULL);
+
+    return result;
+}
+
+
 static GdkPixbuf *entangle_pixbuf_open_image_preview_exiv(EntangleImage *image,
                                                           guint minSize)
 {
@@ -331,7 +407,9 @@ static GdkPixbuf *entangle_pixbuf_open_image_preview_exiv(EntangleImage *image,
 static GdkPixbuf *entangle_pixbuf_open_image_preview(EntangleImage *image)
 {
     if (entangle_pixbuf_is_raw(image)) {
-        GdkPixbuf *result = entangle_pixbuf_open_image_preview_exiv(image, 256);
+        GdkPixbuf *result = entangle_pixbuf_open_image_preview_raw(image);
+        if (!result)
+            result = entangle_pixbuf_open_image_preview_exiv(image, 256);
         if (!result)
             result = entangle_pixbuf_open_image_master_raw(image);
         return result;
@@ -343,10 +421,16 @@ static GdkPixbuf *entangle_pixbuf_open_image_preview(EntangleImage *image)
 
 static GdkPixbuf *entangle_pixbuf_open_image_thumbnail(EntangleImage *image)
 {
-    GdkPixbuf *result = entangle_pixbuf_open_image_preview_exiv(image, 128);
-    if (!result)
-        result = entangle_pixbuf_open_image_master(image);
-    return result;
+    if (entangle_pixbuf_is_raw(image)) {
+        GdkPixbuf *result = entangle_pixbuf_open_image_preview_raw(image);
+        if (!result)
+            result = entangle_pixbuf_open_image_preview_exiv(image, 128);
+        if (!result)
+            result = entangle_pixbuf_open_image_master(image);
+        return result;
+    } else {
+        return entangle_pixbuf_open_image_preview_exiv(image, 128);
+    }
 }
 
 
-- 
1.8.1.4

>From 7869a6ff0eeafd869f63eec4b5600bba2d64972f Mon Sep 17 00:00:00 2001
From: Jesper Pedersen <jesper.peder...@jboss.org>
Date: Fri, 29 Mar 2013 08:45:00 -0400
Subject: [PATCH 4/5] Only import known image extension in the session

---
 src/backend/entangle-session.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/backend/entangle-session.c b/src/backend/entangle-session.c
index fb34612..fb122c3 100644
--- a/src/backend/entangle-session.c
+++ b/src/backend/entangle-session.c
@@ -461,6 +461,30 @@ void entangle_session_remove(EntangleSession *session, EntangleImage *image)
 }
 
 
+static gboolean entangle_session_image_supported(const gchar *name)
+{
+    const char *extlist[] = {
+        ".cr2", ".nef", ".nrw", ".arw", ".orf", ".dng", ".pef",
+        ".crw", ".erf", ".mrw", ".raw", ".rw2", ".raf", ".jpg",
+        ".jpeg", ".png", ".tif", ".tiff", NULL
+    };
+    const char **tmp;
+    const char *filename = g_utf8_strdown(name, -1);
+
+    tmp = extlist;
+    while (*tmp) {
+        const char *ext = *tmp;
+
+        if (g_str_has_suffix(filename, ext))
+            return TRUE;
+
+        tmp++;
+    }
+
+    return FALSE;
+}
+
+
 gboolean entangle_session_load(EntangleSession *session)
 {
     g_return_val_if_fail(ENTANGLE_IS_SESSION(session), FALSE);
@@ -479,12 +503,14 @@ gboolean entangle_session_load(EntangleSession *session)
         if (g_file_info_get_file_type(childinfo) == G_FILE_TYPE_REGULAR ||
             g_file_info_get_file_type(childinfo) == G_FILE_TYPE_SYMBOLIC_LINK) {
 
-            EntangleImage *image = entangle_image_new_file(g_file_get_path(child));
+            if (entangle_session_image_supported(thisname)) {
+                EntangleImage *image = entangle_image_new_file(g_file_get_path(child));
 
-            ENTANGLE_DEBUG("Adding '%s'", g_file_get_path(child));
-            entangle_session_add(session, image);
+                ENTANGLE_DEBUG("Adding '%s'", g_file_get_path(child));
+                entangle_session_add(session, image);
 
-            g_object_unref(image);
+                g_object_unref(image);
+            }
         }
         g_object_unref(child);
         g_object_unref(childinfo);
-- 
1.8.1.4

>From a17958e32a9e227d2d1f5c989973f0fb887a524c Mon Sep 17 00:00:00 2001
From: Jesper Pedersen <jesper.peder...@jboss.org>
Date: Fri, 29 Mar 2013 09:08:35 -0400
Subject: [PATCH 5/5] Add fallback for thumbnail loading

---
 src/backend/entangle-pixbuf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/backend/entangle-pixbuf.c b/src/backend/entangle-pixbuf.c
index 34f51de..ebf1023 100644
--- a/src/backend/entangle-pixbuf.c
+++ b/src/backend/entangle-pixbuf.c
@@ -429,7 +429,10 @@ static GdkPixbuf *entangle_pixbuf_open_image_thumbnail(EntangleImage *image)
             result = entangle_pixbuf_open_image_master(image);
         return result;
     } else {
-        return entangle_pixbuf_open_image_preview_exiv(image, 128);
+        GdkPixbuf *result = entangle_pixbuf_open_image_preview_exiv(image, 128);
+        if (!result)
+            result = entangle_pixbuf_open_image_master(image);
+        return result;
     }
 }
 
-- 
1.8.1.4

<<attachment: histogram.jpeg>>

_______________________________________________
Entangle-devel mailing list
Entangle-devel@gna.org
https://mail.gna.org/listinfo/entangle-devel

Reply via email to