Update grabbag tests to work out-of-tree.

include/share/grabbag/picture.h

@@ -40,6 +40,13 @@ extern "C" {
  */
 FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, const char **error_message);
 
+typedef struct PictureResolution
+{	uint32_t width, height, depth, colors ;
+} PictureResolution ;
+
+FLAC__StreamMetadata *grabbag__picture_from_specification(int type, const char *mime_type, const char * description,
+		const PictureResolution * res, const char * filepath, const char **error_message);
+
 #ifdef __cplusplus
 }
 #endif

src/share/grabbag/picture.c

@@ -261,22 +261,61 @@ static FLAC__bool local__extract_resolution_color_info_(FLAC__StreamMetadata_Pic
 	return false;
 }
 
+static const char *error_messages[] = {
+	"memory allocation error",
+	"invalid picture specification",
+	"invalid picture specification: can't parse resolution/color part",
+	"unable to extract resolution and color info from URL, user must set explicitly",
+	"unable to extract resolution and color info from file, user must set explicitly",
+	"error opening picture file",
+	"error reading picture file",
+	"invalid picture type",
+	"unable to guess MIME type from file, user must set explicitly",
+	"type 1 icon must be a 32x32 pixel PNG",
+	"file not found"
+};
+
+static const char * read_file (const char * filepath, FLAC__StreamMetadata * obj)
+{
+	const FLAC__off_t size = grabbag__file_get_filesize(filepath);
+	FLAC__byte *buffer;
+	FILE *file;
+	const char *error_message=NULL;
+
+	if (size < 0)
+		return error_messages[5];
+
+	if ((buffer = safe_malloc_(size)) == NULL)
+		return error_messages[0];
+
+	if ((file = flac_fopen(filepath, "rb")) == NULL) {
+		free(buffer);
+		return error_messages[5];
+	}
+
+	if (fread(buffer, 1, size, file) != (size_t) size) {
+		fclose(file);
+		free(buffer);
+		return error_messages[6];
+	}
+	fclose(file);
+
+	if (!FLAC__metadata_object_picture_set_data(obj, buffer, size, /*copy=*/false))
+		error_message = error_messages[6];
+	/* try to extract MIME type if user left it blank */
+	else if (*obj->data.picture.mime_type == '\0' && !local__extract_mime_type_(obj))
+		error_message = error_messages[8];
+	/* try to extract resolution/color info if user left it blank */
+	else if ((obj->data.picture.width == 0 || obj->data.picture.height == 0 || obj->data.picture.depth == 0) && !local__extract_resolution_color_info_(&obj->data.picture))
+		error_message = error_messages[4];
+
+	return error_message;
+}
+
 FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, const char **error_message)
 {
 	FLAC__StreamMetadata *obj;
 	int state = 0;
-	static const char *error_messages[] = {
-		"memory allocation error",
-		"invalid picture specification",
-		"invalid picture specification: can't parse resolution/color part",
-		"unable to extract resolution and color info from URL, user must set explicitly",
-		"unable to extract resolution and color info from file, user must set explicitly",
-		"error opening picture file",
-		"error reading picture file",
-		"invalid picture type",
-		"unable to guess MIME type from file, user must set explicitly",
-		"type 1 icon must be a 32x32 pixel PNG"
-	};
 
 	FLAC__ASSERT(0 != spec);
 	FLAC__ASSERT(0 != error_message);
@@ -358,39 +397,7 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con
 					*error_message = error_messages[3];
 			}
 			else { /* regular picture file */
-				const FLAC__off_t size = grabbag__file_get_filesize(spec);
+				*error_message = read_file (spec, obj);
-				if(size < 0)
-					*error_message = error_messages[5];
-				else {
-					FLAC__byte *buffer = safe_malloc_(size);
-					if(0 == buffer)
-						*error_message = error_messages[0];
-					else {
-						FILE *f = flac_fopen(spec, "rb");
-						if(0 == f) {
-							*error_message = error_messages[5];
-							free(buffer);
-						}
-						else {
-							if(fread(buffer, 1, size, f) != (size_t)size)
-								*error_message = error_messages[6];
-							fclose(f);
-							if(0 == *error_message) {
-								if(!FLAC__metadata_object_picture_set_data(obj, buffer, size, /*copy=*/false))
-									*error_message = error_messages[6];
-								/* try to extract MIME type if user left it blank */
-								else if(*obj->data.picture.mime_type == '\0' && !local__extract_mime_type_(obj))
-									*error_message = error_messages[8];
-								/* try to extract resolution/color info if user left it blank */
-								else if((obj->data.picture.width == 0 || obj->data.picture.height == 0 || obj->data.picture.depth == 0) && !local__extract_resolution_color_info_(&obj->data.picture))
-									*error_message = error_messages[4];
-							}
-							else {
-								free(buffer);
-							}
-						}
-					}
-				}
 			}
 		}
 	}
@@ -414,3 +421,80 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con
 
 	return obj;
 }
