[shotwell] Remove explicit memcopy in import

[Jens Georg <jensgeorg src gnome org>]

commit 7e03c54c67f4d791d2d94ec870adc436b62a6c41
Author: Jens Georg <mail jensge org>
Date:   Sun Jun 18 08:36:30 2017 +0200

    Remove explicit memcopy in import

 src/camera/GPhoto.vala                 |   88 +++++++++++++++++---------------
 src/camera/ImportPage.vala             |   10 +---
 src/data_imports/DataImportSource.vala |   12 ++---
 src/photos/PhotoMetadata.vala          |   26 ++++-----
 src/util/misc.vala                     |    9 ---
 5 files changed, 63 insertions(+), 82 deletions(-)
---
diff --git a/src/camera/GPhoto.vala b/src/camera/GPhoto.vala
index fb0fd4a..3d327df 100644
--- a/src/camera/GPhoto.vala
+++ b/src/camera/GPhoto.vala
@@ -115,6 +115,22 @@ namespace GPhoto {
         return true;
     }
 
+    public Bytes? camera_file_to_bytes (Context context, CameraFile file) {
+        // if buffer can be loaded into memory, return a Bytes class with
+        // CameraFile being the owner of the data. This way, the CameraFile is freed
+        // when the Bytes are freed
+        unowned uint8 *data;
+        ulong data_len;
+        var res = file.get_data_and_size(out data, out data_len);
+        if (res != Result.OK)
+            return null;
+
+        unowned uint8[] buffer = (uint8[]) data;
+        buffer.length = (int) data_len;
+
+        return Bytes.new_with_owner<GPhoto.CameraFile>(buffer, file);
+    }
+
     // Libgphoto will in some instances refuse to get metadata from a camera, but the camera is accessible 
as a
     // filesystem.  In these cases shotwell can access the file directly. See:
     // http://redmine.yorba.org/issues/2959
