2016-09-25 02:14 CEST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0001580MPDCommandspublic2009-06-20 22:44
Reporterdaekharel 
Assigned Tocirrus 
PrioritynormalSeverityfeatureReproducibilityalways
StatusassignedResolutionopen 
Product Version 
Target Version0.16.xFixed in Version 
Summary0001580: [PATCH] add complex/regex searching to mpd
DescriptionBecause I use this feature a lot in my current music player, but would like to switch to mpd, I added complex and regular-expression-based searching to mpd, via a new command, "complex_search". The syntax is a bit strange, because I wanted to make the parser as small as possible (currently it's a single 114-line recursive function). I'd really like to see this feature make it into mainline mpd, so if in order to achieve this the command needs new syntax, I'd be happy to do that for you, although I'd feel happier splitting it out into a separate file in that case.

Also, this program relies on POSIX "regex.h" to compile and match regular expressions. I don't know how unportable this may or may not make this program. It compiled and ran without modification to the autoconf magic (which I know nothing about) on the system I created it on, which is x86 Linux (Red-Hat based).

The syntax is as follows:

The equivalent of "search <tagname> <value>" is "complex_search :tag <tagname> <value". To do a regex search, do "complex_search :tag-regex <tagname> <regex>". Regex syntax is POSIX extended regular expressions. Tag can also be "file", "filename" or "any", just like in a regular search.

To 'AND' multiple queries, you do ":all <query> <query> ... !". To 'OR' multiple queries, you do ":any <query> <query> ... !". To 'NOT' multiple queries, do ":none <query> <query> ... !". Multiple exclamation points must each occupy their own argument; you cannot do ":all :tag foo bar :any :tag this that :tag xyzzy plugh !!". What you CAN do, however, is put a number after an exclamation point to indicate how many higher-level queries it ends. For example:

"complex_search :all :none :tag artist "bob dylan" ! :any :tag genre folk :tag genre jazz :tag genre blues !2"

This would search for all songs in the genres "folk", "jazz", or "blues" by artists not named "bob dylan" (ignoring case).

Attached is patch including my changes (produced by "diff -r").
TagsNo tags attached.
Attached Files
  • patch file icon mpd-search.patch (8,402 bytes) 2007-09-25 22:03 -
    diff -r mpd-0.13.0/src/command.c mpd-hacked/src/command.c
    103a104
    > #define COMMAND_COMPLEXSEARCH   "complex_search"
    531a533,549
    > static int handleComplexSearch(int fd, int *permission, int argc, char *argv[])
    > {
    > 	LocateQuery *query = newLocateQueryFromArgArray(argv + 1, argc - 1);
    > 
    > 	if (query == NULL) {
    > 		commandError(fd, ACK_ERROR_ARG, "incorrect arguments");
    > 		return -1;
    > 	}
    > 
    > 	int ret = complexSearchForSongsIn(fd, NULL, query);
    > 
    > 	freeLocateQuery(query);
    > 	
    > 	return ret;
    > }
    > 
    > 	
    1126a1145
    > 	addCommand(COMMAND_COMPLEXSEARCH,    PERMISSION_READ,    2,   -1,  handleComplexSearch,        NULL);
    diff -r mpd-0.13.0/src/dbUtils.c mpd-hacked/src/dbUtils.c
    109a110,121
    > static int complexSearchInDirectory(int fd, Song * song, void *data)
    > {
    > 	if (locateQueryMatches(song, (LocateQuery *) data))
    > 		printSongInfo(fd, song);
    > 	
    > 	return 0;
    > }
    > 
    > int complexSearchForSongsIn(int fd, char *name, LocateQuery *query) {
    > 	return traverseAllIn(fd, name, complexSearchInDirectory, NULL, query);
    > }
    > 
    diff -r mpd-0.13.0/src/dbUtils.h mpd-hacked/src/dbUtils.h
    36a37,38
    > int complexSearchForSongsIn(int fd, char *name, LocateQuery *query);
    > 
    diff -r mpd-0.13.0/src/locate.c mpd-hacked/src/locate.c
    26a27,32
    > #define LOCATE_SYNTAX_ERROR  -1
    > #define LOCATE_SUCCESS        0
    > 
    > #define LOCATE_ASK_QUERY      0
    > #define LOCATE_ASK_STRING     1
    > 
    156a163,190
    > static int regexSearchTag(Song * song, int type, regex_t *regex) {
    > 	int ret = 0;
    > 	int i;
    > 
    > 	/* zero -> success for regexec */
    > 	
    > 	if (type == LOCATE_TAG_FILE_TYPE || type == LOCATE_TAG_ANY_TYPE) {
    > 		if (!regexec(regex, getSongUrl(song), 0, NULL, 0))
    > 			ret = 1;
    > 		if (ret == 1 || type == LOCATE_TAG_FILE_TYPE)
    > 			return ret;
    > 	}
    > 
    > 	if (!song->tag) return 0;
    > 
    > 	for (i = 0; i < song->tag->numOfItems && !ret; i++) {
    > 		if (type != LOCATE_TAG_ANY_TYPE &&
    > 		    song->tag->items[i].type != type) {
    > 			continue;
    > 		}
    > 
    > 		if (!regexec(regex, song->tag->items[i].value, 0, NULL, 0))
    > 			ret = 1;
    > 	}
    > 	
    > 	return ret;
    > }
    > 
    211a246,439
    > 
    > static void freeLocateQueryList(LocateQueryList *list) {
    > 	LocateQueryList *temp;
    > 	while (list) {
    > 		temp = list->next;
    > 		
    > 		switch (list->query.queryType) {
    > 		default:
    > 			freeLocateQueryList(list->query.value.list);
    > 			break;
    > 			
    > 		case LOCATE_QUERY_NORMAL_TYPE:
    > 			free(list->query.value.tagItem.needle.string);
    > 			break;
    > 			
    > 		case LOCATE_QUERY_REGEX_TYPE:
    > 			regfree(&list->query.value.tagItem.needle.regex);
    > 			break;
    > 		}
    > 		free(list);
    > 		list = temp;
    > 	}
    > }
    > 
    > void freeLocateQuery(LocateQuery *query) {
    > 	switch (query->queryType) {
    > 	default:
    > 		freeLocateQueryList(query->value.list);
    > 		break;
    > 		
    > 	case LOCATE_QUERY_NORMAL_TYPE:
    > 		free(query->value.tagItem.needle.string);
    > 		break;
    > 		
    > 	case LOCATE_QUERY_REGEX_TYPE:
    > 		regfree(&query->value.tagItem.needle.regex);
    > 		break;
    > 	}
    > 	free(query);
    > }
    > 
    > static int _consumeArgs(int *argc, char **argv[], int ask, void *retPtr) {
    > 	char *str;
    > 	int ret;
    > 	LocateQuery *retQuery;
    > 	LocateQuery query;
    > 	LocateQueryList *iter;
    > 	
    > 	if ((*argc)-- == 0) return LOCATE_SYNTAX_ERROR;
    > 	str = *((*argv)++);
    > 	
    > 	switch (str[0]) {
    > 	case '\\':
    > 		/* ignore escaped, fall through */
    > 		if (str[1] == '!' || str[1] == ':')
    > 			++str;
    > 		
    > 	default:
    > 		if (ask != LOCATE_ASK_STRING)
    > 			return LOCATE_SYNTAX_ERROR;
    > 		*((char **) retPtr) = str;
    > 		return 0;
    > 
    > 	case '!':
    > 		if (ask != LOCATE_ASK_QUERY)
    > 			return LOCATE_SYNTAX_ERROR;
    > 		
    > 		((LocateQuery *) retPtr)->queryType = LOCATE_QUERY_NULL_TYPE;
    > 		if (str[1]) {
    > 			sscanf(str + 1, "%d", &ret);
    > 			if (ret <= 0) return LOCATE_SYNTAX_ERROR;
    > 			return ret;
    > 		} else return 1;
    > 		
    > 	case ':':
    > 		if (ask != LOCATE_ASK_QUERY)
    > 			return LOCATE_SYNTAX_ERROR;
    > 		retQuery = (LocateQuery *) retPtr;
    > 		++str;
    > 		
    > 		if (!strcmp(str, "tag"))
    > 			retQuery->queryType = LOCATE_QUERY_NORMAL_TYPE;
    > 		else if (!strcmp(str, "tag-regex"))
    > 			retQuery->queryType = LOCATE_QUERY_REGEX_TYPE;
    > 		else goto build_list;
    > 		
    > 		/* get the tag type */
    > 		ret = _consumeArgs(argc, argv, LOCATE_ASK_STRING, &str);
    > 		if (ret > 0) return LOCATE_SYNTAX_ERROR;
    > 		else if (ret) return ret;
    > 		retQuery->value.tagItem.tagType = getLocateTagItemType(str);
    > 		
    > 		/* get the tag value */
    > 		ret = _consumeArgs(argc, argv, LOCATE_ASK_STRING, &str);
    > 		if (retQuery->queryType == LOCATE_QUERY_NORMAL_TYPE)
    > 			retQuery->value.tagItem.needle.string =
    > 				strDupToUpper(str);
    > 		else
    > 			regcomp(&retQuery->value.tagItem.needle.regex,
    > 				str,
    > 				REG_EXTENDED | REG_ICASE | REG_NOSUB);
    > 		return LOCATE_SUCCESS;
    > 		
    > 	build_list:
    > 		if (!strcmp(str, "all"))
    > 			retQuery->queryType = LOCATE_QUERY_ALL_TYPE;
    > 		else if (!strcmp(str, "any"))
    > 			retQuery->queryType = LOCATE_QUERY_ANY_TYPE;
    > 		else if (!strcmp(str, "none"))
    > 			retQuery->queryType = LOCATE_QUERY_NONE_TYPE;
    > 		else return LOCATE_SYNTAX_ERROR;
    > 
    > 		retQuery->value.list = NULL;
    > 		
    > 		ret = _consumeArgs(argc, argv, LOCATE_ASK_QUERY, &query);
    > 		if (ret < 0) return ret;
    > 		else if (ret > 0) {
    > 			if (query.queryType != LOCATE_QUERY_NULL_TYPE) {
    > 				iter = retQuery->value.list = xmalloc(
    > 					sizeof(LocateQueryList));
    > 				iter->query = query;
    > 				iter->next = NULL;
    > 			}
    > 			return ret - 1;
    > 		}
    > 
    > 		iter = retQuery->value.list = xmalloc(sizeof(LocateQueryList));
    > 		iter->query = query;
    > 		
    > 		while (0 == (ret = _consumeArgs(argc, argv, LOCATE_ASK_QUERY,
    > 						&query))) {
    > 			iter->next = xmalloc(sizeof(LocateQueryList));
    > 			iter = iter->next;
    > 			iter->query = query;
    > 		}
    > 			
    > 		if (ret < 0) {
    > 			/* free memory and propagate error */
    > 			iter->next = NULL;
    > 			freeLocateQueryList(retQuery->value.list);
    > 			return ret;
    > 		}
    > 		
    > 		/* finish the list */
    > 		if (query.queryType != LOCATE_QUERY_NULL_TYPE) {
    > 			/* add it to the list */
    > 			iter->next = xmalloc(sizeof(LocateQueryList));
    > 			iter = iter->next;
    > 			iter->query = query;
    > 		}
    > 		iter->next = NULL;
    > 		return ret - 1;
    > 	}
    > }
    > 
    > LocateQuery *newLocateQueryFromArgArray(char *argArray[], int numArgs)
    > {
    > 	LocateQuery *query = xmalloc(sizeof(LocateQuery));
    > 	int res = _consumeArgs(&numArgs, &argArray, LOCATE_ASK_QUERY, query);
    > 	if (res) return NULL;
    > 	return query;
    > }
    > 
    > int locateQueryMatches(Song * song, LocateQuery *query) {
    > 	LocateQueryList *iter;
    > 	
    > 	switch (query->queryType) {
    > 	case LOCATE_QUERY_NORMAL_TYPE:
    > 		return strstrSearchTag(song, query->value.tagItem.tagType,
    > 				       query->value.tagItem.needle.string);
    > 		
    > 	case LOCATE_QUERY_REGEX_TYPE:
    > 		return regexSearchTag(song, query->value.tagItem.tagType,
    > 				      &query->value.tagItem.needle.regex);
    > 		
    > 	case LOCATE_QUERY_ALL_TYPE:
    > 		for (iter = query->value.list; iter; iter = iter->next)
    > 			if(!locateQueryMatches(song, &iter->query)) return 0;
    > 		return 1;
    > 		
    > 	case LOCATE_QUERY_ANY_TYPE:
    > 		for (iter = query->value.list; iter; iter = iter->next)
    > 			if (locateQueryMatches(song, &iter->query)) return 1;
    > 		return 0;
    > 		
    > 	case LOCATE_QUERY_NONE_TYPE:
    > 		for (iter = query->value.list; iter; iter = iter->next)
    > 			if (locateQueryMatches(song, &iter->query)) return 0;
    > 		return 1;
    > 		
    > 	default:
    > 		return -1;
    > 	}
    > }
    diff -r mpd-0.13.0/src/locate.h mpd-hacked/src/locate.h
    18a19,20
    > #include <regex.h>
    > 
    23a26,32
    > #define LOCATE_QUERY_NULL_TYPE    0
    > #define LOCATE_QUERY_NORMAL_TYPE  1
    > #define LOCATE_QUERY_REGEX_TYPE   2
    > #define LOCATE_QUERY_ALL_TYPE     3
    > #define LOCATE_QUERY_ANY_TYPE     4
    > #define LOCATE_QUERY_NONE_TYPE    5
    > 
    30a40,59
    > /* struct used for complex_search query */
    > struct _LocateQueryList;	/* forward declaration */
    > 
    > typedef struct _LocateQuery {
    > 	mpd_sint8 queryType;
    > 	union {
    > 		struct _LocateQueryList *list;
    > 		struct {
    > 			mpd_sint8 tagType;
    > 			union { char *string; regex_t regex; } needle;
    > 		} tagItem;
    > 	} value;
    > } LocateQuery;
    > 
    > typedef struct _LocateQueryList {
    > 	struct _LocateQueryList *next;
    > 	LocateQuery query;
    > } LocateQueryList;
    > 
    > 
    46a76,83
    > 
    > /* returns NULL on malformed query */
    > LocateQuery *newLocateQueryFromArgArray(char *argArray[], int numArgs);
    > 
    > /* return -1 on error, unspecified but > 0 otherwise */
    > int locateQueryMatches(Song * song, LocateQuery *query);
    > 
    > void freeLocateQuery(LocateQuery *query);
    
    patch file icon mpd-search.patch (8,402 bytes) 2007-09-25 22:03 +

-Relationships
+Relationships

-Notes

~0001650

qball (administrator)

This looks like a _very_ interesting patch.
But out of curiosity, what is the performance of the complex_search?

~0001834

qball (administrator)

Again, can you look at performance? does this still work with 100.000 songs?

~0002453

Avuton Olrich (administrator)

What would be even better then at this point, since glib is now ingrained in MPD if Glib/GRegex could be utilized for this purpose. The patch would probably be much smaller in that case, and much easier to accept.

~0003400

cirrus (administrator)

Moved to target version 0.16, due to time constraints.

~0003682

Misery (reporter)

My dynamic-playlist plugin for gmpc would benefit a lot from complex searches.

~0004016

Misery (reporter)

By the way... it would be nice if it would support stickers like
:stickers song rate 5
or if stickers used as blacklist
:none :stickers artist blocked true
+Notes

-Issue History
Date Modified Username Field Change
2007-09-25 22:03 daekharel New Issue
2007-09-25 22:03 daekharel Status new => assigned
2007-09-25 22:03 daekharel Assigned To => shank
2007-09-25 22:03 daekharel File Added: mpd-search.patch
2007-09-26 09:11 qball Note Added: 0001650
2007-12-06 17:14 qball Note Added: 0001834
2008-10-25 19:01 Avuton Olrich Assigned To shank => cirrus
2008-10-25 22:00 Avuton Olrich Category => Commands
2008-11-23 01:38 Avuton Olrich Note Added: 0002453
2009-02-25 10:42 cirrus Target Version => 0.15
2009-03-14 12:10 cirrus Target Version 0.15 => 0.16.x
2009-03-14 12:11 cirrus Note Added: 0003400
2009-04-26 01:18 Misery Note Added: 0003682
2009-06-20 22:44 Misery Note Added: 0004016
+Issue History