[mpd] Refactor parsing filter/window arguments and add support for

"group" argument in "list" command

src/mpd.c

@@ -59,6 +59,13 @@
 #include "remote_pairing.h"
 
 
+enum mpd_type {
+  MPD_TYPE_INT,
+  MPD_TYPE_STRING,
+  MPD_TYPE_SPECIAL,
+};
+
+
 #define MPD_ALL_IDLE_LISTENER_EVENTS (LISTENER_PLAYER | LISTENER_QUEUE | LISTENER_VOLUME | LISTENER_SPEAKER | LISTENER_OPTIONS | LISTENER_DATABASE | LISTENER_UPDATE | LISTENER_STORED_PLAYLIST | LISTENER_RATING)
 #define MPD_RATING_FACTOR 10.0
 
@@ -145,6 +152,65 @@ static const char * const ffmpeg_mime_types[] = { "application/flv", "applicatio
     NULL
 };
 
+struct mpd_tagtype
+{
+  char *tag;
+  char *field;
+  char *sort_field;
+  char *group_field;
+  enum mpd_type type;
+  ssize_t mfi_offset;
+
+  /*
+   * This allows omitting the "group" fields in the created group by clause to improve
+   * performance in the "list" command. For example listing albums and artists already
+   * groups by their persistent id, an additional group clause by artist/album will
+   * decrease performance of the select query and will in general not change the result
+   * (e. g. album persistent id is generated by artist and album and listing albums
+   * grouped by artist is therefor not necessary).
+   */
+  bool group_in_listcommand;
+};
+
+static struct mpd_tagtype tagtypes[] =
+  {
+    /* tag               | db field             | db sort field                        | db group field  | type             | media_file offset                | group_in_listcommand */
+
+    // We treat the artist tag as album artist, this allows grouping over the artist-persistent-id index and increases performance
+    // { "Artist",           "f.artist",             "f.artist",             "f.artist",             MPD_TYPE_STRING,   dbmfi_offsetof(artist), },
+    { "Artist",           "f.album_artist",       "f.album_artist_sort, f.album_artist", "f.songartistid", MPD_TYPE_STRING,   dbmfi_offsetof(album_artist),      false, },
+    { "ArtistSort",       "f.album_artist_sort",  "f.album_artist_sort, f.album_artist", "f.songartistid", MPD_TYPE_STRING,   dbmfi_offsetof(album_artist_sort), false, },
+    { "AlbumArtist",      "f.album_artist",       "f.album_artist_sort, f.album_artist", "f.songartistid", MPD_TYPE_STRING,   dbmfi_offsetof(album_artist),      false, },
+    { "AlbumArtistSort",  "f.album_artist_sort",  "f.album_artist_sort, f.album_artist", "f.songartistid", MPD_TYPE_STRING,   dbmfi_offsetof(album_artist_sort), false, },
+    { "Album",            "f.album",              "f.album_sort, f.album",               "f.songalbumid",  MPD_TYPE_STRING,   dbmfi_offsetof(album),             false, },
+    { "Title",            "f.title",              "f.title",                             "f.title",        MPD_TYPE_STRING,   dbmfi_offsetof(title),             true, },
+    { "Track",            "f.track",              "f.track",                             "f.track",        MPD_TYPE_INT,      dbmfi_offsetof(track),             true, },
+    { "Genre",            "f.genre",              "f.genre",                             "f.genre",        MPD_TYPE_STRING,   dbmfi_offsetof(genre),             true, },
+    { "Disc",             "f.disc",               "f.disc",                              "f.disc",         MPD_TYPE_INT,      dbmfi_offsetof(disc),              true, },
+    { "Date",             "f.year",               "f.year",                              "f.year",         MPD_TYPE_INT,      dbmfi_offsetof(year),              true, },
+    { "file",             NULL,                   NULL,                                  NULL,             MPD_TYPE_SPECIAL,  -1,                                true, },
+    { "base",             NULL,                   NULL,                                  NULL,             MPD_TYPE_SPECIAL,  -1,                                true, },
+    { "any",              NULL,                   NULL,                                  NULL,             MPD_TYPE_SPECIAL,  -1,                                true, },
+
+  };
+
+static struct mpd_tagtype *
+find_tagtype(const char *tag)
+{
+  int i;
+
+  if (!tag)
+    return 0;
+
+  for (i = 0; i < ARRAY_SIZE(tagtypes); i++)
+    {
+      if (strcasecmp(tag, tagtypes[i].tag) == 0)
+	return &tagtypes[i];
+    }
+
+  return NULL;
+}
+
 /*
  * MPD client connection data
  */