@@ -146,9 +162,10 @@ namespace GPhoto {
     }
     
     public Gdk.Pixbuf? load_preview(Context context, Camera camera, string folder, string filename,
-        out uint8[] raw, out size_t raw_length) throws Error {
-        raw = null;
-        raw_length = 0;
+            out string? preview_md5) throws Error {
+        Bytes? raw = null;
+        Bytes? out_bytes = null;
+        preview_md5 = null;
         
         try {
             raw = load_file_into_buffer(context, camera, folder, filename, GPhoto.CameraFileType.PREVIEW);
@@ -158,32 +175,38 @@ namespace GPhoto {
                 return null;
             if(0 == metadata.get_preview_count())
                 return null;
-            PhotoPreview? preview = metadata.get_preview(metadata.get_preview_count() - 1);
+
+            // Get the smallest preview from meta-data
+            var preview = metadata.get_preview (metadata.get_preview_count() - 1);
             raw = preview.flatten();
+            preview_md5 = Checksum.compute_for_bytes(ChecksumType.MD5, raw);
         }
         
-        if (raw == null) {
-            raw_length = 0;
-            return null;
-        }
-        
-        raw_length = raw.length;
-        
         // Try to make sure last two bytes are JPEG footer.
         // This is necessary because GPhoto sometimes includes a few extra bytes. See
         // Yorba bug #2905 and the following GPhoto bug:
         // http://sourceforge.net/tracker/?func=detail&aid=3141521&group_id=8874&atid=108874
+
+        var raw_length = raw.get_size ();
         if (raw_length > 32) {
-            for (size_t i = raw_length - 2; i > raw_length - 32; i--) {
-                if (raw[i] == Jpeg.MARKER_PREFIX && raw[i + 1] == Jpeg.Marker.EOI) {
+            for (var i = raw_length - 2; i > raw_length - 32; i--) {
+                if (raw.get((int) i) == Jpeg.MARKER_PREFIX && raw.get((int) i + 1) == Jpeg.Marker.EOI) {
                     debug("Adjusted length of thumbnail for: %s", filename);
                     raw_length = i + 2;
                     break;
                 }
             }
         }
-        
-        MemoryInputStream mins = new MemoryInputStream.from_data(raw, null);
+
+        if (raw_length != raw.get_size()) {
+            out_bytes = new Bytes.from_bytes (raw, 0, raw_length);
+        } else {
+            out_bytes = raw;
+        }
+        preview_md5 = Checksum.compute_for_bytes(ChecksumType.MD5, out_bytes);
+
+        MemoryInputStream mins = new MemoryInputStream.from_bytes (raw);
+
         return new Gdk.Pixbuf.from_stream_at_scale(mins, ImportPreview.MAX_SCALE, ImportPreview.MAX_SCALE, 
true, null);
     }
     
@@ -221,7 +244,7 @@ namespace GPhoto {
     
     public PhotoMetadata? load_metadata(Context context, Camera camera, string folder, string filename)
         throws Error {
-        uint8[] camera_raw = null;
+        Bytes? camera_raw = null;
         try {
             camera_raw = load_file_into_buffer(context, camera, folder, filename, 
GPhoto.CameraFileType.EXIF);
         } catch {
@@ -254,17 +277,9 @@ namespace GPhoto {
         // if entire file fits in memory, return a stream from that ...
         // The camera_file is set as data on the object to keep it alive while
         // the MemoryInputStream is alive.
-        unowned uint8 *data;
-        ulong data_len;
-        res = camera_file.get_data_and_size(out data, out data_len);
-        if (res == Result.OK) {
-            unowned uint8[] buffer = (uint8[])data;
-            buffer.length = (int) data_len;
-            
-            var mis = new MemoryInputStream.from_data(buffer, () => {});
-            mis.set_data<GPhoto.CameraFile>("camera-file", camera_file);
-
-            return mis;
+        var bytes = camera_file_to_bytes (context, camera_file);
+        if (bytes != null) {
+            return new MemoryInputStream.from_bytes(bytes);
         }
 
         // if not stored in memory, try copying it to a temp file and then reading out of that
@@ -278,30 +293,19 @@ namespace GPhoto {
     }
     
     // Returns a buffer with the requested file, if within reason.  Use load_file for larger files.
-    public uint8[]? load_file_into_buffer(Context context, Camera camera, string folder,
+    public Bytes? load_file_into_buffer(Context context, Camera camera, string folder,
         string filename, CameraFileType filetype) throws Error {
         GPhoto.CameraFile camera_file;
         GPhoto.Result res = GPhoto.CameraFile.create(out camera_file);
         if (res != Result.OK)
             throw new GPhotoError.LIBRARY("[%d] Error allocating camera file: %s", (int) res, 
res.as_string());
-        
+
         res = camera.get_file(folder, filename, filetype, camera_file, context);
         if (res != Result.OK)
             throw new GPhotoError.LIBRARY("[%d] Error retrieving file object for %s/%s: %s", 
                 (int) res, folder, filename, res.as_string());
-        
-        // if buffer can be loaded into memory, return a copy of that (can't return buffer itself
-        // as it will be destroyed when the camera_file is unref'd)
-        unowned uint8 *data;
-        ulong data_len;
-        res = camera_file.get_data_and_size(out data, out data_len);
-        if (res != Result.OK)
-            return null;
-        
-        uint8[] buffer = new uint8[data_len];
-        Memory.copy(buffer, data, buffer.length);
-        
-        return buffer;
+
+        return camera_file_to_bytes (context, camera_file);
     }
 }
 
diff --git a/src/camera/ImportPage.vala b/src/camera/ImportPage.vala
index c5d7934..2360e99 100644
--- a/src/camera/ImportPage.vala
+++ b/src/camera/ImportPage.vala
@@ -1562,9 +1562,8 @@ public class ImportPage : CheckerboardPage {
             // this means the preview orientation will be wrong and the MD5 is not generated
             // if the EXIF did not parse properly (see above)
             
-            uint8[] preview_raw = null;
-            size_t preview_raw_length = 0;
             Gdk.Pixbuf preview = null;
+            string? preview_md5 = null;
             try {
                 string preview_fulldir = fulldir;
                 string preview_filename = filename;
@@ -1573,7 +1572,7 @@ public class ImportPage : CheckerboardPage {
                     preview_filename = associated.get_filename();
                 }
                 preview = GPhoto.load_preview(spin_idle_context.context, camera, preview_fulldir,
-                    preview_filename, out preview_raw, out preview_raw_length);
+                    preview_filename, out preview_md5);
             } catch (Error err) {
                 // only issue the warning message if we're not reading a video. GPhoto is capable
                 // of reading video previews about 50% of the time, so we don't want to put a guard
@@ -1585,11 +1584,6 @@ public class ImportPage : CheckerboardPage {
                 }
             }
             
-            // calculate thumbnail fingerprint
-            string? preview_md5 = null;
-            if (preview != null && preview_raw != null && preview_raw_length > 0)
-                preview_md5 = md5_binary(preview_raw, preview_raw_length);
-            
 #if TRACE_MD5
             debug("camera MD5 %s: exif=%s preview=%s", filename, exif_only_md5, preview_md5);
 #endif
diff --git a/src/data_imports/DataImportSource.vala b/src/data_imports/DataImportSource.vala
index 9d16761..ba00be3 100644
--- a/src/data_imports/DataImportSource.vala
+++ b/src/data_imports/DataImportSource.vala
@@ -58,15 +58,11 @@ public class DataImportSource {
             } else {
                 exposure_time = (metadata != null) ? metadata.get_exposure_date_time() : null;
             }
-            PhotoPreview? preview = metadata != null ? metadata.get_preview(0) : null;
-            if (preview != null) {
-                try {
-                    uint8[] preview_raw = preview.flatten();
-                    preview_md5 = md5_binary(preview_raw, preview_raw.length);
-                } catch(Error e) {
-                    warning("Could not get raw preview for %s: %s", get_filename(), e.message);
-                }
+
+            if (metadata != null) {
+                preview_md5 = metadata.thumbnail_hash();
             }
+
 #if TRACE_MD5
             debug("Photo MD5 %s: preview=%s", get_filename(), preview_md5);
 #endif
diff --git a/src/photos/PhotoMetadata.vala b/src/photos/PhotoMetadata.vala
index e02eb5c..2c2d6c5 100644
--- a/src/photos/PhotoMetadata.vala
+++ b/src/photos/PhotoMetadata.vala
@@ -190,16 +190,16 @@ public abstract class PhotoPreview {
         return extension;
     }
     
-    public abstract uint8[] flatten() throws Error;
+    public abstract Bytes flatten() throws Error;
     
     public virtual Gdk.Pixbuf? get_pixbuf() throws Error {
-        uint8[] flattened = flatten();
+        var flattened = flatten();
         
         // Need to create from stream or file for decode ... catch decode error and return null,
         // different from an I/O error causing the problem
         try {
-            return new Gdk.Pixbuf.from_stream(new MemoryInputStream.from_data(flattened, null),
-                null);
+            return new Gdk.Pixbuf.from_stream(new
+                    MemoryInputStream.from_bytes(flattened));
         } catch (Error err) {
             warning("Unable to decode thumbnail for %s: %s", name, err.message);
             
@@ -236,11 +236,12 @@ public class PhotoMetadata : MediaMetadata {
             this.number = number;
         }
         
-        public override uint8[] flatten() throws Error {
+        public override Bytes flatten() throws Error {
             unowned GExiv2.PreviewProperties?[] props = owner.exiv2.get_preview_properties();
             assert(props != null && props.length > number);
             
-            return owner.exiv2.get_preview_image(props[number]).get_data();
+            return new
+                Bytes(owner.exiv2.get_preview_image(props[number]).get_data());
         }
     }
     
@@ -280,18 +281,13 @@ public class PhotoMetadata : MediaMetadata {
         source_name = "<memory buffer %d bytes>".printf(length);
     }
     
-    public void read_from_app1_segment(uint8[] buffer, int length = 0) throws Error {
-        if (length <= 0)
-            length = buffer.length;
-        
-        assert(buffer.length >= length);
-        
+    public void read_from_app1_segment(Bytes buffer) throws Error {
         exiv2 = new GExiv2.Metadata();
         exif = null;
         
-        exiv2.from_app1_segment(buffer, length);
-        exif = Exif.Data.new_from_data(buffer, length);
-        source_name = "<app1 segment %d bytes>".printf(length);
+        exiv2.from_app1_segment(buffer.get_data(), (long) buffer.get_size());
+        exif = Exif.Data.new_from_data(buffer.get_data(), buffer.get_size());
+        source_name = "<app1 segment %zu bytes>".printf(buffer.get_size());
     }
     
     public static MetadataDomain get_tag_domain(string tag) {
diff --git a/src/util/misc.vala b/src/util/misc.vala
index cbc6dfa..6111ea3 100644
--- a/src/util/misc.vala
+++ b/src/util/misc.vala
@@ -72,15 +72,6 @@ public inline time_t now_time_t() {
     return (time_t) now_sec();
 }
 
-public string md5_binary(uint8 *buffer, size_t length) {
-    assert(length != 0);
-
-    Checksum md5 = new Checksum(ChecksumType.MD5);
-    md5.update((uchar []) buffer, length);
-    
-    return md5.get_string();
-}
-
 public string md5_file(File file) throws Error {
     Checksum md5 = new Checksum(ChecksumType.MD5);
     uint8[] buffer = new uint8[64 * 1024];