Unify loading of image data and header values

This moves the entry point for loading MsgPack data to a new function,
image_read_data_block.  Inside image.c, the image structure can be
created using exactly the same functions as for data files.

4 files changed, 280 insertions, 206 deletions

diff --git a/libcrystfel/src/image-msgpack.c b/libcrystfel/src/image-msgpack.c
index 2c9947a9..f7a4d349 100644
--- a/libcrystfel/src/image-msgpack.c
+++ b/libcrystfel/src/image-msgpack.c
@@ -150,163 +150,111 @@ ImageFeatureList *image_msgpack_read_peaks(const DataTemplate *dtempl,
 }
 
 
-static int unpack_slab(struct image *image,
-                       const DataTemplate *dtempl,
-                       double *data,
-                       int data_width, int data_height)
+double image_msgpack_get_value(const char *name,
+                               void *data_block,
+                               size_t data_block_size,
+                               char *ptype)
 {
-	int pi;
-
-	image->dp = malloc(dtempl->n_panels*sizeof(float *));
-	if ( image->dp == NULL ) {
-		ERROR("Failed to allocate data arrays.\n");
-		return 1;
-	}
-
-	for ( pi=0; pi<dtempl->n_panels; pi++ ) {
-
-		struct panel_template *p;
-		int fs, ss;
-		int p_w, p_h;
-
-		p = &dtempl->panels[pi];
-		p_w = p->orig_max_fs - p->orig_min_fs + 1;
-		p_h = p->orig_max_ss - p->orig_min_ss + 1;
-
-		image->dp[pi] = malloc(p_w*p_h*sizeof(float));
-		if ( image->dp[pi] == NULL ) {
-			ERROR("Failed to allocate panel\n");
-			return 1;
-		}
-
-		if ( (p->orig_min_fs + p_w > data_width)
-		  || (p->orig_min_ss + p_h > data_height) )
-		{
-			ERROR("Panel %s is outside range of data provided\n",
-			      p->name);
-			return 1;
-		}
-
-		for ( ss=0; ss<p_h; ss++) {
-		for ( fs=0; fs<p_w; fs++) {
-
-			int idx;
-			int cfs, css;
-
-			cfs = fs+p->orig_min_fs;
-			css = ss+p->orig_min_ss;
-			idx = cfs + css*data_width;
+	*ptype = 'f';
+	return NAN;
+}
 
-			image->dp[pi][fs+p_w*ss] = data[idx];
 
-		}
-		}
+static int load_msgpack_data(struct panel_template *p,
+                             msgpack_object *map_obj,
+                             float **data)
+{
+	msgpack_object *obj;
+	msgpack_object *data_obj;
 
+	obj = find_msgpack_kv(map_obj, p->data);
+	if ( obj == NULL ) {
+		ERROR("Couldn't find '%s' in MessagePack object\n",
+		      p->data);
+		return 1;
 	}
 
-	return 0;
-}
-
-
-static double *find_msgpack_data(msgpack_object *obj, int *width, int *height)
-{
-	FILE *fh = fopen("msgpack.data", "a");
-	fprintf(fh, "object %p:\n", obj);
-	msgpack_object_print(fh, *obj);
-	fprintf(fh, "\n\n\n");
-	fclose(fh);
-
-	#if 0
-	printf("Data type: %i\n", obj->type);
-	if ( obj->type == MSGPACK_OBJECT_POSITIVE_INTEGER ) {
-		printf("got an integer: %li\n", obj->via.i64);
+	if ( obj->type != MSGPACK_OBJECT_MAP ) {
+		ERROR("MessagePack object '%s' is not a map\n", p->data);
+		return 1;
 	}
 
-	if ( obj->type == MSGPACK_OBJECT_ARRAY ) {
+	//type = get_msgpack_kv_string(obj, "type");
+	//printf("data type is '%s'\n", type);
 
-		int i;
-		printf("Array %i items\n", obj->via.array.size);
+	//get_msgpack_kv_tuple(obj, "shape", &w, &h);
 
-		for ( i=0; i<obj->via.array.size; i++ ) {
-			msgpack_object *obj2 = obj->via.array.ptr[i];
-			printf("Item %i: type %i\n", i, obj2->type);
-			if ( obj2->type == MSGPACK_OBJECT_MAP ) {
-				printf("Map: '%s' -> ");
-			}
-		}
-	}
-	#endif
+	//data_obj = find_msgpack_kv(obj, "data");
 
-	*width = 2068;
-	*height = 2162;
-	return NULL;
+	return 0;
 }
 
 
-/* Unpacks the raw panel data from a msgpack_object, applies panel geometry,
- * and stores the resulting data in an image struct. Object has structure
- * {
- * "corr_data":
- *   {
- *     "data": binary_data,
- *     "shape": [data_height, data_width],
- *           ...
- *           ...
- *   },
- *   "key2": val2,
- *        ...
- *        ...
- * }
- */
-struct image *image_msgpack_read(DataTemplate *dtempl,
-                                 void *data,
-                                 size_t data_size,
-                                 int no_image_data,
-                                 int no_mask_data)
+/* Read the image data from 'data_block' into 'image', according to 'dtempl' */
+int image_msgpack_read(struct image *image,
+                       DataTemplate *dtempl,
+                       void *data_block,
+                       size_t data_block_size)
 {
-	struct image *image;
-	int data_width, data_height;
-	double *image_data;
 	msgpack_unpacked unpacked;
 	int r;
+	int n_obj;
+	int i;
+	msgpack_object *the_obj;
 
-	if ( data == NULL ) {
+	if ( image->data_block == NULL ) {
 		ERROR("No MessagePack object!\n");
-		return NULL;
+		return 1;
 	}
 
 	if ( dtempl == NULL ) {
 		ERROR("NULL data template!\n");
-		return NULL;
+		return 1;
 	}
 
 	msgpack_unpacked_init(&unpacked);
-	r = msgpack_unpack_next(&unpacked, data, data_size, NULL);
+	r = msgpack_unpack_next(&unpacked, data_block, data_block_size, NULL);
 	if ( r != MSGPACK_UNPACK_SUCCESS ) {
-		ERROR("Msgpack unpack failed: %i\n", r);
-		return NULL;
+		ERROR("MessagePack unpack failed: %i\n", r);
+		return 1;
 	}
 
-	image = image_new();
-	if ( image == NULL ) {
-		ERROR("Couldn't allocate image structure.\n");
-		return NULL;
+	if ( unpacked.data.type != MSGPACK_OBJECT_ARRAY ) {
+		ERROR("MessagePack data isn't an array - ignoring.\n");
+		msgpack_unpacked_destroy(&unpacked);
+		return 1;
 	}
 
-	if ( !no_image_data ) {
-		image_data = find_msgpack_data(&unpacked.data,
-		                               &data_width, &data_height);
-		if ( image_data == NULL ) {
-			ERROR("No image data in MessagePack object.\n");
-			return NULL;
-		}
-		unpack_slab(image, dtempl, image_data,
-		            data_width, data_height);
-	} else {
-		image_set_zero_data(image, dtempl);
+	n_obj = unpacked.data.via.array.size;
+	if ( n_obj < 1 ) {
+		ERROR("No array elements in MessagePack object?\n");
+		msgpack_unpacked_destroy(&unpacked);
+		return 1;
+	}
+	if ( n_obj > 1 ) {
+		ERROR("WARNING: Too many items in MessagePack object (%i)\n",
+		      n_obj);
+		/* Can continue */
+	}
+
+	the_obj = &unpacked.data.via.array.ptr[0];
+
+	image->dp = malloc(dtempl->n_panels*sizeof(float *));
+	if ( image->dp == NULL ) {
+		ERROR("Failed to allocate data array.\n");
+		return 1;
 	}
 
-	image_set_zero_mask(image, dtempl);
+	/* Set all pointers to NULL for easier clean-up */
+	for ( i=0; i<dtempl->n_panels; i++ ) image->dp[i] = NULL;
 
-	return image;
+	for ( i=0; i<dtempl->n_panels; i++ ) {
+		if ( load_msgpack_data(&dtempl->panels[i], the_obj, &image->dp[i]) )
+		{
+			ERROR("Failed to load panel data\n");
+			return 1;
+		}
+	}
+
+	return 0;
 }
diff --git a/libcrystfel/src/image-msgpack.h b/libcrystfel/src/image-msgpack.h
index 06fcffec..ad05d8fe 100644
--- a/libcrystfel/src/image-msgpack.h
+++ b/libcrystfel/src/image-msgpack.h
@@ -33,24 +33,29 @@
 
 #if defined(HAVE_MSGPACK)
 
-extern struct image *image_msgpack_read(DataTemplate *dtempl,
-                                        void *data,
-                                        size_t data_size,
-                                        int no_image_data);
+extern int image_msgpack_read(struct image *image,
+                              const DataTemplate *dtempl,
+                              void *data,
+                              size_t data_size);
 
 extern ImageFeatureList *image_msgpack_read_peaks(const DataTemplate *dtempl,
                                                   void *data,
                                                   size_t data_size,
                                                   int half_pixel_shift);
 
+extern double image_msgpack_get_value(const char *name,
+                                      void *data_block,
+                                      size_t data_block_size,
+                                      char *ptype);
+
 #else /* defined(HAVE_MSGPACK) */
 
-static UNUSED struct image *image_msgpack_read(DataTemplate *dtempl,
-                                               void *data,
-                                               size_t data_size,
-                                               int no_image_data)
+static UNUSED int image_msgpack_read(struct image *image,
+                                     const DataTemplate *dtempl,
+                                     void *data,
+                                     size_t data_size)
 {
-	return NULL;
+	return 0;
 }
 
 static UNUSED ImageFeatureList *image_msgpack_read_peaks(const DataTemplate *dtempl,
@@ -61,6 +66,17 @@ static UNUSED ImageFeatureList *image_msgpack_read_peaks(const DataTemplate *dte
 	return NULL;
 }
 
+static UNUSED double image_msgpack_get_value(const char *name,
+                                             void *data_block,
+                                             size_t data_block_size,
+                                             char *ptype)
+{
+	*ptype = 'f';
+	return NAN;
+}
+
+
+
 #endif /* defined(HAVE_MSGPACK) */
 
 #endif	/* IMAGE_MSGPACK_H */
diff --git a/libcrystfel/src/image.c b/libcrystfel/src/image.c
index d5029a1c..f41452bb 100644
--- a/libcrystfel/src/image.c
+++ b/libcrystfel/src/image.c
@@ -43,6 +43,7 @@
 #include "detgeom.h"
 #include "image-hdf5.h"
 #include "image-cbf.h"
+#include "image-msgpack.h"
 
 #include "datatemplate.h"
 #include "datatemplate_priv.h"
@@ -375,21 +376,33 @@ static double get_value(struct image *image, const char *from,
 			return NAN;
 		}
 
-	} else if ( is_hdf5_file(image->filename) ) {
-		val = image_hdf5_get_value(from, image->filename, image->ev,
-		                           &type);
+	} else {
 
-	} else if ( is_cbf_file(image->filename) ) {
-		/* FIXME: From headers */
-		return NAN;
+		switch ( image->data_source_type ) {
 
-	} else if ( is_cbfgz_file(image->filename) ) {
-		/* FIXME: From headers */
-		return NAN;
+			case DST_HDF5:
+			val = image_hdf5_get_value(from, image->filename,
+			                           image->ev, &type);
+			break;
+
+			case DST_CBF:
+			case DST_CBFGZ:
+			/* FIXME: Implementation */
+			val = NAN;
+			break;
+
+			case DST_MSGPACK:
+			val = image_msgpack_get_value(from, image->data_block,
+			                              image->data_block_size,
+			                              &type);
+			break;
+
+			default:
+			ERROR("Unrecognised file type %i\n", image->data_source_type);
+			return 1;
+
+		}
 
-	} else {
-		ERROR("Unrecognised file type: %s\n", image->filename);
-		return NAN;
 	}
 
 	if ( type == 'f' ) {
@@ -401,6 +414,29 @@ static double get_value(struct image *image, const char *from,
 }
 
 
+static DataSourceType file_type(const char *filename)
+{
+	if ( !file_exists(filename) ) {
+		ERROR("File not found: %s\n", filename);
+		return DST_UNKNOWN;
+	}
+
+	if ( is_hdf5_file(filename) ) {
+		return DST_HDF5;
+
+	} else if ( is_cbf_file(filename) ) {
+		return DST_CBF;
+
+	} else if ( is_cbfgz_file(filename) ) {
+		return DST_CBFGZ;
+
+	} else {
+		ERROR("Unrecognised file type: %s\n", filename);
+		return DST_UNKNOWN;
+	}
+}
+
+
 static char *get_value_and_units(struct image *image, const char *from,
                                  double *pvalue,
                                  int *is_literal_number)
@@ -630,28 +666,39 @@ int image_set_zero_mask(struct image *image,
 
 
 static int image_read_image_data(struct image *image,
-                                 const DataTemplate *dtempl,
-                                 const char *filename,
-                                 const char *event)
+                                 const DataTemplate *dtempl)
 {
-	if ( !file_exists(filename) ) {
-		ERROR("File not found: %s\n", filename);
+	if ( (image->filename != NULL)
+	  && (!file_exists(image->filename)) )
+	{
+		ERROR("File not found: %s\n", image->filename);
 		return image_set_zero_data(image, dtempl);
 	}
 
-	if ( is_hdf5_file(filename) ) {
-		return image_hdf5_read(image, dtempl, filename, event);
+	switch ( image->data_source_type ) {
 
-	} else if ( is_cbf_file(filename) ) {
-		return image_cbf_read(image, dtempl, filename, event, 0);
+		case DST_HDF5:
+		return image_hdf5_read(image, dtempl,
+		                       image->filename, image->ev);
 
-	} else if ( is_cbfgz_file(filename) ) {
-		return image_cbf_read(image, dtempl, filename, event, 1);
+		case DST_CBF:
+		return image_cbf_read(image, dtempl,
+		                      image->filename, image->ev,
+		                      0);
 
-	}
+		case DST_CBFGZ:
+		return image_cbf_read(image, dtempl,
+		                      image->filename, image->ev,
+		                      1);
 
-	ERROR("Unrecognised file type: %s\n", filename);
-	return 1;
+		case DST_MSGPACK:
+		return image_msgpack_read(image, dtempl, image->data_block,
+		                          image->data_block_size);
+
+		default:
+		ERROR("Unrecognised file type %i\n", image->data_source_type);
+		return 1;
+	}
 }
 
 
@@ -1084,6 +1131,31 @@ struct image *image_create_for_simulation(const DataTemplate *dtempl)
 }
 
 
+static int do_image_read(struct image *image, const DataTemplate *dtempl,
+                         int no_image_data, int no_mask_data)
+{
+	int i;
+
+	/* Load the image data */
+	if ( !no_image_data ) {
+		if ( image_read_image_data(image, dtempl) ) return 1;
+	} else {
+		if ( image_set_zero_data(image, dtempl) ) return 1;
+	}
+
+	set_image_parameters(image, dtempl);
+	if ( create_detgeom(image, dtempl) ) return 1;
+	if ( create_badmap(image, dtempl, no_mask_data) ) return 1;
+	if ( create_satmap(image, dtempl) ) return 1;
+
+	for ( i=0; i<dtempl->n_headers_to_copy; i++ ) {
+		get_value(image, dtempl->headers_to_copy[i], NULL);
+	}
+
+	return 0;
+}
+
+
 struct image *image_read(const DataTemplate *dtempl,
                          const char *filename,
                          const char *event,
@@ -1091,8 +1163,6 @@ struct image *image_read(const DataTemplate *dtempl,
                          int no_mask_data)
 {
 	struct image *image;
-	int r;
-	int i;
 
 	if ( dtempl == NULL ) {
 		ERROR("NULL data template!\n");
@@ -1111,40 +1181,52 @@ struct image *image_read(const DataTemplate *dtempl,
 	} else {
 		image->ev = strdup("//");  /* Null event */
 	}
+	image->data_block = NULL;
+	image->data_block_size = 0;
 
-	/* Load the image data */
-	if ( !no_image_data ) {
-		r = image_read_image_data(image, dtempl,
-		                          filename, event);
-	} else {
-		r = image_set_zero_data(image, dtempl);
-	}
-	if ( r ) {
+	image->data_source_type = file_type(image->filename);
+
+	if ( do_image_read(image, dtempl, no_image_data, no_mask_data) ) {
 		image_free(image);
 		return NULL;
 	}
 
-	set_image_parameters(image, dtempl);
+	return image;
+}
 
-	if ( create_detgeom(image, dtempl) ) {
-		image_free(image);
+
+struct image *image_read_data_block(const DataTemplate *dtempl,
+                                    void *data_block,
+                                    size_t data_block_size,
+                                    DataSourceType type,
+                                    int no_image_data,
+                                    int no_mask_data)
+{
+	struct image *image;
+
+	if ( dtempl == NULL ) {
+		ERROR("NULL data template!\n");
 		return NULL;
 	}
 
-	if ( create_badmap(image, dtempl, no_mask_data) ) {
-		image_free(image);
+	image = image_new();
+	if ( image == NULL ) {
+		ERROR("Couldn't allocate image structure.\n");
 		return NULL;
 	}
 
-	if ( create_satmap(image, dtempl) ) {
+	image->filename = NULL;
+	image->ev = NULL;
+	image->data_block = data_block;
+	image->data_block_size = data_block_size;
+
+	image->data_source_type = file_type(image->filename);
+
+	if ( do_image_read(image, dtempl, no_image_data, no_mask_data) ) {
 		image_free(image);
 		return NULL;
 	}
 
-	for ( i=0; i<dtempl->n_headers_to_copy; i++ ) {
-		get_value(image, dtempl->headers_to_copy[i], NULL);
-	}
-
 	return image;
 }
 
@@ -1159,6 +1241,7 @@ void image_free(struct image *image)
 	spectrum_free(image->spectrum);
 	free(image->filename);
 	free(image->ev);
+	free(image->data_block);
 
 	if ( image->detgeom != NULL ) {
 		np = image->detgeom->n_panels;
@@ -1187,32 +1270,36 @@ void image_free(struct image *image)
 
 struct image *image_new()
 {
-       struct image *image;
-
-       image = malloc(sizeof(struct image));
-       if ( image == NULL ) return NULL;
-
-       image->dp = NULL;
-       image->bad = NULL;
-       image->sat = NULL;
-       image->hit = 0;
-       image->crystals = NULL;
-       image->n_crystals = 0;
-       image->indexed_by = INDEXING_NONE;
-       image->detgeom = NULL;
-       image->filename = NULL;
-       image->ev = NULL;
-       image->n_cached_headers = 0;
-       image->id = 0;
-       image->serial = 0;
-       image->spectrum = NULL;
-       image->lambda = -1.0;
-       image->div = 0.0;
-       image->bw = -1.0;
-       image->peak_resolution = -1.0;
-       image->features = NULL;
-
-       return image;
+	struct image *image;
+
+	image = malloc(sizeof(struct image));
+	if ( image == NULL ) return NULL;
+
+	image->dp = NULL;
+	image->bad = NULL;
+	image->sat = NULL;
+	image->hit = 0;
+	image->crystals = NULL;
+	image->n_crystals = 0;
+	image->indexed_by = INDEXING_NONE;
+	image->detgeom = NULL;
+	image->filename = NULL;
+	image->ev = NULL;
+	image->data_block = NULL;
+	image->data_block_size = 0;
+	image->data_source_type = DST_UNKNOWN;
+
+	image->n_cached_headers = 0;
+	image->id = 0;
+	image->serial = 0;
+	image->spectrum = NULL;
+	image->lambda = -1.0;
+	image->div = 0.0;
+	image->bw = -1.0;
+	image->peak_resolution = -1.0;
+	image->features = NULL;
+
+	return image;
 }
 
 
diff --git a/libcrystfel/src/image.h b/libcrystfel/src/image.h
index 181d0433..58228e35 100644
--- a/libcrystfel/src/image.h
+++ b/libcrystfel/src/image.h
@@ -88,6 +88,16 @@ struct header_cache_entry {
 };
 
 
+typedef enum
+{
+	DST_UNKNOWN,
+	DST_HDF5,
+	DST_CBF,
+	DST_CBFGZ,
+	DST_MSGPACK
+} DataSourceType;
+
+
 struct image
 {
 	/** The image data, by panel */
@@ -117,12 +127,19 @@ struct image
 	/** The detector structure */
 	struct detgeom          *detgeom;
 
+	DataSourceType           data_source_type;
+
 	/** \name The filename and event ID for the image
 	 * @{ */
 	char                    *filename;
 	char                    *ev;
 	/** @} */
 
+	/** The data block, e.g. received over ZMQ, for the image.
+	 * filenename/ev OR this should be filled in, but not both */
+	void                    *data_block;
+	size_t                   data_block_size;
+
 	/** A list of metadata read from the stream */
 	struct header_cache_entry *header_cache[HEADER_CACHE_SIZE];
 	int                        n_cached_headers;
@@ -195,6 +212,12 @@ extern struct image *image_read(const DataTemplate *dtempl,
                                 int no_image_data,
                                 int no_mask_data);
 extern struct image *image_create_for_simulation(const DataTemplate *dtempl);
+extern struct image *image_read_data_block(const DataTemplate *dtempl,
+                                           void *data_block,
+                                           size_t data_block_size,
+                                           DataSourceType type,
+                                           int no_image_data,
+                                           int no_mask_data);
 extern void image_free(struct image *image);
 
 extern void image_cache_header_float(struct image *image,