From 6e94c90e606b9fcaff60bfb961dced8292075cb2 Mon Sep 17 00:00:00 2001 From: Josh Coalson Date: Sun, 24 Sep 2006 16:17:53 +0000 Subject: [PATCH] add parsing of picture type from spec; add check that type 1 picture is 32x32 PNG --- include/share/grabbag/picture.h | 11 ++++- src/share/grabbag/picture.c | 55 +++++++++++++++++++++--- src/test_grabbag/picture/main.c | 76 +++++++++++++++++++-------------- 3 files changed, 103 insertions(+), 39 deletions(-) diff --git a/include/share/grabbag/picture.h b/include/share/grabbag/picture.h index ed864f7d..e69072aa 100644 --- a/include/share/grabbag/picture.h +++ b/include/share/grabbag/picture.h @@ -27,7 +27,16 @@ extern "C" { #endif -/* spec and error_message must not be NULL */ +/* spec should be of the form "[TYPE]|MIME_TYPE|[DESCRIPTION]|[WIDTHxHEIGHTxDEPTH[/COLORS]]|FILE", e.g. + * "|image/jpeg|||cover.jpg" + * "4|image/jpeg||300x300x24|backcover.jpg" + * "|image/png|description|300x300x24/71|cover.png" + * "-->|image/gif||300x300x24/71|http://blah.blah.blah/cover.gif" + * + * empty type means default to FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER + * empty resolution spec means to get from the file (cannot get used with "-->" linked images) + * spec and error_message must not be NULL + */ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, const char **error_message); #ifdef __cplusplus diff --git a/src/share/grabbag/picture.c b/src/share/grabbag/picture.c index 2e5779cb..6137447f 100644 --- a/src/share/grabbag/picture.c +++ b/src/share/grabbag/picture.c @@ -37,6 +37,31 @@ static char *local__strndup_(const char *s, size_t size) return x; } +static FLAC__bool local__parse_type_(const char *s, size_t len, FLAC__StreamMetadata_Picture *picture) +{ + size_t i; + FLAC__uint32 val = 0; + + picture->type = FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER;; + + if(len == 0) + return true; /* empty string implies default to 'front cover' */ + + for(i = 0; i < len; i++) { + if(s[i] >= '0' && s[i] <= '9') + val = 10*val + (FLAC__uint32)(s[i] - '0'); + else + return false; + } + + if(i == len) + picture->type = val; + else + return false; + + return true; +} + static FLAC__bool local__parse_resolution_(const char *s, size_t len, FLAC__StreamMetadata_Picture *picture) { int state = 0; @@ -236,7 +261,9 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con "unable to extract resolution and color info from file, user must set explicitly", "error opening picture file", "error reading picture file", - "invalid MIME type" + "invalid picture type", + "invalid MIME type", + "type 1 icon must be a 32x32 pixel PNG" }; FLAC__ASSERT(0 != spec); @@ -256,21 +283,25 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con for(p = spec; *error_message==0 && *p; ) { if(*p == '|') { switch(state) { - case 0: /* mime type */ - if(p-spec == 0) + case 0: /* type */ + if(!local__parse_type_(spec, p-spec, &obj->data.picture)) *error_message = error_messages[7]; + break; + case 1: /* mime type */ + if(p-spec == 0) + *error_message = error_messages[8]; else if(0 == (q = local__strndup_(spec, p-spec))) *error_message = error_messages[0]; else if(!FLAC__metadata_object_picture_set_mime_type(obj, q, /*copy=*/false)) *error_message = error_messages[0]; break; - case 1: /* description */ + case 2: /* description */ if(0 == (q = local__strndup_(spec, p-spec))) *error_message = error_messages[0]; else if(!FLAC__metadata_object_picture_set_description(obj, (FLAC__byte*)q, /*copy=*/false)) *error_message = error_messages[0]; break; - case 2: /* resolution/color (e.g. [300x300x16[/1234]] */ + case 3: /* resolution/color (e.g. [300x300x16[/1234]] */ if(!local__parse_resolution_(spec, p-spec, &obj->data.picture)) *error_message = error_messages[2]; break; @@ -287,7 +318,7 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con } /* parse filename, read file, try to extract resolution/color info if needed */ if(*error_message == 0) { - if(state != 3) + if(state != 4) *error_message = error_messages[1]; else { /* 'spec' points to filename/URL */ if(0 == strcmp(obj->data.picture.mime_type, "-->")) { /* magic MIME type means URL */ @@ -328,6 +359,18 @@ FLAC__StreamMetadata *grabbag__picture_parse_specification(const char *spec, con } } + if(*error_message == 0) { + 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; diff --git a/src/test_grabbag/picture/main.c b/src/test_grabbag/picture/main.c index 898930ec..c17f4bca 100644 --- a/src/test_grabbag/picture/main.c +++ b/src/test_grabbag/picture/main.c @@ -32,23 +32,24 @@ typedef struct { FLAC__uint32 height; FLAC__uint32 depth; FLAC__uint32 colors; + FLAC__StreamMetadata_Picture_Type type; } PictureFile; PictureFile picturefiles[] = { - { "0.gif", "image/gif" , "", 24, 24, 24, 2 }, - { "1.gif", "image/gif" , "", 12, 8, 24, 256 }, - { "2.gif", "image/gif" , "", 16, 14, 24, 128 }, - { "0.jpg", "image/jpeg", "", 30, 20, 8, 0 }, - { "4.jpg", "image/jpeg", "", 31, 47, 24, 0 }, - { "0.png", "image/png" , "", 30, 20, 8, 0 }, - { "1.png", "image/png" , "", 30, 20, 8, 0 }, - { "2.png", "image/png" , "", 30, 20, 24, 7 }, - { "3.png", "image/png" , "", 30, 20, 24, 7 }, - { "4.png", "image/png" , "", 31, 47, 24, 0 }, - { "5.png", "image/png" , "", 31, 47, 24, 0 }, - { "6.png", "image/png" , "", 31, 47, 24, 23 }, - { "7.png", "image/png" , "", 31, 47, 24, 23 }, - { "8.png", "image/png" , "", 32, 32, 32, 0 } + { "0.gif", "image/gif" , "", 24, 24, 24, 2, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "1.gif", "image/gif" , "", 12, 8, 24, 256, FLAC__STREAM_METADATA_PICTURE_TYPE_BACK_COVER }, + { "2.gif", "image/gif" , "", 16, 14, 24, 128, FLAC__STREAM_METADATA_PICTURE_TYPE_OTHER }, + { "0.jpg", "image/jpeg", "", 30, 20, 8, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "4.jpg", "image/jpeg", "", 31, 47, 24, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "0.png", "image/png" , "", 30, 20, 8, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "1.png", "image/png" , "", 30, 20, 8, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "2.png", "image/png" , "", 30, 20, 24, 7, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "3.png", "image/png" , "", 30, 20, 24, 7, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "4.png", "image/png" , "", 31, 47, 24, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "5.png", "image/png" , "", 31, 47, 24, 0, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "6.png", "image/png" , "", 31, 47, 24, 23, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "7.png", "image/png" , "", 31, 47, 24, 23, FLAC__STREAM_METADATA_PICTURE_TYPE_FRONT_COVER }, + { "8.png", "image/png" , "", 32, 32, 32, 0, 999 } }; static FLAC__bool debug_ = false; @@ -68,13 +69,16 @@ static FLAC__bool test_one_picture(const char *prefix, const PictureFile *pf, co FLAC__StreamMetadata *obj; const char *error; char s[4096]; - snprintf(s, sizeof(s)-1, "%s|%s|%s|%s/%s", pf->mime_type, pf->description, res, prefix, pf->path); + snprintf(s, sizeof(s)-1, "%u|%s|%s|%s|%s/%s", (unsigned)pf->type, pf->mime_type, pf->description, res, prefix, pf->path); printf("testing grabbag__picture_parse_specification(\"%s\")... ", s); if(0 == (obj = grabbag__picture_parse_specification(s, &error))) return failed_(error); if(debug_) { - printf("\nmime_type=%s\ndescription=%s\nwidth=%u\nheight=%u\ndepth=%u\ncolors=%u\ndata_length=%u\n", + printf("\ntype=%u (%s)\nmime_type=%s\ndescription=%s\nwidth=%u\nheight=%u\ndepth=%u\ncolors=%u\ndata_length=%u\n", + obj->data.picture.type, + obj->data.picture.type < FLAC__STREAM_METADATA_PICTURE_TYPE_UNDEFINED? + FLAC__StreamMetadata_Picture_TypeString[obj->data.picture.type] : "UNDEFINED", obj->data.picture.mime_type, obj->data.picture.description, obj->data.picture.width, @@ -84,6 +88,8 @@ static FLAC__bool test_one_picture(const char *prefix, const PictureFile *pf, co obj->data.picture.data_length ); } + if(obj->data.picture.type != pf->type) + return failed_("picture type mismatch"); if(strcmp(obj->data.picture.mime_type, pf->mime_type)) return failed_("picture MIME type mismatch"); if(strcmp((const char *)obj->data.picture.description, (const char *)pf->description)) @@ -116,49 +122,55 @@ static FLAC__bool do_picture(const char *prefix) printf("OK (failed as expected, error: %s)\n", error); /* invalid spec: no filename */ - printf("testing grabbag__picture_parse_specification(\"|||\")... "); - if(0 != (obj = grabbag__picture_parse_specification("|||", &error))) + printf("testing grabbag__picture_parse_specification(\"||||\")... "); + if(0 != (obj = grabbag__picture_parse_specification("||||", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: no filename */ - printf("testing grabbag__picture_parse_specification(\"image/gif|||\")... "); - if(0 != (obj = grabbag__picture_parse_specification("image/gif|||", &error))) + printf("testing grabbag__picture_parse_specification(\"|image/gif|||\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|image/gif|||", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: bad resolution */ - printf("testing grabbag__picture_parse_specification(\"image/gif|desc|320|0.gif\")... "); - if(0 != (obj = grabbag__picture_parse_specification("image/gif|desc|320|0.gif", &error))) + printf("testing grabbag__picture_parse_specification(\"|image/gif|desc|320|0.gif\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|image/gif|desc|320|0.gif", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: bad resolution */ - printf("testing grabbag__picture_parse_specification(\"image/gif|desc|320x240|0.gif\")... "); - if(0 != (obj = grabbag__picture_parse_specification("image/gif|desc|320x240|0.gif", &error))) + printf("testing grabbag__picture_parse_specification(\"|image/gif|desc|320x240|0.gif\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|image/gif|desc|320x240|0.gif", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: no filename */ - printf("testing grabbag__picture_parse_specification(\"image/gif|desc|320x240x9|\")... "); - if(0 != (obj = grabbag__picture_parse_specification("image/gif|desc|320x240x9|", &error))) + printf("testing grabbag__picture_parse_specification(\"|image/gif|desc|320x240x9|\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|image/gif|desc|320x240x9|", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: #colors exceeds color depth */ - printf("testing grabbag__picture_parse_specification(\"image/gif|desc|320x240x9/2345|0.gif\")... "); - if(0 != (obj = grabbag__picture_parse_specification("image/gif|desc|320x240x9/2345|0.gif", &error))) + printf("testing grabbag__picture_parse_specification(\"|image/gif|desc|320x240x9/2345|0.gif\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|image/gif|desc|320x240x9/2345|0.gif", &error))) + return failed_("expected error, got object"); + printf("OK (failed as expected: %s)\n", error); + + /* invalid spec: standard icon has to be 32x32 PNG */ + printf("testing grabbag__picture_parse_specification(\"1|-->|desc|32x24x9|0.gif\")... "); + if(0 != (obj = grabbag__picture_parse_specification("1|-->|desc|32x24x9|0.gif", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); /* invalid spec: need resolution for linked URL */ - printf("testing grabbag__picture_parse_specification(\"-->|desc||http://blah.blah.blah/z.gif\")... "); - if(0 != (obj = grabbag__picture_parse_specification("-->|desc||http://blah.blah.blah/z.gif", &error))) + printf("testing grabbag__picture_parse_specification(\"|-->|desc||http://blah.blah.blah/z.gif\")... "); + if(0 != (obj = grabbag__picture_parse_specification("|-->|desc||http://blah.blah.blah/z.gif", &error))) return failed_("expected error, got object"); printf("OK (failed as expected: %s)\n", error); - printf("testing grabbag__picture_parse_specification(\"-->|desc|320x240x9|http://blah.blah.blah/z.gif\")... "); - if(0 == (obj = grabbag__picture_parse_specification("-->|desc|320x240x9|http://blah.blah.blah/z.gif", &error))) + printf("testing grabbag__picture_parse_specification(\"|-->|desc|320x240x9|http://blah.blah.blah/z.gif\")... "); + if(0 == (obj = grabbag__picture_parse_specification("|-->|desc|320x240x9|http://blah.blah.blah/z.gif", &error))) return failed_(error); printf("OK\n");