+
+FLAC__StreamMetadata *grabbag__picture_from_specification(int type, const char *mime_type_in, const char * description,
+		const PictureResolution * res, const char * filepath, const char **error_message)
+{
+
+	FLAC__StreamMetadata *obj;
+	char mime_type [64] ;
+
+	if (error_message == 0)
+		return 0;
+
+	strncpy (mime_type, mime_type_in, sizeof (mime_type));
+
+	*error_message = 0;
+
+	if ((obj = FLAC__metadata_object_new(FLAC__METADATA_TYPE_PICTURE)) == 0) {
+		*error_message = error_messages[0];
+		return obj;
+	}
+
+	/* Picture type if known. */
+	obj->data.picture.type = type >= 0 ? type : FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER;
+
+	/* Mime type if known. */
+	if (mime_type_in && ! FLAC__metadata_object_picture_set_mime_type(obj, mime_type, /*copy=*/true)) {
+		*error_message = error_messages[0];
+		return obj;
+	}
+
+	/* Description if present. */
+	if (description && ! FLAC__metadata_object_picture_set_description(obj, (FLAC__byte*) description, /*copy=*/true)) {
+		*error_message = error_messages[0];
+		return obj;
+	}
+
+	if (res == NULL) {
+		obj->data.picture.width = 0;
+		obj->data.picture.height = 0;
+		obj->data.picture.depth = 0;
+		obj->data.picture.colors = 0;
+	}
+	else {
+		obj->data.picture.width = res->width;
+		obj->data.picture.height = res->height;
+		obj->data.picture.depth = res->depth;
+		obj->data.picture.colors = res->colors;
+	}
+
+	if (strcmp(obj->data.picture.mime_type, "-->") == 0) { /* magic MIME type means URL */
+		if (!FLAC__metadata_object_picture_set_data(obj, (FLAC__byte*)filepath, strlen(filepath), /*copy=*/true))
+			*error_message = error_messages[0];
+		else if (obj->data.picture.width == 0 || obj->data.picture.height == 0 || obj->data.picture.depth == 0)
+			*error_message = error_messages[3];
+	}
+	else {
+		*error_message = read_file (filepath, obj);
+	}
+
+	if (*error_message == NULL) {
+		if (
+			obj->data.picture.type == FLAC__STREAM_METADATA_PICTURE_TYPE_FILE_ICON_STANDARD &&
+			(
+				(strcmp(obj->data.picture.mime_type, "image/png") && strcmp(obj->data.picture.mime_type, "-->")) ||
+				obj->data.picture.width != 32 ||
+				obj->data.picture.height != 32
+			)
+		)
+			*error_message = error_messages[9];
+	}
+
+	if (*error_message && obj) {
+		FLAC__metadata_object_delete(obj);
+		obj = 0;
+	}
+
+	return obj;
+}

src/test_grabbag/picture/main.c

@@ -65,18 +65,22 @@ static FLAC__bool failed_(const char *msg)
     return false;
 }
 
