diff --git a/include/share/grabbag/picture.h b/include/share/grabbag/picture.h index a8322124..bae6536a 100644 --- a/include/share/grabbag/picture.h +++ b/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 diff --git a/src/share/grabbag/picture.c b/src/share/grabbag/picture.c index 1bf96cfc..ab8b5719 100644 --- a/src/share/grabbag/picture.c +++ b/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); - 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); - } - } - } - } + *error_message = read_file (spec, obj); } } } @@ -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; +} diff --git a/src/test_grabbag/picture/main.c b/src/test_grabbag/picture/main.c index ea7ad728..a21f3d65 100644 --- a/src/test_grabbag/picture/main.c +++ b/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; - picturefiles[0].height = 240; - picturefiles[0].depth = 3; - picturefiles[0].colors = 2; - if(!test_one_picture(prefix, picturefiles+0, "320x240x3/2", /*fn_only=*/false)) + res.width = picturefiles[0].width = 320; + res.height = picturefiles[0].height = 240; + res.depth = picturefiles[0].depth = 3; + res.colors = picturefiles[0].colors = 2; + if(!test_one_picture(prefix, picturefiles+0, &res, /*fn_only=*/false)) return false; return true; diff --git a/test/test_grabbag.sh b/test/test_grabbag.sh index 5156a268..627a9fa4 100755 --- a/test/test_grabbag.sh +++ b/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 -good_cuesheets=cuesheets/good.*.cue +bad_cuesheets=${top_srcdir}/test/cuesheets/bad.*.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)"