media: move addition of pictures out of create_picture()

If we want to make addition of pictures undoable, then create_picture()
must not add directly to the dive. Instead, return the dive to which the
picture should be added and let the caller perform the addition.

This means that the picture-test has to be adapted.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

core/dive.c

@@ -3556,7 +3556,7 @@ bool picture_check_valid_time(timestamp_t timestamp, int shift_time)
 	return false;
 }
 
-static void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture, struct dive_site_table *table)
+void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture, struct dive_site_table *table)
 {
 	struct dive_site *ds = dive->dive_site;
 	if (!dive_site_has_gps_location(ds) && has_location(&picture->location)) {
@@ -3570,31 +3570,31 @@ static void dive_set_geodata_from_picture(struct dive *dive, struct picture *pic
 	}
 }
 
-void create_picture(const char *filename, int shift_time, bool match_all)
+/* Creates a picture and indicates the dive to which this picture should be added.
+ * The caller is responsible for actually adding the picture to the dive.
+ * If no appropriate dive was found, no picture is created and NULL is returned.
+ */
+struct picture *create_picture(const char *filename, int shift_time, bool match_all, struct dive **dive)
 {
 	struct metadata metadata;
-	struct dive *dive;
 	timestamp_t timestamp;
 
 	get_metadata(filename, &metadata);
 	timestamp = metadata.timestamp + shift_time;
-	dive = nearest_selected_dive(timestamp);
+	*dive = nearest_selected_dive(timestamp);
 
-	if (!dive)
+	if (!*dive)
-		return;
+		return NULL;
-	if (get_picture_idx(&dive->pictures, filename) >= 0)
+	if (get_picture_idx(&(*dive)->pictures, filename) >= 0)
-		return;
+		return NULL;
-	if (!match_all && !dive_check_picture_time(dive, timestamp))
+	if (!match_all && !dive_check_picture_time(*dive, timestamp))
-		return;
+		return NULL;
 
-	struct picture picture;
+	struct picture *picture = malloc(sizeof(struct picture));
-	picture.filename = strdup(filename);
+	picture->filename = strdup(filename);
-	picture.offset.seconds = metadata.timestamp - dive->when + shift_time;
+	picture->offset.seconds = metadata.timestamp - (*dive)->when + shift_time;
-	picture.location = metadata.location;
+	picture->location = metadata.location;
-
+	return picture;
-	add_picture(&dive->pictures, picture);
-	dive_set_geodata_from_picture(dive, &picture, &dive_site_table);
-	invalidate_dive_cache(dive);
 }
 
 /* clones a dive and moves given dive computer to front */

core/dive.h

@@ -210,8 +210,9 @@ extern enum divemode_t get_divemode_at_time(const struct divecomputer *dc, int d
 		for (struct picture *picture = (_dive)->pictures.pictures;		\
 		     picture < (_dive)->pictures.pictures + (_dive)->pictures.nr;	\
 		     picture++)
-extern void create_picture(const char *filename, int shift_time, bool match_all);
+extern struct picture *create_picture(const char *filename, int shift_time, bool match_all, struct dive **dive);
 extern bool picture_check_valid_time(timestamp_t timestamp, int shift_time);
+extern void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture, struct dive_site_table *table);
 
 extern bool has_gaschange_event(const struct dive *dive, const struct divecomputer *dc, int idx);
 extern int explicit_first_cylinder(const struct dive *dive, const struct divecomputer *dc);

desktop-widgets/divelistview.cpp

@@ -11,6 +11,7 @@
 #include "desktop-widgets/divepicturewidget.h"
 #include "core/selection.h"
 #include "core/divefilter.h"
+#include "core/divesite.h" // for dive_site_table. TODO: remove once adding pictures is undoified
 #include <unistd.h>
 #include <QSettings>
 #include <QKeyEvent>
@@ -890,8 +891,16 @@ void DiveListView::matchImagesToDives(QStringList fileNames)
 		return;
 	updateLastImageTimeOffset(shiftDialog.amount());
 
-	for (const QString &fileName: fileNames)
+	for (const QString &fileName: fileNames) {
-		create_picture(qPrintable(fileName), shiftDialog.amount(), shiftDialog.matchAll());
+		struct dive *d;
+		picture *pic = create_picture(qPrintable(fileName), shiftDialog.amount(), shiftDialog.matchAll(), &d);
+		if (!pic)
+			continue;
+		add_picture(&d->pictures, *pic);
+		dive_set_geodata_from_picture(d, pic, &dive_site_table);
+		invalidate_dive_cache(d);
+		free(pic);
+	}
 
 	mark_divelist_changed(true);
 	copy_dive(current_dive, &displayed_dive);

tests/testpicture.cpp

@@ -22,7 +22,7 @@ void TestPicture::initTestCase()
 
 void TestPicture::addPicture()
 {
-	struct dive *dive;
+	struct dive *dive, *dive1, *dive2;
 	struct picture *pic1, *pic2;
 	verbose = 1;
 
@@ -34,9 +34,19 @@ void TestPicture::addPicture()
 	// So far no picture in dive
 	QVERIFY(dive->pictures.nr == 0);
 
-	create_picture(SUBSURFACE_TEST_DATA "/dives/images/wreck.jpg", 0, false);
+	pic1 = create_picture(SUBSURFACE_TEST_DATA "/dives/images/wreck.jpg", 0, false, &dive1);
-	create_picture(SUBSURFACE_TEST_DATA "/dives/images/data_after_EOI.jpg", 0, false);
+	pic2 = create_picture(SUBSURFACE_TEST_DATA "/dives/images/data_after_EOI.jpg", 0, false, &dive2);
-	// Now there are two picture2
+	QVERIFY(pic1 != NULL);
+	QVERIFY(pic2 != NULL);
+	QVERIFY(dive1 == dive);
+	QVERIFY(dive2 == dive);
+
+	add_picture(&dive->pictures, *pic1);
+	add_picture(&dive->pictures, *pic2);
+	free(pic1);
+	free(pic2);
+
+	// Now there are two pictures
 	QVERIFY(dive->pictures.nr == 2);
 	pic1 = &dive->pictures.pictures[0];
 	pic2 = &dive->pictures.pictures[1];