-static FLAC__bool test_one_picture(const char *prefix, const PictureFile *pf, const char *res, FLAC__bool fn_only)
+static FLAC__bool test_one_picture(const char *prefix, const PictureFile *pf, const PictureResolution * res, FLAC__bool fn_only)
 {
 	FLAC__StreamMetadata *obj;
 	const char *error;
 	char s[4096];
 	if(fn_only)
-		flac_snprintf(s, sizeof(s), "%s/%s", prefix, pf->path);
+		flac_snprintf(s, sizeof(s), "pictures/%s", pf->path);
+	else if (res == NULL)
+		flac_snprintf(s, sizeof(s), "%u|%s|%s||pictures/%s", (unsigned)pf->type, pf->mime_type, pf->description, pf->path);
 	else
-		flac_snprintf(s, sizeof(s), "%u|%s|%s|%s|%s/%s", (unsigned)pf->type, pf->mime_type, pf->description, res, prefix, pf->path);
+		flac_snprintf(s, sizeof(s), "%u|%s|%s|%dx%dx%d/%d|pictures/%s", (unsigned)pf->type, pf->mime_type, pf->description, res->width, res->height, res->depth, res->colors, pf->path);
 
 	printf("testing grabbag__picture_parse_specification(\"%s\")... ", s);
-	if(0 == (obj = grabbag__picture_parse_specification(s, &error)))
+
+	flac_snprintf(s, sizeof(s), "%s/%s", prefix, pf->path);
+	if((obj = grabbag__picture_from_specification(fn_only? FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER : pf->type, pf->mime_type, pf->description, res, s, &error)) == 0)
 		return failed_(error);
 	if(debug_) {
 		printf("\ntype=%u (%s)\nmime_type=%s\ndescription=%s\nwidth=%u\nheight=%u\ndepth=%u\ncolors=%u\ndata_length=%u\n",
@@ -114,6 +118,7 @@ static FLAC__bool test_one_picture(const char *prefix, const PictureFile *pf, co
 static FLAC__bool do_picture(const char *prefix)
 {
 	FLAC__StreamMetadata *obj;
+	PictureResolution res;
 	const char *error;
 	size_t i;
 
@@ -181,19 +186,19 @@ static FLAC__bool do_picture(const char *prefix)
 
 	/* test automatic parsing of picture files from only the file name */
 	for(i = 0; i < sizeof(picturefiles)/sizeof(picturefiles[0]); i++)
-		if(!test_one_picture(prefix, picturefiles+i, "", /*fn_only=*/true))
+		if(!test_one_picture(prefix, picturefiles+i, NULL, /*fn_only=*/true))
 			return false;
 
 	/* test automatic parsing of picture files to get resolution/color info */
 	for(i = 0; i < sizeof(picturefiles)/sizeof(picturefiles[0]); i++)
-		if(!test_one_picture(prefix, picturefiles+i, "", /*fn_only=*/false))
+		if(!test_one_picture(prefix, picturefiles+i, NULL, /*fn_only=*/false))
 			return false;
 
-	picturefiles[0].width = 320;
+	res.width = picturefiles[0].width = 320;
-	picturefiles[0].height = 240;
+	res.height = picturefiles[0].height = 240;
-	picturefiles[0].depth = 3;
+	res.depth = picturefiles[0].depth = 3;
-	picturefiles[0].colors = 2;
+	res.colors = picturefiles[0].colors = 2;
-	if(!test_one_picture(prefix, picturefiles+0, "320x240x3/2", /*fn_only=*/false))
+	if(!test_one_picture(prefix, picturefiles+0, &res, /*fn_only=*/false))
 		return false;
 
 	return true;

test/test_grabbag.sh

@@ -60,7 +60,7 @@ fi
 ########################################################################
 
 log=picture.log
-picture_dir=pictures
+picture_dir=${top_srcdir}/test/pictures
 
 echo "Running test_picture..."
 
@@ -69,9 +69,9 @@ rm -f $log
 run_test_picture $picture_dir >> $log 2>&1
 
 if [ $is_win = yes ] ; then
-	diff -w picture.ok $log > picture.diff || die "Error: .log file does not match .ok file, see picture.diff"
+	diff -w ${top_srcdir}/test/picture.ok $log > picture.diff || die "Error: .log file does not match .ok file, see picture.diff"
 else
-	diff picture.ok $log > picture.diff || die "Error: .log file does not match .ok file, see picture.diff"
+	diff ${top_srcdir}/test/picture.ok $log > picture.diff || die "Error: .log file does not match .ok file, see picture.diff"
 fi
 
 echo "PASSED (results are in $log)"
@@ -83,8 +83,8 @@ echo "PASSED (results are in $log)"
 ########################################################################
 
 log=cuesheet.log
-bad_cuesheets=cuesheets/bad.*.cue
+bad_cuesheets=${top_srcdir}/test/cuesheets/bad.*.cue
-good_cuesheets=cuesheets/good.*.cue
+good_cuesheets=${top_srcdir}/test/cuesheets/good.*.cue
 good_leadout=`expr 80 \* 60 \* 44100`
 bad_leadout=`expr $good_leadout + 1`
 
@@ -96,7 +96,7 @@ rm -f $log
 # negative tests
 #
 for cuesheet in $bad_cuesheets ; do
-	echo "NEGATIVE $cuesheet" >> $log 2>&1
+	echo "NEGATIVE $cuesheet" | sed "s|${top_srcdir}/test/||" >> $log 2>&1
 	run_test_cuesheet $cuesheet $good_leadout 44100 cdda >> $log 2>&1 || exit_code=$?
 	if [ "$exit_code" = 255 ] ; then
 		die "Error: test script is broken"
@@ -110,7 +110,7 @@ done
 # positve tests
 #
 for cuesheet in $good_cuesheets ; do
-	echo "POSITIVE $cuesheet" >> $log 2>&1
+	echo "POSITIVE $cuesheet" | sed "s|${top_srcdir}/test/||" >> $log 2>&1
 	run_test_cuesheet $cuesheet $good_leadout 44100 cdda >> $log 2>&1
 	exit_code=$?
 	if [ "$exit_code" = 255 ] ; then
@@ -125,9 +125,9 @@ for cuesheet in $good_cuesheets ; do
 done
 
 if [ $is_win = yes ] ; then
-	diff -w cuesheet.ok $log > cuesheet.diff || die "Error: .log file does not match .ok file, see cuesheet.diff"
+	diff -w ${top_srcdir}/test/cuesheet.ok $log > cuesheet.diff || die "Error: .log file does not match .ok file, see cuesheet.diff"
 else
-	diff cuesheet.ok $log > cuesheet.diff || die "Error: .log file does not match .ok file, see cuesheet.diff"
+	diff ${top_srcdir}/test/cuesheet.ok $log > cuesheet.diff || die "Error: .log file does not match .ok file, see cuesheet.diff"
 fi
 
 echo "PASSED (results are in $log)"