@@ -586,11 +652,34 @@ mpd_add_db_media_file_info(struct evbuffer *evbuf, struct db_media_file_info *db
   return ret;
 }
 
-static int
+static void
-mpd_get_query_params_find(int argc, char **argv, struct query_params *qp)
+append_string(char **a, const char *b, const char *separator)
 {
+  char *temp;
+
+  if (*a)
+    temp = db_mprintf("%s%s%s", *a, (separator ? separator : ""), b);
+  else
+    temp = db_mprintf("%s", b);
+
+  free(*a);
+  *a = temp;
+}
+
+/*
+ * Sets the filter (where clause) and the window (limit clause) in the given query_params
+ * based on the given arguments
+ *
+ * @param argc Number of arguments in argv
+ * @param argv Pointer to the first filter parameter
+ * @param exact_match If true, creates filter for exact matches (e. g. find command) otherwise matches substrings (e. g. search command)
+ * @param qp Query parameters
+ */
+static int
+parse_filter_window_params(int argc, char **argv, bool exact_match, struct query_params *qp)
+{
+  struct mpd_tagtype *tagtype;
   char *c1;
-  char *c2;
   int start_pos;
   int end_pos;
   int i;
@@ -598,221 +687,139 @@ mpd_get_query_params_find(int argc, char **argv, struct query_params *qp)
   int ret;
 
   c1 = NULL;
-  c2 = NULL;
 
   for (i = 0; i < argc; i += 2)
     {
-      if (0 == strcasecmp(argv[i], "any"))
+      // End of filter key/value pairs reached, if keywords "window" or "group" found
-	{
+      if (0 == strcasecmp(argv[i], "window") || 0 == strcasecmp(argv[i], "group"))
-	  c1 = db_mprintf("(f.artist LIKE '%%%q%%' OR f.album LIKE '%%%q%%' OR f.title LIKE '%%%q%%')", argv[i + 1], argv[i + 1], argv[i + 1]);
+	break;
-	}
+
-      else if (0 == strcasecmp(argv[i], "file"))
+      // Process filter key/value pair
-	{
+      if ((i + 1) < argc)
-	  c1 = db_mprintf("(f.virtual_path = '/%q')", argv[i + 1]);
+        {
-	}
+	  tagtype = find_tagtype(argv[i]);
-      else if (0 == strcasecmp(argv[i], "base"))
+
-	{
+	  if (!tagtype)
-	  c1 = db_mprintf("(f.virtual_path LIKE '/%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "modified-since"))
-	{
-	  DPRINTF(E_WARN, L_MPD, "Special parameter 'modified-since' is not supported by forked-daapd and will be ignored\n");
-	}
-      else if (0 == strcasecmp(argv[i], "window"))
-	{
-	  ret = mpd_pars_range_arg(argv[i + 1], &start_pos, &end_pos);
-	  if (ret == 0)
 	    {
-	      qp->idx_type = I_SUB;
+	      DPRINTF(E_WARN, L_MPD, "Parameter '%s' is not supported by forked-daapd and will be ignored\n", argv[i]);
-	      qp->limit = end_pos - start_pos;
+	      continue;
-	      qp->offset = start_pos;
 	    }
-	  else
+
+	  if (tagtype->type == MPD_TYPE_STRING)
 	    {
-	      DPRINTF(E_LOG, L_MPD, "Window argument doesn't convert to integer or range: '%s'\n", argv[i + 1]);
+	      if (exact_match)
+		c1 = db_mprintf("(%s = '%q')", tagtype->field, argv[i + 1]);
+	      else
+		c1 = db_mprintf("(%s LIKE '%%%q%%')", tagtype->field, argv[i + 1]);
+	    }
+	  else if (tagtype->type == MPD_TYPE_INT)
+	    {
+	      ret = safe_atou32(argv[i + 1], &num);
+	      if (ret < 0)
+		DPRINTF(E_WARN, L_MPD, "%s parameter '%s' is not an integer and will be ignored\n", tagtype->tag, argv[i + 1]);
+	      else
+		c1 = db_mprintf("(%s = %d)", tagtype->field, num);
+	    }
+	  else if (tagtype->type == MPD_TYPE_SPECIAL)
+	    {
+	      if (0 == strcasecmp(tagtype->tag, "any"))
+	        {
+		  c1 = db_mprintf("(f.artist LIKE '%%%q%%' OR f.album LIKE '%%%q%%' OR f.title LIKE '%%%q%%')", argv[i + 1], argv[i + 1], argv[i + 1]);
+		}
+	      else if (0 == strcasecmp(tagtype->tag, "file"))
+	        {
+		  if (exact_match)
+		    c1 = db_mprintf("(f.virtual_path = '/%q')", argv[i + 1]);
+		  else
+		    c1 = db_mprintf("(f.virtual_path LIKE '%%%q%%')", argv[i + 1]);
+		}
+	      else if (0 == strcasecmp(tagtype->tag, "base"))
+	        {
+		  c1 = db_mprintf("(f.virtual_path LIKE '/%q%%')", argv[i + 1]);
+		}
+	      else
+	        {
+		  DPRINTF(E_WARN, L_MPD, "Unknown special parameter '%s' will be ignored\n", tagtype->tag);
+		}
 	    }
-	}
-      else if (0 == strcasecmp(argv[i], "artist"))
-	{
-	  c1 = db_mprintf("(f.artist = '%q')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "albumartist"))
-	{
-	  c1 = db_mprintf("(f.album_artist = '%q')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "album"))
-	{
-	  c1 = db_mprintf("(f.album = '%q')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "title"))
-	{
-	  c1 = db_mprintf("(f.title = '%q')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "genre"))
-	{
-	  c1 = db_mprintf("(f.genre = '%q')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "disc"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    DPRINTF(E_WARN, L_MPD, "Disc parameter '%s' is not an integer and will be ignored\n", argv[i + 1]);
-	  else
-	    c1 = db_mprintf("(f.disc = %d)", num);
-	}
-      else if (0 == strcasecmp(argv[i], "track"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    DPRINTF(E_WARN, L_MPD, "Track parameter '%s' is not an integer and will be ignored\n", argv[i + 1]);
-	  else
-	    c1 = db_mprintf("(f.track = %d)", num);
-	}
-      else if (0 == strcasecmp(argv[i], "date"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    c1 = db_mprintf("(f.year = 0 OR f.year IS NULL)");
-	  else
-	    c1 = db_mprintf("(f.year = %d)", num);
 	}
       else if (i == 0 && argc == 1)
-	{
+        {
 	  // Special case: a single token is allowed if listing albums for an artist
 	  c1 = db_mprintf("(f.album_artist = '%q')", argv[i]);
 	}
       else
-	{
+        {
-	  DPRINTF(E_WARN, L_MPD, "Parameter '%s' is not supported by forked-daapd and will be ignored\n", argv[i]);
+	  DPRINTF(E_WARN, L_MPD, "Missing value for parameter '%s', ignoring '%s'\n", argv[i], argv[i]);
 	}
 
       if (c1)
-	{
+        {
-	  if (qp->filter)
+	  append_string(&qp->filter, c1, " AND ");
-	    c2 = db_mprintf("%s AND %s", qp->filter, c1);
-	  else
-	    c2 = db_mprintf("%s", c1);
 
-	  free(qp->filter);
-
-	  qp->filter = c2;
-	  c2 = NULL;
 	  free(c1);
 	  c1 = NULL;
 	}
     }
 
+  if ((i + 1) < argc && 0 == strcasecmp(argv[i], "window"))
+    {
+      ret = mpd_pars_range_arg(argv[i + 1], &start_pos, &end_pos);
+      if (ret == 0)
+        {
+	  qp->idx_type = I_SUB;
+	  qp->limit = end_pos - start_pos;
+	  qp->offset = start_pos;
+	}
+      else
+        {
+	  DPRINTF(E_LOG, L_MPD, "Window argument doesn't convert to integer or range: '%s'\n", argv[i + 1]);
+	}
+      i += 2;
+    }
+
   return 0;
 }
 
 static int
-mpd_get_query_params_search(int argc, char **argv, struct query_params *qp)
+parse_group_params(int argc, char **argv, bool group_in_listcommand, struct query_params *qp, struct mpd_tagtype ***group, int *groupsize)
 {
-  char *c1;
+  int first_group;
-  char *c2;
-  int start_pos;
-  int end_pos;
   int i;
-  uint32_t num;
+  int j;
-  int ret;
+  struct mpd_tagtype *tagtype;
 
-  c1 = NULL;
+  *groupsize = 0;
-  c2 = NULL;
+  *group = NULL;
 
-  for (i = 0; i < argc; i += 2)
+  // Iterate through arguments to the first "group" argument
+  for (first_group = 0; first_group < argc; first_group++)
     {
-      if (0 == strcasecmp(argv[i], "any"))
+      if (0 == strcasecmp(argv[first_group], "group"))
-	{
+	break;
-	  c1 = db_mprintf("(f.artist LIKE '%%%q%%' OR f.album LIKE '%%%q%%' OR f.title LIKE '%%%q%%')", argv[i + 1], argv[i + 1], argv[i + 1]);
+    }
-	}
+
-      else if (0 == strcasecmp(argv[i], "file"))
+  // Early return if no group keyword in arguments (or group keyword not followed by field argument)
-	{
+  if ((first_group + 1) >= argc || (argc - first_group) % 2 != 0)
-	  c1 = db_mprintf("(f.virtual_path LIKE '%%%q%%')", argv[i + 1]);
+    return 0;
-	}
+
-      else if (0 == strcasecmp(argv[i], "base"))
+  *groupsize = (argc - first_group) / 2;
-	{
+  *group = calloc(*groupsize, sizeof(struct mpd_tagtype *));
-	  c1 = db_mprintf("(f.virtual_path LIKE '/%q%%')", argv[i + 1]);
+
-	}
+  // Now process all group/field arguments
-      else if (0 == strcasecmp(argv[i], "modified-since"))
+  for (j = 0; j < (*groupsize); j++)
-	{
+    {
-	  DPRINTF(E_WARN, L_MPD, "Special parameter 'modified-since' is not supported by forked-daapd and will be ignored\n");
+      i = first_group + (j * 2);
-	}
+
-      else if (0 == strcasecmp(argv[i], "window"))
+      if ((i + 1) < argc && 0 == strcasecmp(argv[i], "group"))
-	{
+        {
-	  ret = mpd_pars_range_arg(argv[i + 1], &start_pos, &end_pos);
+	  tagtype = find_tagtype(argv[i + 1]);
-	  if (ret == 0)
+	  if (tagtype && tagtype->type != MPD_TYPE_SPECIAL)
 	    {
-	      qp->idx_type = I_SUB;
+	      if (group_in_listcommand)
-	      qp->limit = end_pos - start_pos;
+		append_string(&qp->group, tagtype->group_field, ", ");
-	      qp->offset = start_pos;
+	      (*group)[j] = tagtype;
 	    }
-	  else
-	    {
-	      DPRINTF(E_LOG, L_MPD, "Window argument doesn't convert to integer or range: '%s'\n", argv[i + 1]);
-	    }
-	}
-      else if (0 == strcasecmp(argv[i], "artist"))
-	{
-	  c1 = db_mprintf("(f.artist LIKE '%%%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "albumartist"))
-	{
-	  c1 = db_mprintf("(f.album_artist LIKE '%%%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "album"))
-	{
-	  c1 = db_mprintf("(f.album LIKE '%%%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "title"))
-	{
-	  c1 = db_mprintf("(f.title LIKE '%%%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "genre"))
-	{
-	  c1 = db_mprintf("(f.genre LIKE '%%%q%%')", argv[i + 1]);
-	}
-      else if (0 == strcasecmp(argv[i], "disc"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    DPRINTF(E_WARN, L_MPD, "Disc parameter '%s' is not an integer and will be ignored\n", argv[i + 1]);
-	  else
-	    c1 = db_mprintf("(f.disc = %d)", num);
-	}
-      else if (0 == strcasecmp(argv[i], "track"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    DPRINTF(E_WARN, L_MPD, "Track parameter '%s' is not an integer and will be ignored\n", argv[i + 1]);
-	  else
-	    c1 = db_mprintf("(f.track = %d)", num);
-	}
-      else if (0 == strcasecmp(argv[i], "date"))
-	{
-	  ret = safe_atou32(argv[i + 1], &num);
-	  if (ret < 0)
-	    c1 = db_mprintf("(f.year = 0 OR f.year IS NULL)");
-	  else
-	    c1 = db_mprintf("(f.year = %d)", num);
-	}
-      else
-	{
-	  DPRINTF(E_WARN, L_MPD, "Parameter '%s' is not supported by forked-daapd and will be ignored\n", argv[i]);
-	}
-
-      if (c1)
-	{
-	  if (qp->filter)
-	    c2 = db_mprintf("%s AND %s", qp->filter, c1);
-	  else
-	    c2 = db_mprintf("%s", c1);
-
-	  free(qp->filter);
-
-	  qp->filter = c2;
-	  c2 = NULL;
-	  free(c1);
-	  c1 = NULL;
 	}
     }
 
@@ -2054,7 +2061,7 @@ mpd_command_playlistfind(struct evbuffer *evbuf, int argc, char **argv, char **e
       return ACK_ERROR_ARG;
     }
 
-  mpd_get_query_params_find(argc - 1, argv + 1, &query_params);
+  parse_filter_window_params(argc - 1, argv + 1, true, &query_params);
 
   ret = db_queue_enum_start(&query_params);
   if (ret < 0)
@@ -2098,7 +2105,7 @@ mpd_command_playlistsearch(struct evbuffer *evbuf, int argc, char **argv, char *
       return ACK_ERROR_ARG;
     }
 
-  mpd_get_query_params_search(argc - 1, argv + 1, &query_params);
+  parse_filter_window_params(argc - 1, argv + 1, false, &query_params);
 
   ret = db_queue_enum_start(&query_params);
   if (ret < 0)
@@ -2598,7 +2605,7 @@ mpd_command_count(struct evbuffer *evbuf, int argc, char **argv, char **errmsg,
 
   memset(&qp, 0, sizeof(struct query_params));
   qp.type = Q_COUNT_ITEMS;
-  mpd_get_query_params_find(argc - 1, argv + 1, &qp);
+  parse_filter_window_params(argc - 1, argv + 1, true, &qp);
 
   ret = db_filecount_get(&fci, &qp);
   if (ret < 0)
@@ -2640,7 +2647,7 @@ mpd_command_find(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, s
   qp.sort = S_NAME;
   qp.idx_type = I_NONE;
 
-  mpd_get_query_params_find(argc - 1, argv + 1, &qp);
+  parse_filter_window_params(argc - 1, argv + 1, true, &qp);
 
   ret = db_query_start(&qp);
   if (ret < 0)
@@ -2686,7 +2693,7 @@ mpd_command_findadd(struct evbuffer *evbuf, int argc, char **argv, char **errmsg
   qp.sort = S_ARTIST;
   qp.idx_type = I_NONE;
 
-  mpd_get_query_params_find(argc - 1, argv + 1, &qp);
+  parse_filter_window_params(argc - 1, argv + 1, true, &qp);
 
   player_get_status(&status);
 
@@ -2704,11 +2711,13 @@ mpd_command_findadd(struct evbuffer *evbuf, int argc, char **argv, char **errmsg
 static int
 mpd_command_list(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, struct mpd_client_ctx *ctx)
 {
+  struct mpd_tagtype *tagtype;
   struct query_params qp;
-  struct db_group_info dbgri;
+  struct mpd_tagtype **group;
-  char *type;
+  int groupsize;
-  char *browse_item;
+  struct db_media_file_info dbmfi;
-  char *sort_item;
+  char **strval;
+  int i;
   int ret;
 
   if (argc < 2 || ((argc % 2) != 0))
@@ -2720,111 +2729,77 @@ mpd_command_list(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, s
 	}
     }
 
-  memset(&qp, 0, sizeof(struct query_params));
+  tagtype = find_tagtype(argv[1]);
 
-  if (0 == strcasecmp(argv[1], "artist"))
+  if (!tagtype || tagtype->type == MPD_TYPE_SPECIAL) //FIXME allow "file" tagtype
-    {
-      qp.type = Q_GROUP_ARTISTS;
-      qp.sort = S_ARTIST;
-      type = "Artist: ";
-    }
-  else if (0 == strcasecmp(argv[1], "albumartist"))
-      {
-	qp.type = Q_GROUP_ARTISTS;
-	qp.sort = S_ARTIST;
-	type = "AlbumArtist: ";
-      }
-  else if (0 == strcasecmp(argv[1], "album"))
-    {
-      qp.type = Q_GROUP_ALBUMS;
-      qp.sort = S_ALBUM;
-      type = "Album: ";
-    }
-  else if (0 == strcasecmp(argv[1], "date"))
-    {
-      qp.type = Q_BROWSE_YEARS;
-      type = "Date: ";
-    }
-  else if (0 == strcasecmp(argv[1], "genre"))
-    {
-      qp.type = Q_BROWSE_GENRES;
-      type = "Genre: ";
-    }
-  else if (0 == strcasecmp(argv[1], "disc"))
-    {
-      qp.type = Q_BROWSE_DISCS;
-      type = "Disc: ";
-    }
-  else if (0 == strcasecmp(argv[1], "track"))
-    {
-      qp.type = Q_BROWSE_TRACKS;
-      type = "Track: ";
-    }
-  else if (0 == strcasecmp(argv[1], "file"))
-    {
-      qp.type = Q_BROWSE_VPATH;
-      type = "file: ";
-    }
-  else
     {
       DPRINTF(E_WARN, L_MPD, "Unsupported type argument for command 'list': %s\n", argv[1]);
       return 0;
     }
 
+  memset(&qp, 0, sizeof(struct query_params));
+  qp.type = Q_ITEMS;
   qp.idx_type = I_NONE;
+  qp.order = tagtype->sort_field;
+  qp.group = strdup(tagtype->group_field);
 
   if (argc > 2)
     {
-      mpd_get_query_params_find(argc - 2, argv + 2, &qp);
+      parse_filter_window_params(argc - 2, argv + 2, true, &qp);
     }
 
+  group = NULL;
+  groupsize = 0;
+  parse_group_params(argc - 2, argv + 2, tagtype->group_in_listcommand, &qp, &group, &groupsize);
+
   ret = db_query_start(&qp);
   if (ret < 0)
     {
       db_query_end(&qp);
       free(qp.filter);
+      free(qp.group);
+      free(group);
 
       *errmsg = safe_asprintf("Could not start query");
       return ACK_ERROR_UNKNOWN;
     }
 
-  if (qp.type & Q_F_BROWSE)
+  while (((ret = db_query_fetch_file(&qp, &dbmfi)) == 0) && (dbmfi.id))
     {
-      if (qp.type == Q_BROWSE_VPATH)
+      strval = (char **) ((char *)&dbmfi + tagtype->mfi_offset);
+
+      if (!(*strval) || (**strval == '\0'))
+	continue;
+
+      evbuffer_add_printf(evbuf,
+			  "%s: %s\n",
+			  tagtype->tag,
+			  *strval);
+
+      if (group && groupsize > 0)
 	{
-	  while (((ret = db_query_fetch_string_sort(&qp, &browse_item, &sort_item)) == 0) && (browse_item))
+	  for (i = 0; i < groupsize; i++)
 	    {
-		// Remove the first "/" from the virtual_path
+	      if (!group[i])
-		evbuffer_add_printf(evbuf,
+		continue;
-		      "%s%s\n",
+
-		      type,
+	      strval = (char **) ((char *)&dbmfi + group[i]->mfi_offset);
-		      (browse_item + 1));
+
+	      if (!(*strval) || (**strval == '\0'))
+		continue;
+
+	      evbuffer_add_printf(evbuf,
+	      			  "%s: %s\n",
+				  group[i]->tag,
+	      			  *strval);
 	    }
 	}
-      else
-	{
-	  while (((ret = db_query_fetch_string_sort(&qp, &browse_item, &sort_item)) == 0) && (browse_item))
-	    {
-		evbuffer_add_printf(evbuf,
-		      "%s%s\n",
-		      type,
-		      browse_item);
-	    }
-	}
-    }
-  else
-    {
-      while ((ret = db_query_fetch_group(&qp, &dbgri)) == 0)
-	{
-	  evbuffer_add_printf(evbuf,
-		"%s%s\n",
-		type,
-		dbgri.itemname);
-	}
     }
 
   db_query_end(&qp);
   free(qp.filter);
+  free(qp.group);
+  free(group);
 
   return 0;
 }
@@ -3140,7 +3115,7 @@ mpd_command_search(struct evbuffer *evbuf, int argc, char **argv, char **errmsg,
   qp.sort = S_NAME;
   qp.idx_type = I_NONE;
 
-  mpd_get_query_params_search(argc - 1, argv + 1, &qp);
+  parse_filter_window_params(argc - 1, argv + 1, false, &qp);
 
   ret = db_query_start(&qp);
   if (ret < 0)
@@ -3186,7 +3161,7 @@ mpd_command_searchadd(struct evbuffer *evbuf, int argc, char **argv, char **errm
   qp.sort = S_ARTIST;
   qp.idx_type = I_NONE;
 
-  mpd_get_query_params_search(argc - 1, argv + 1, &qp);
+  parse_filter_window_params(argc - 1, argv + 1, false, &qp);
 
   player_get_status(&status);
 
@@ -3962,16 +3937,13 @@ mpd_command_commands(struct evbuffer *evbuf, int argc, char **argv, char **errms
 static int
 mpd_command_tagtypes(struct evbuffer *evbuf, int argc, char **argv, char **errmsg, struct mpd_client_ctx *ctx)
 {
-  evbuffer_add_printf(evbuf,
+  int i;
-      "tagtype: Artist\n"
+
-      "tagtype: AlbumArtist\n"
+  for (i = 0; i < ARRAY_SIZE(tagtypes); i++)
-      "tagtype: ArtistSort\n"
+    {
-      "tagtype: AlbumArtistSort\n"
+      if (tagtypes[i].type != MPD_TYPE_SPECIAL)
-      "tagtype: Album\n"
+	evbuffer_add_printf(evbuf, "tagtype: %s\n", tagtypes[i].tag);
-      "tagtype: Title\n"
+    }
-      "tagtype: Track\n"
-      "tagtype: Genre\n"
-      "tagtype: Disc\n");
 
   return 0;
 }