From 8ae390f65142ed38a0b5e2474fc6a21866092e84 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:11 +0200 Subject: tag: renamed MpdTag and MpdTagItem to struct mpd_tag, struct tag_item Getting rid of CamelCase; not having typedefs also allows us to forward-declare the structures. --- src/tag.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index c1ccd0a42..b50756036 100644 --- a/src/tag.c +++ b/src/tag.c @@ -114,7 +114,7 @@ void printTagTypes(int fd) } } -void printMpdTag(int fd, MpdTag * tag) +void printMpdTag(int fd, struct mpd_tag *tag) { int i; @@ -164,8 +164,8 @@ static id3_utf8_t * processID3FieldString (int is_id3v1, const id3_ucs4_t *ucs4, return utf8; } -static MpdTag *getID3Info( - struct id3_tag *tag, const char *id, int type, MpdTag * mpdTag) +static struct mpd_tag *getID3Info( + struct id3_tag *tag, const char *id, int type, struct mpd_tag *mpdTag) { struct id3_frame const *frame; id3_ucs4_t const *ucs4; @@ -283,9 +283,9 @@ static MpdTag *getID3Info( #endif #ifdef HAVE_ID3TAG -MpdTag *parseId3Tag(struct id3_tag * tag) +struct mpd_tag *parseId3Tag(struct id3_tag * tag) { - MpdTag *ret = NULL; + struct mpd_tag *ret = NULL; ret = getID3Info(tag, ID3_FRAME_ARTIST, TAG_ITEM_ARTIST, ret); ret = getID3Info(tag, ID3_FRAME_TITLE, TAG_ITEM_TITLE, ret); @@ -425,9 +425,9 @@ static struct id3_tag *findId3TagFromEnd(FILE * stream) } #endif -MpdTag *id3Dup(char *file) +struct mpd_tag *id3Dup(char *file) { - MpdTag *ret = NULL; + struct mpd_tag *ret = NULL; #ifdef HAVE_ID3TAG struct id3_tag *tag; FILE *stream; @@ -453,9 +453,9 @@ MpdTag *id3Dup(char *file) return ret; } -MpdTag *apeDup(char *file) +struct mpd_tag *apeDup(char *file) { - MpdTag *ret = NULL; + struct mpd_tag *ret = NULL; FILE *fp; int tagCount; char *buffer = NULL; @@ -574,16 +574,16 @@ fail: return ret; } -MpdTag *newMpdTag(void) +struct mpd_tag *newMpdTag(void) { - MpdTag *ret = xmalloc(sizeof(MpdTag)); + struct mpd_tag *ret = xmalloc(sizeof(*ret)); ret->items = NULL; ret->time = -1; ret->numOfItems = 0; return ret; } -static void deleteItem(MpdTag * tag, int idx) +static void deleteItem(struct mpd_tag *tag, int idx) { assert(idx < tag->numOfItems); tag->numOfItems--; @@ -598,14 +598,14 @@ static void deleteItem(MpdTag * tag, int idx) if (tag->numOfItems > 0) { tag->items = xrealloc(tag->items, - tag->numOfItems * sizeof(MpdTagItem)); + tag->numOfItems * sizeof(*tag->items)); } else { free(tag->items); tag->items = NULL; } } -void clearItemsFromMpdTag(MpdTag * tag, enum tag_type type) +void clearItemsFromMpdTag(struct mpd_tag *tag, enum tag_type type) { int i; @@ -618,7 +618,7 @@ void clearItemsFromMpdTag(MpdTag * tag, enum tag_type type) } } -static void clearMpdTag(MpdTag * tag) +static void clearMpdTag(struct mpd_tag *tag) { int i; @@ -636,15 +636,15 @@ static void clearMpdTag(MpdTag * tag) tag->time = -1; } -void freeMpdTag(MpdTag * tag) +void freeMpdTag(struct mpd_tag *tag) { clearMpdTag(tag); free(tag); } -MpdTag *mpdTagDup(MpdTag * tag) +struct mpd_tag *mpdTagDup(struct mpd_tag *tag) { - MpdTag *ret; + struct mpd_tag *ret; int i; if (!tag) @@ -660,7 +660,7 @@ MpdTag *mpdTagDup(MpdTag * tag) return ret; } -int mpdTagsAreEqual(MpdTag * tag1, MpdTag * tag2) +int mpdTagsAreEqual(struct mpd_tag *tag1, struct mpd_tag *tag2) { int i; @@ -696,7 +696,7 @@ int mpdTagsAreEqual(MpdTag * tag1, MpdTag * tag2) } \ } -static void appendToTagItems(MpdTag * tag, enum tag_type type, +static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, const char *value, size_t len) { unsigned int i = tag->numOfItems; @@ -709,7 +709,8 @@ static void appendToTagItems(MpdTag * tag, enum tag_type type, stripReturnChar(duplicated); tag->numOfItems++; - tag->items = xrealloc(tag->items, tag->numOfItems * sizeof(MpdTagItem)); + tag->items = xrealloc(tag->items, + tag->numOfItems * sizeof(*tag->items)); tag->items[i].type = type; tag->items[i].value = getTagItemString(type, duplicated); @@ -717,7 +718,7 @@ static void appendToTagItems(MpdTag * tag, enum tag_type type, free(duplicated); } -void addItemToMpdTagWithLen(MpdTag * tag, enum tag_type itemType, +void addItemToMpdTagWithLen(struct mpd_tag *tag, enum tag_type itemType, const char *value, size_t len) { if (ignoreTagItems[itemType]) -- cgit v1.2.3 From 95451b5821da1383f476cd8d6c6c8d12e683b777 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:21 +0200 Subject: tag: renamed functions, no CamelCase --- src/tag.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index b50756036..8c9fddf91 100644 --- a/src/tag.c +++ b/src/tag.c @@ -55,7 +55,7 @@ const char *mpdTagItemKeys[TAG_NUM_OF_ITEM_TYPES] = { static mpd_sint8 ignoreTagItems[TAG_NUM_OF_ITEM_TYPES]; -void initTagConfig(void) +void tag_lib_init(void) { int quit = 0; char *temp; @@ -104,7 +104,7 @@ void initTagConfig(void) free(temp); } -void printTagTypes(int fd) +void tag_print_types(int fd) { int i; @@ -114,7 +114,7 @@ void printTagTypes(int fd) } } -void printMpdTag(int fd, struct mpd_tag *tag) +void tag_print(int fd, struct mpd_tag *tag) { int i; @@ -224,8 +224,8 @@ static struct mpd_tag *getID3Info( continue; if (mpdTag == NULL) - mpdTag = newMpdTag(); - addItemToMpdTag(mpdTag, type, (char *)utf8); + mpdTag = tag_new(); + tag_add_item(mpdTag, type, (char *)utf8); free(utf8); } } @@ -256,8 +256,8 @@ static struct mpd_tag *getID3Info( if(utf8) { if (mpdTag == NULL) - mpdTag = newMpdTag(); - addItemToMpdTag(mpdTag, type, (char *)utf8); + mpdTag = tag_new(); + tag_add_item(mpdTag, type, (char *)utf8); free(utf8); } } @@ -283,7 +283,7 @@ static struct mpd_tag *getID3Info( #endif #ifdef HAVE_ID3TAG -struct mpd_tag *parseId3Tag(struct id3_tag * tag) +struct mpd_tag *tag_id3_import(struct id3_tag * tag) { struct mpd_tag *ret = NULL; @@ -425,7 +425,7 @@ static struct id3_tag *findId3TagFromEnd(FILE * stream) } #endif -struct mpd_tag *id3Dup(char *file) +struct mpd_tag *tag_id3_load(char *file) { struct mpd_tag *ret = NULL; #ifdef HAVE_ID3TAG @@ -434,7 +434,7 @@ struct mpd_tag *id3Dup(char *file) stream = fopen(file, "r"); if (!stream) { - DEBUG("id3Dup: Failed to open file: '%s', %s\n", file, + DEBUG("tag_id3_load: Failed to open file: '%s', %s\n", file, strerror(errno)); return NULL; } @@ -447,13 +447,13 @@ struct mpd_tag *id3Dup(char *file) if (!tag) return NULL; - ret = parseId3Tag(tag); + ret = tag_id3_import(tag); id3_tag_delete(tag); #endif return ret; } -struct mpd_tag *apeDup(char *file) +struct mpd_tag *tag_ape_load(char *file) { struct mpd_tag *ret = NULL; FILE *fp; @@ -556,9 +556,9 @@ struct mpd_tag *apeDup(char *file) for (i = 0; i < 7; i++) { if (strcasecmp(key, apeItems[i]) == 0) { if (!ret) - ret = newMpdTag(); - addItemToMpdTagWithLen(ret, tagItems[i], - p, size); + ret = tag_new(); + tag_add_item_n(ret, tagItems[i], + p, size); } } } @@ -574,7 +574,7 @@ fail: return ret; } -struct mpd_tag *newMpdTag(void) +struct mpd_tag *tag_new(void) { struct mpd_tag *ret = xmalloc(sizeof(*ret)); ret->items = NULL; @@ -605,7 +605,7 @@ static void deleteItem(struct mpd_tag *tag, int idx) } } -void clearItemsFromMpdTag(struct mpd_tag *tag, enum tag_type type) +void tag_clear_items_by_type(struct mpd_tag *tag, enum tag_type type) { int i; @@ -636,13 +636,13 @@ static void clearMpdTag(struct mpd_tag *tag) tag->time = -1; } -void freeMpdTag(struct mpd_tag *tag) +void tag_free(struct mpd_tag *tag) { clearMpdTag(tag); free(tag); } -struct mpd_tag *mpdTagDup(struct mpd_tag *tag) +struct mpd_tag *tag_dup(struct mpd_tag *tag) { struct mpd_tag *ret; int i; @@ -650,17 +650,17 @@ struct mpd_tag *mpdTagDup(struct mpd_tag *tag) if (!tag) return NULL; - ret = newMpdTag(); + ret = tag_new(); ret->time = tag->time; for (i = 0; i < tag->numOfItems; i++) { - addItemToMpdTag(ret, tag->items[i].type, tag->items[i].value); + tag_add_item(ret, tag->items[i].type, tag->items[i].value); } return ret; } -int mpdTagsAreEqual(struct mpd_tag *tag1, struct mpd_tag *tag2) +int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) { int i; @@ -718,8 +718,8 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, free(duplicated); } -void addItemToMpdTagWithLen(struct mpd_tag *tag, enum tag_type itemType, - const char *value, size_t len) +void tag_add_item_n(struct mpd_tag *tag, enum tag_type itemType, + const char *value, size_t len) { if (ignoreTagItems[itemType]) { -- cgit v1.2.3 From 323835240107ba08c080bbd8ed7aebe742fc4475 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:27 +0200 Subject: tag: moved code to tag_id3.c The ID3 code uses only the public tag API, but is otherwise unrelated. Move it to a separate source file. --- src/tag.c | 340 -------------------------------------------------------------- 1 file changed, 340 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 8c9fddf91..d06c6c31e 100644 --- a/src/tag.c +++ b/src/tag.c @@ -22,23 +22,9 @@ #include "utf8.h" #include "log.h" #include "conf.h" -#include "charConv.h" #include "tagTracker.h" #include "song.h" -#ifdef HAVE_ID3TAG -# define isId3v1(tag) (id3_tag_options(tag, 0, 0) & ID3_TAG_OPTION_ID3V1) -# ifndef ID3_FRAME_COMPOSER -# define ID3_FRAME_COMPOSER "TCOM" -# endif -# ifndef ID3_FRAME_PERFORMER -# define ID3_FRAME_PERFORMER "TOPE" -# endif -# ifndef ID3_FRAME_DISC -# define ID3_FRAME_DISC "TPOS" -# endif -#endif - const char *mpdTagItemKeys[TAG_NUM_OF_ITEM_TYPES] = { "Artist", "Album", @@ -127,332 +113,6 @@ void tag_print(int fd, struct mpd_tag *tag) } } -#ifdef HAVE_ID3TAG -/* This will try to convert a string to utf-8, - */ -static id3_utf8_t * processID3FieldString (int is_id3v1, const id3_ucs4_t *ucs4, int type) -{ - id3_utf8_t *utf8; - id3_latin1_t *isostr; - char *encoding; - - if (type == TAG_ITEM_GENRE) - ucs4 = id3_genre_name(ucs4); - /* use encoding field here? */ - if (is_id3v1 && - (encoding = getConfigParamValue(CONF_ID3V1_ENCODING))) { - isostr = id3_ucs4_latin1duplicate(ucs4); - if (mpd_unlikely(!isostr)) { - return NULL; - } - setCharSetConversion("UTF-8", encoding); - utf8 = xmalloc(strlen((char *)isostr) + 1); - utf8 = (id3_utf8_t *)char_conv_str((char *)utf8, (char *)isostr); - if (!utf8) { - DEBUG("Unable to convert %s string to UTF-8: " - "'%s'\n", encoding, isostr); - free(isostr); - return NULL; - } - free(isostr); - } else { - utf8 = id3_ucs4_utf8duplicate(ucs4); - if (mpd_unlikely(!utf8)) { - return NULL; - } - } - return utf8; -} - -static struct mpd_tag *getID3Info( - struct id3_tag *tag, const char *id, int type, struct mpd_tag *mpdTag) -{ - struct id3_frame const *frame; - id3_ucs4_t const *ucs4; - id3_utf8_t *utf8; - union id3_field const *field; - unsigned int nstrings, i; - - frame = id3_tag_findframe(tag, id, 0); - /* Check frame */ - if (!frame) - { - return mpdTag; - } - /* Check fields in frame */ - if(frame->nfields == 0) - { - DEBUG(__FILE__": Frame has no fields\n"); - return mpdTag; - } - - /* Starting with T is a stringlist */ - if (id[0] == 'T') - { - /* This one contains 2 fields: - * 1st: Text encoding - * 2: Stringlist - * Shamefully this isn't the RL case. - * But I am going to enforce it anyway. - */ - if(frame->nfields != 2) - { - DEBUG(__FILE__": Invalid number '%i' of fields for TXX frame\n",frame->nfields); - return mpdTag; - } - field = &frame->fields[0]; - /** - * First field is encoding field. - * This is ignored by mpd. - */ - if(field->type != ID3_FIELD_TYPE_TEXTENCODING) - { - DEBUG(__FILE__": Expected encoding, found: %i\n",field->type); - } - /* Process remaining fields, should be only one */ - field = &frame->fields[1]; - /* Encoding field */ - if(field->type == ID3_FIELD_TYPE_STRINGLIST) { - /* Get the number of strings available */ - nstrings = id3_field_getnstrings(field); - for (i = 0; i < nstrings; i++) { - ucs4 = id3_field_getstrings(field,i); - if(!ucs4) - continue; - utf8 = processID3FieldString(isId3v1(tag),ucs4, type); - if(!utf8) - continue; - - if (mpdTag == NULL) - mpdTag = tag_new(); - tag_add_item(mpdTag, type, (char *)utf8); - free(utf8); - } - } - else { - ERROR(__FILE__": Field type not processed: %i\n",(int)id3_field_gettextencoding(field)); - } - } - /* A comment frame */ - else if(!strcmp(ID3_FRAME_COMMENT, id)) - { - /* A comment frame is different... */ - /* 1st: encoding - * 2nd: Language - * 3rd: String - * 4th: FullString. - * The 'value' we want is in the 4th field - */ - if(frame->nfields == 4) - { - /* for now I only read the 4th field, with the fullstring */ - field = &frame->fields[3]; - if(field->type == ID3_FIELD_TYPE_STRINGFULL) - { - ucs4 = id3_field_getfullstring(field); - if(ucs4) - { - utf8 = processID3FieldString(isId3v1(tag),ucs4, type); - if(utf8) - { - if (mpdTag == NULL) - mpdTag = tag_new(); - tag_add_item(mpdTag, type, (char *)utf8); - free(utf8); - } - } - } - else - { - DEBUG(__FILE__": 4th field in comment frame differs from expected, got '%i': ignoring\n",field->type); - } - } - else - { - DEBUG(__FILE__": Invalid 'comments' tag, got '%i' fields instead of 4\n", frame->nfields); - } - } - /* Unsupported */ - else { - DEBUG(__FILE__": Unsupported tag type requrested\n"); - return mpdTag; - } - - return mpdTag; -} -#endif - -#ifdef HAVE_ID3TAG -struct mpd_tag *tag_id3_import(struct id3_tag * tag) -{ - struct mpd_tag *ret = NULL; - - ret = getID3Info(tag, ID3_FRAME_ARTIST, TAG_ITEM_ARTIST, ret); - ret = getID3Info(tag, ID3_FRAME_TITLE, TAG_ITEM_TITLE, ret); - ret = getID3Info(tag, ID3_FRAME_ALBUM, TAG_ITEM_ALBUM, ret); - ret = getID3Info(tag, ID3_FRAME_TRACK, TAG_ITEM_TRACK, ret); - ret = getID3Info(tag, ID3_FRAME_YEAR, TAG_ITEM_DATE, ret); - ret = getID3Info(tag, ID3_FRAME_GENRE, TAG_ITEM_GENRE, ret); - ret = getID3Info(tag, ID3_FRAME_COMPOSER, TAG_ITEM_COMPOSER, ret); - ret = getID3Info(tag, ID3_FRAME_PERFORMER, TAG_ITEM_PERFORMER, ret); - ret = getID3Info(tag, ID3_FRAME_COMMENT, TAG_ITEM_COMMENT, ret); - ret = getID3Info(tag, ID3_FRAME_DISC, TAG_ITEM_DISC, ret); - - return ret; -} -#endif - -#ifdef HAVE_ID3TAG -static int fillBuffer(void *buf, size_t size, FILE * stream, - long offset, int whence) -{ - if (fseek(stream, offset, whence) != 0) return 0; - return fread(buf, 1, size, stream); -} -#endif - -#ifdef HAVE_ID3TAG -static int getId3v2FooterSize(FILE * stream, long offset, int whence) -{ - id3_byte_t buf[ID3_TAG_QUERYSIZE]; - int bufsize; - - bufsize = fillBuffer(buf, ID3_TAG_QUERYSIZE, stream, offset, whence); - if (bufsize <= 0) return 0; - return id3_tag_query(buf, bufsize); -} -#endif - -#ifdef HAVE_ID3TAG -static struct id3_tag *getId3Tag(FILE * stream, long offset, int whence) -{ - struct id3_tag *tag; - id3_byte_t queryBuf[ID3_TAG_QUERYSIZE]; - id3_byte_t *tagBuf; - int tagSize; - int queryBufSize; - int tagBufSize; - - /* It's ok if we get less than we asked for */ - queryBufSize = fillBuffer(queryBuf, ID3_TAG_QUERYSIZE, - stream, offset, whence); - if (queryBufSize <= 0) return NULL; - - /* Look for a tag header */ - tagSize = id3_tag_query(queryBuf, queryBufSize); - if (tagSize <= 0) return NULL; - - /* Found a tag. Allocate a buffer and read it in. */ - tagBuf = xmalloc(tagSize); - if (!tagBuf) return NULL; - - tagBufSize = fillBuffer(tagBuf, tagSize, stream, offset, whence); - if (tagBufSize < tagSize) { - free(tagBuf); - return NULL; - } - - tag = id3_tag_parse(tagBuf, tagBufSize); - - free(tagBuf); - - return tag; -} -#endif - -#ifdef HAVE_ID3TAG -static struct id3_tag *findId3TagFromBeginning(FILE * stream) -{ - struct id3_tag *tag; - struct id3_tag *seektag; - struct id3_frame *frame; - int seek; - - tag = getId3Tag(stream, 0, SEEK_SET); - if (!tag) { - return NULL; - } else if (isId3v1(tag)) { - /* id3v1 tags don't belong here */ - id3_tag_delete(tag); - return NULL; - } - - /* We have an id3v2 tag, so let's look for SEEK frames */ - while ((frame = id3_tag_findframe(tag, "SEEK", 0))) { - /* Found a SEEK frame, get it's value */ - seek = id3_field_getint(id3_frame_field(frame, 0)); - if (seek < 0) - break; - - /* Get the tag specified by the SEEK frame */ - seektag = getId3Tag(stream, seek, SEEK_CUR); - if (!seektag || isId3v1(seektag)) - break; - - /* Replace the old tag with the new one */ - id3_tag_delete(tag); - tag = seektag; - } - - return tag; -} -#endif - -#ifdef HAVE_ID3TAG -static struct id3_tag *findId3TagFromEnd(FILE * stream) -{ - struct id3_tag *tag; - struct id3_tag *v1tag; - int tagsize; - - /* Get an id3v1 tag from the end of file for later use */ - v1tag = getId3Tag(stream, -128, SEEK_END); - - /* Get the id3v2 tag size from the footer (located before v1tag) */ - tagsize = getId3v2FooterSize(stream, (v1tag ? -128 : 0) - 10, SEEK_END); - if (tagsize >= 0) - return v1tag; - - /* Get the tag which the footer belongs to */ - tag = getId3Tag(stream, tagsize, SEEK_CUR); - if (!tag) - return v1tag; - - /* We have an id3v2 tag, so ditch v1tag */ - id3_tag_delete(v1tag); - - return tag; -} -#endif - -struct mpd_tag *tag_id3_load(char *file) -{ - struct mpd_tag *ret = NULL; -#ifdef HAVE_ID3TAG - struct id3_tag *tag; - FILE *stream; - - stream = fopen(file, "r"); - if (!stream) { - DEBUG("tag_id3_load: Failed to open file: '%s', %s\n", file, - strerror(errno)); - return NULL; - } - - tag = findId3TagFromBeginning(stream); - if (!tag) - tag = findId3TagFromEnd(stream); - - fclose(stream); - - if (!tag) - return NULL; - ret = tag_id3_import(tag); - id3_tag_delete(tag); -#endif - return ret; -} - struct mpd_tag *tag_ape_load(char *file) { struct mpd_tag *ret = NULL; -- cgit v1.2.3 From 6aa1a7a2565bd1789fd34c7c71d5dbb34eaa1fee Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:29 +0200 Subject: tag: converted MpdTag.items to a pointer list This prepares the following patches, which aim to reduce MPD's memory usage: we plan to share tag_item instances, instead of just their values. --- src/tag.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index d06c6c31e..311b49d50 100644 --- a/src/tag.c +++ b/src/tag.c @@ -108,8 +108,8 @@ void tag_print(int fd, struct mpd_tag *tag) fdprintf(fd, SONG_TIME "%i\n", tag->time); for (i = 0; i < tag->numOfItems; i++) { - fdprintf(fd, "%s: %s\n", mpdTagItemKeys[tag->items[i].type], - tag->items[i].value); + fdprintf(fd, "%s: %s\n", mpdTagItemKeys[tag->items[i]->type], + tag->items[i]->value); } } @@ -248,7 +248,8 @@ static void deleteItem(struct mpd_tag *tag, int idx) assert(idx < tag->numOfItems); tag->numOfItems--; - removeTagItemString(tag->items[idx].type, tag->items[idx].value); + removeTagItemString(tag->items[idx]->type, tag->items[idx]->value); + free(tag->items[idx]); /* free(tag->items[idx].value); */ if (tag->numOfItems - idx > 0) { @@ -270,7 +271,7 @@ void tag_clear_items_by_type(struct mpd_tag *tag, enum tag_type type) int i; for (i = 0; i < tag->numOfItems; i++) { - if (tag->items[i].type == type) { + if (tag->items[i]->type == type) { deleteItem(tag, i); /* decrement since when just deleted this node */ i--; @@ -283,8 +284,9 @@ static void clearMpdTag(struct mpd_tag *tag) int i; for (i = 0; i < tag->numOfItems; i++) { - removeTagItemString(tag->items[i].type, tag->items[i].value); + removeTagItemString(tag->items[i]->type, tag->items[i]->value); /* free(tag->items[i].value); */ + free(tag->items[i]); } if (tag->items) @@ -314,7 +316,7 @@ struct mpd_tag *tag_dup(struct mpd_tag *tag) ret->time = tag->time; for (i = 0; i < tag->numOfItems; i++) { - tag_add_item(ret, tag->items[i].type, tag->items[i].value); + tag_add_item(ret, tag->items[i]->type, tag->items[i]->value); } return ret; @@ -336,9 +338,9 @@ int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) return 0; for (i = 0; i < tag1->numOfItems; i++) { - if (tag1->items[i].type != tag2->items[i].type) + if (tag1->items[i]->type != tag2->items[i]->type) return 0; - if (strcmp(tag1->items[i].value, tag2->items[i].value)) { + if (strcmp(tag1->items[i]->value, tag2->items[i]->value)) { return 0; } } @@ -372,8 +374,9 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, tag->items = xrealloc(tag->items, tag->numOfItems * sizeof(*tag->items)); - tag->items[i].type = type; - tag->items[i].value = getTagItemString(type, duplicated); + tag->items[i] = xmalloc(sizeof(*tag->items[i])); + tag->items[i]->type = type; + tag->items[i]->value = getTagItemString(type, duplicated); free(duplicated); } -- cgit v1.2.3 From b24dbaef8dc73ccff85ae3948ff508c1cab4ede9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:33 +0200 Subject: tag: converted tag_item.value to a char array The value is stored in the same memory allocation as the tag_item struct; this saves memory because we do not store the value pointer anymore. Also remove the getTagItemString()/removeTagItemString() dummies. --- src/tag.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 311b49d50..2a4b27488 100644 --- a/src/tag.c +++ b/src/tag.c @@ -248,7 +248,6 @@ static void deleteItem(struct mpd_tag *tag, int idx) assert(idx < tag->numOfItems); tag->numOfItems--; - removeTagItemString(tag->items[idx]->type, tag->items[idx]->value); free(tag->items[idx]); /* free(tag->items[idx].value); */ @@ -284,7 +283,6 @@ static void clearMpdTag(struct mpd_tag *tag) int i; for (i = 0; i < tag->numOfItems; i++) { - removeTagItemString(tag->items[i]->type, tag->items[i]->value); /* free(tag->items[i].value); */ free(tag->items[i]); } @@ -374,9 +372,10 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, tag->items = xrealloc(tag->items, tag->numOfItems * sizeof(*tag->items)); - tag->items[i] = xmalloc(sizeof(*tag->items[i])); + len = strlen(duplicated); + tag->items[i] = xmalloc(sizeof(*tag->items[i]) + len); tag->items[i]->type = type; - tag->items[i]->value = getTagItemString(type, duplicated); + memcpy(tag->items[i]->value, duplicated, len + 1); free(duplicated); } -- cgit v1.2.3 From 318a284906f0e594f7c70c6fe72f234923fb6153 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:37 +0200 Subject: tag: added a pool for tag items The new source tag_pool.c manages a pool of reference counted tag_item objects. This is used to merge tag items of the same type and value, saving lots of memory. Formerly, only the value itself was pooled, wasting memory for all the pointers and tag_item structs. The following results were measured with massif. Started MPD on amd64, typed "mpc", no song being played. My music database contains 35k tagged songs. The results are what massif reports as "peak". 0.13.2: total 14,131,392; useful 11,408,972; extra 2,722,420 eric: total 18,370,696; useful 15,648,182; extra 2,722,514 mk f34f694: total 15,833,952; useful 13,111,470; extra 2,722,482 mk now: total 12,837,632; useful 10,626,383; extra 2,211,249 This patch set saves 20% memory, and does a good job in reducing heap fragmentation. --- src/tag.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 2a4b27488..8cc27b551 100644 --- a/src/tag.c +++ b/src/tag.c @@ -17,6 +17,7 @@ */ #include "tag.h" +#include "tag_pool.h" #include "myfprintf.h" #include "utils.h" #include "utf8.h" @@ -248,7 +249,7 @@ static void deleteItem(struct mpd_tag *tag, int idx) assert(idx < tag->numOfItems); tag->numOfItems--; - free(tag->items[idx]); + tag_pool_put_item(tag->items[idx]); /* free(tag->items[idx].value); */ if (tag->numOfItems - idx > 0) { @@ -284,7 +285,7 @@ static void clearMpdTag(struct mpd_tag *tag) for (i = 0; i < tag->numOfItems; i++) { /* free(tag->items[i].value); */ - free(tag->items[i]); + tag_pool_put_item(tag->items[i]); } if (tag->items) @@ -373,9 +374,7 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, tag->numOfItems * sizeof(*tag->items)); len = strlen(duplicated); - tag->items[i] = xmalloc(sizeof(*tag->items[i]) + len); - tag->items[i]->type = type; - memcpy(tag->items[i]->value, duplicated, len + 1); + tag->items[i] = tag_pool_get_item(type, duplicated, len); free(duplicated); } -- cgit v1.2.3 From bbafa24ba65878780101191b9313e9e70208937e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:54 +0200 Subject: tag: converted macro fixUtf8() to an inline function Since the inline function cannot modify its caller's variables (which is a good thing for code readability), the new string pointer is the return value. The resulting binary should be the same as with the macro. --- src/tag.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 8cc27b551..8b80bc308 100644 --- a/src/tag.c +++ b/src/tag.c @@ -347,14 +347,16 @@ int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) return 1; } -#define fixUtf8(str) { \ - if(str && !validUtf8String(str)) { \ - char * temp; \ - DEBUG("not valid utf8 in tag: %s\n",str); \ - temp = latin1StrToUtf8Dup(str); \ - free(str); \ - str = temp; \ - } \ +static inline char *fix_utf8(char *str) { + char *temp; + + if (str != NULL && validUtf8String(str)) + return str; + + DEBUG("not valid utf8 in tag: %s\n",str); + temp = latin1StrToUtf8Dup(str); + free(str); + return temp; } static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, @@ -366,7 +368,7 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, memcpy(duplicated, value, len); duplicated[len] = '\0'; - fixUtf8(duplicated); + duplicated = fix_utf8(duplicated); stripReturnChar(duplicated); tag->numOfItems++; -- cgit v1.2.3 From e3805dafb34a6fb3ae0421bd0414ee7fd27e2106 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:56 +0200 Subject: assert value!=NULL in fix_utf8() We must never pass value==NULL to fix_utf(). Replace the run-time check with an assertion. --- src/tag.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 8b80bc308..927f1b789 100644 --- a/src/tag.c +++ b/src/tag.c @@ -350,7 +350,9 @@ int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) static inline char *fix_utf8(char *str) { char *temp; - if (str != NULL && validUtf8String(str)) + assert(str != NULL); + + if (validUtf8String(str)) return str; DEBUG("not valid utf8 in tag: %s\n",str); -- cgit v1.2.3 From 07c4a01f9fa55b620c9bf87a4e64e45de22b7e5f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:38:58 +0200 Subject: added "length" parameter to validUtf8String() At several places, we create temporary copies of non-null-terminated strings, just to use them in functions like validUtf8String(). We can save this temporary allocation and avoid heap fragmentation if we add a length parameter instead of expecting a null-terminated string. --- src/tag.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 927f1b789..b93456641 100644 --- a/src/tag.c +++ b/src/tag.c @@ -352,7 +352,7 @@ static inline char *fix_utf8(char *str) { assert(str != NULL); - if (validUtf8String(str)) + if (validUtf8String(str, strlen(str))) return str; DEBUG("not valid utf8 in tag: %s\n",str); -- cgit v1.2.3 From 55e297674f35a90ea9f2deeda2267e8520f1f414 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:39:01 +0200 Subject: tag: pass length to fix_utf8() Same as the previous patch, prepare the function fix_utf8() this time. --- src/tag.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index b93456641..7bbf27135 100644 --- a/src/tag.c +++ b/src/tag.c @@ -347,17 +347,18 @@ int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) return 1; } -static inline char *fix_utf8(char *str) { +static inline char *fix_utf8(char *str, size_t *length_r) { char *temp; assert(str != NULL); - if (validUtf8String(str, strlen(str))) + if (validUtf8String(str, *length_r)) return str; DEBUG("not valid utf8 in tag: %s\n",str); temp = latin1StrToUtf8Dup(str); free(str); + *length_r = strlen(temp); return temp; } @@ -370,7 +371,7 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, memcpy(duplicated, value, len); duplicated[len] = '\0'; - duplicated = fix_utf8(duplicated); + duplicated = fix_utf8(duplicated, &len); stripReturnChar(duplicated); tag->numOfItems++; -- cgit v1.2.3 From 82a37682c692896bbf74fbaa91f5da727e8e1b0e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:39:04 +0200 Subject: tag: try not to duplicate the input string Try to detect if the string needs Latin1-UTF8 conversion, or whitespace cleanup. If not, we don't need to allocate temporary memory, leading to decreased heap fragmentation. --- src/tag.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index 7bbf27135..e5d49df3e 100644 --- a/src/tag.c +++ b/src/tag.c @@ -347,8 +347,8 @@ int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) return 1; } -static inline char *fix_utf8(char *str, size_t *length_r) { - char *temp; +static inline const char *fix_utf8(const char *str, size_t *length_r) { + const char *temp; assert(str != NULL); @@ -357,7 +357,6 @@ static inline char *fix_utf8(char *str, size_t *length_r) { DEBUG("not valid utf8 in tag: %s\n",str); temp = latin1StrToUtf8Dup(str); - free(str); *length_r = strlen(temp); return temp; } @@ -366,22 +365,28 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, const char *value, size_t len) { unsigned int i = tag->numOfItems; - char *duplicated = xmalloc(len + 1); - - memcpy(duplicated, value, len); - duplicated[len] = '\0'; - - duplicated = fix_utf8(duplicated, &len); - stripReturnChar(duplicated); + const char *p; + + p = fix_utf8(value, &len); + if (memchr(p, '\n', len) != NULL) { + char *duplicated = xmalloc(len + 1); + memcpy(duplicated, p, len); + duplicated[len] = '\0'; + if (p != value) + free(deconst_ptr(p)); + + stripReturnChar(duplicated); + p = duplicated; + } tag->numOfItems++; tag->items = xrealloc(tag->items, tag->numOfItems * sizeof(*tag->items)); - len = strlen(duplicated); - tag->items[i] = tag_pool_get_item(type, duplicated, len); + tag->items[i] = tag_pool_get_item(type, p, len); - free(duplicated); + if (p != value) + free(deconst_ptr(p)); } void tag_add_item_n(struct mpd_tag *tag, enum tag_type itemType, -- cgit v1.2.3 From c82544458231694c197b0c967f6e2844e8612bc6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:39:08 +0200 Subject: tag: try not to reallocate tag.items in every add() call If many tag_items are added at once while the tag cache is being loaded, manage these items in a static fixed list, instead of reallocating the list with every newly created item. This reduces heap fragmentation. Massif results again: mk before: total 12,837,632; useful 10,626,383; extra 2,211,249 mk now: total 12,736,720; useful 10,626,383; extra 2,110,337 The "useful" value is the same since this patch only changes the way we allocate the same amount of memory, but heap fragmentation was reduced by 5%. --- src/tag.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index e5d49df3e..b0e3e8618 100644 --- a/src/tag.c +++ b/src/tag.c @@ -361,6 +361,53 @@ static inline const char *fix_utf8(const char *str, size_t *length_r) { return temp; } +/** + * Maximum number of items managed in the bulk list; if it is + * exceeded, we switch back to "normal" reallocation. + */ +#define BULK_MAX 64 + +static struct { +#ifndef NDEBUG + int busy; +#endif + struct tag_item *items[BULK_MAX]; +} bulk; + +void tag_begin_add(struct mpd_tag *tag) +{ + assert(!bulk.busy); + assert(tag != NULL); + assert(tag->items == NULL); + assert(tag->numOfItems == 0); + +#ifndef NDEBUG + bulk.busy = 1; +#endif + tag->items = bulk.items; +} + +void tag_end_add(struct mpd_tag *tag) +{ + if (tag->items == bulk.items) { + assert(tag->numOfItems <= BULK_MAX); + + if (tag->numOfItems > 0) { + /* copy the tag items from the bulk list over + to a new list (which fits exactly) */ + tag->items = xmalloc(tag->numOfItems * + sizeof(tag->items[0])); + memcpy(tag->items, bulk.items, + tag->numOfItems * sizeof(tag->items[0])); + } else + tag->items = NULL; + } + +#ifndef NDEBUG + bulk.busy = 0; +#endif +} + static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, const char *value, size_t len) { @@ -380,8 +427,19 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, } tag->numOfItems++; - tag->items = xrealloc(tag->items, - tag->numOfItems * sizeof(*tag->items)); + + if (tag->items != bulk.items) + /* bulk mode disabled */ + tag->items = xrealloc(tag->items, + tag->numOfItems * sizeof(*tag->items)); + else if (tag->numOfItems >= BULK_MAX) { + /* bulk list already full - switch back to non-bulk */ + assert(bulk.busy); + + tag->items = xmalloc(tag->numOfItems * sizeof(tag->items[0])); + memcpy(tag->items, bulk.items, + (tag->numOfItems - 1) * sizeof(tag->items[0])); + } tag->items[i] = tag_pool_get_item(type, p, len); -- cgit v1.2.3 From b8eca08893a5738d732355cd581bcfc26c4812ea Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 14:48:39 +0200 Subject: const pointers Yet another patch which converts pointer arguments to "const". --- src/tag.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index b0e3e8618..cd5e227c1 100644 --- a/src/tag.c +++ b/src/tag.c @@ -101,7 +101,7 @@ void tag_print_types(int fd) } } -void tag_print(int fd, struct mpd_tag *tag) +void tag_print(int fd, const struct mpd_tag *tag) { int i; @@ -114,7 +114,7 @@ void tag_print(int fd, struct mpd_tag *tag) } } -struct mpd_tag *tag_ape_load(char *file) +struct mpd_tag *tag_ape_load(const char *file) { struct mpd_tag *ret = NULL; FILE *fp; @@ -303,7 +303,7 @@ void tag_free(struct mpd_tag *tag) free(tag); } -struct mpd_tag *tag_dup(struct mpd_tag *tag) +struct mpd_tag *tag_dup(const struct mpd_tag *tag) { struct mpd_tag *ret; int i; @@ -321,7 +321,7 @@ struct mpd_tag *tag_dup(struct mpd_tag *tag) return ret; } -int tag_equal(struct mpd_tag *tag1, struct mpd_tag *tag2) +int tag_equal(const struct mpd_tag *tag1, const struct mpd_tag *tag2) { int i; -- cgit v1.2.3 From ad6fadbd47ccdf16477bd0b65cdca90c21c3041d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 15:04:49 +0200 Subject: tag: optimize tag_dup(), copy item references Don't call tag_pool_get_item() for duplicating tags, just increase the item's reference counter instead. --- src/tag.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index cd5e227c1..ee26e271d 100644 --- a/src/tag.c +++ b/src/tag.c @@ -313,9 +313,11 @@ struct mpd_tag *tag_dup(const struct mpd_tag *tag) ret = tag_new(); ret->time = tag->time; + ret->numOfItems = tag->numOfItems; + ret->items = xmalloc(ret->numOfItems * sizeof(ret->items[0])); for (i = 0; i < tag->numOfItems; i++) { - tag_add_item(ret, tag->items[i]->type, tag->items[i]->value); + ret->items[i] = tag_pool_dup_item(tag->items[i]); } return ret; -- cgit v1.2.3 From cdc9bb460e9536577d2747d51c76306a91b3d064 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 3 Sep 2008 02:14:52 -0700 Subject: tag: fix segfault on update clearMpdTag could be called on a tag that was still in a tag_begin_add transaction before tag_end_add is called. This was causing free() to attempt to operate on bulk.items; which is un-free()-able. Now instead we unmark the bulk.busy to avoid committing the tags to the heap only to be immediately freed. Additionally, we need to remember to call tag_end_add() when a song is updated before we NULL song->tag to avoid tripping an assertion the next time tag_begin_add() is called. --- src/tag.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) (limited to 'src/tag.c') diff --git a/src/tag.c b/src/tag.c index ee26e271d..0e0ff1ab7 100644 --- a/src/tag.c +++ b/src/tag.c @@ -26,6 +26,19 @@ #include "tagTracker.h" #include "song.h" +/** + * Maximum number of items managed in the bulk list; if it is + * exceeded, we switch back to "normal" reallocation. + */ +#define BULK_MAX 64 + +static struct { +#ifndef NDEBUG + int busy; +#endif + struct tag_item *items[BULK_MAX]; +} bulk; + const char *mpdTagItemKeys[TAG_NUM_OF_ITEM_TYPES] = { "Artist", "Album", @@ -288,8 +301,15 @@ static void clearMpdTag(struct mpd_tag *tag) tag_pool_put_item(tag->items[i]); } - if (tag->items) + if (tag->items == bulk.items) { +#ifndef NDEBUG + assert(bulk.busy); + bulk.busy = 0; +#endif + } else if (tag->items) { free(tag->items); + } + tag->items = NULL; tag->numOfItems = 0; @@ -363,19 +383,6 @@ static inline const char *fix_utf8(const char *str, size_t *length_r) { return temp; } -/** - * Maximum number of items managed in the bulk list; if it is - * exceeded, we switch back to "normal" reallocation. - */ -#define BULK_MAX 64 - -static struct { -#ifndef NDEBUG - int busy; -#endif - struct tag_item *items[BULK_MAX]; -} bulk; - void tag_begin_add(struct mpd_tag *tag) { assert(!bulk.busy); -- cgit v1.2.3