Re: Opinions on multi-source search



Hi,

On Mon, 31 May 2010 12:07:34 +0200, Iago Toral <itoral igalia com> wrote:
> I would like to work on adding a new feature to Grilo that I had planned long 
> ago and I would like to hear your opinions as to how this should be done (or 
> if it shouldn't be done :))
> 
> The idea is to provide API to allow users to search content in all sources (or 
> maybe a specific subset of sources). The use case is simple: you want media 
> content related to something but you are not sure where to get it from.

find attached a set of patches with an implementation of this, let me know what 
you think.

Summary:

1) Added grl-multiple.[ch]: Implementation of the API to operate on multiple 
sources. Includes search on all available sources and also operation cancellation 
support:

  - grl_multiple_search ()
  - grl_multiple_cancel ()

2) I also added support in grilo-test-ui for this

  - It uses grl_multiple_search when selecting "All" in the search combo.
  - It uses grl_multiple_cancel when the current peration is a grl_multiple_search

3) Fixed a bug I found in grl_media_source that got in the way of the implementation

4) Some plugins are not implementing cancel() properly, I realized this when 
implementing the cancellation support for the multiple search. I filed bugs 
about this in bugzilla

5) The count parameter is a maximum total count.

  - for exmaple if you request count = 100 and you have 4 searchable sources 
it will search each one with count=25
  - I think it could be better to let this parameter be the count for each individual 
source instead, so that if you request count = 100 it will request count=100 
for each individual source available.

As for future work:

1) This implementation might need additional work to ensure it works in situations 
like plugins being unloaded while a multiple operation is being performed on 
them.

2) I think it could be interesting to add a GList of sources as parameter to 
grl_multiple_search, this would allow users to select what sources they are 
interested in (if NULL it would search on all available sources as it does now).

3) It may be nice to have something like grl_multiple_sarch_grouped. This API 
would not return results as they arrive, instead it would store them in a hashtable 
grouped by source and will push the whole hashtable to the user when all the 
results have benn received.


Iago

From 9b61460b5d140d9c7757af7096dee59812f22afd Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 16:40:44 +0200
Subject: [PATCH] [core] Do not setup auto-split when count is lower than threshold
 in search and query opeations.

---
 src/grl-media-source.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index 72ee3ef..4af6031 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -623,7 +623,10 @@ browse_result_relay_cb (GrlMediaSource *source,
   struct BrowseRelayCb *brc;
   guint plugin_remaining = remaining;
 
-  g_debug ("browse_result_relay_cb");
+  g_debug ("browse_result_relay_cb, op:%u, source:%s, remaining:%u",
+	   browse_id,
+	   grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)),
+	   remaining);
 
   brc = (struct BrowseRelayCb *) user_data;
 
@@ -735,7 +738,8 @@ browse_result_relay_cb (GrlMediaSource *source,
 
   /* Free callback data when we processed the last result */
   if (remaining == 0) {
-    g_debug ("Got remaining '0' for operation %d", browse_id);
+    g_debug ("Got remaining '0' for operation %d (%s)",
+	     browse_id,  grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)));
     if (brc->bspec) {
       free_browse_operation_spec (brc->bspec);
     } else if (brc->sspec) {
@@ -1328,7 +1332,8 @@ grl_media_source_search (GrlMediaSource *source,
   brc->sspec = ss;
 
   /* Setup auto-split management if requested */
-  if (source->priv->auto_split_threshold > 0) {
+  if (source->priv->auto_split_threshold > 0 &&
+      count > source->priv->auto_split_threshold) {
     g_debug ("auto-split: enabled");
     struct AutoSplitCtl *as_ctl = g_new0 (struct AutoSplitCtl, 1);
     as_ctl->count = count;
@@ -1460,7 +1465,8 @@ grl_media_source_query (GrlMediaSource *source,
   brc->qspec = qs;
 
   /* Setup auto-split management if requested */
-  if (source->priv->auto_split_threshold > 0) {
+  if (source->priv->auto_split_threshold > 0 &&
+      count > source->priv->auto_split_threshold) {
     g_debug ("auto-split: enabled");
     struct AutoSplitCtl *as_ctl = g_new0 (struct AutoSplitCtl, 1);
     as_ctl->count = count;
-- 
1.6.0.4

From 505b82a5e52076c2ba6b1bbc459bb7146afbd0e8 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 16:44:52 +0200
Subject: [PATCH] [core] Make grl_media_source_gen_browse_id protected

---
 src/Makefile.am             |    5 +++--
 src/grl-media-source-priv.h |   41 +++++++++++++++++++++++++++++++++++++++++
 src/grl-media-source.c      |    5 +----
 3 files changed, 45 insertions(+), 6 deletions(-)
 create mode 100644 src/grl-media-source-priv.h

diff --git a/src/Makefile.am b/src/Makefile.am
index f2c4075..a95a9bf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -23,7 +23,7 @@ lib GRL_NAME@_la_SOURCES =					\
 	grl-plugin-registry.c					\
 	grl-metadata-key.c					\
 	grl-metadata-source.c grl-metadata-source-priv.h	\
-	grl-media-source.c					\
+	grl-media-source.c grl-media-source-priv.h		\
 	grl-log.c
 
 data_c_sources =		\
@@ -63,7 +63,8 @@ lib GRL_NAME@inc_HEADERS += $(data_h_headers)
 
 noinst_HEADERS =		\
 	grl-media-plugin-priv.h	\
-	grl-metadata-source-priv.h
+	grl-metadata-source-priv.h \
+	grl-media-source-priv.h
 
 MAINTAINERCLEANFILES =	\
 	*.in		\
diff --git a/src/grl-media-source-priv.h b/src/grl-media-source-priv.h
new file mode 100644
index 0000000..c3c8198
--- /dev/null
+++ b/src/grl-media-source-priv.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2010 Igalia S.L.
+ *
+ * Contact: Iago Toral Quiroga <itoral igalia com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef _GRL_MEDIA_SOURCE_PRIV_H_
+#define _GRL_MEDIA_SOURCE_PRIV_H_
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "grl-media-source.h"
+
+#include <glib.h>
+#include <glib-object.h>
+
+G_BEGIN_DECLS
+
+guint grl_media_source_gen_browse_id (GrlMediaSource *source);
+
+G_END_DECLS
+
+#endif /* _GRL_MEDIA_SOURCE_PRIV_H_ */
diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index 4af6031..61f9670 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -156,9 +156,6 @@ static void grl_media_source_set_property (GObject *object,
                                            const GValue *value,
                                            GParamSpec *pspec);
 
-static guint
-grl_media_source_gen_browse_id (GrlMediaSource *source);
-
 static GrlSupportedOps
 grl_media_source_supported_operations (GrlMetadataSource *metadata_source);
 
@@ -1079,7 +1076,7 @@ metadata_full_resolution_ctl_cb (GrlMediaSource *source,
   }
 }
 
-static guint
+guint
 grl_media_source_gen_browse_id (GrlMediaSource *source)
 {
   GrlMediaSourceClass *klass;
-- 
1.6.0.4

From b25a2ba02030ba8745b8fbd32c6d9785db38cde6 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 16:46:34 +0200
Subject: [PATCH] [core] Added grl_multiple_search API.

---
 src/Makefile.am    |    6 +-
 src/grilo.h        |    1 +
 src/grl-multiple.c |  157 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/grl-multiple.h |   42 ++++++++++++++
 4 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 src/grl-multiple.c
 create mode 100644 src/grl-multiple.h

diff --git a/src/Makefile.am b/src/Makefile.am
index a95a9bf..c825479 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,7 +24,8 @@ lib GRL_NAME@_la_SOURCES =					\
 	grl-metadata-key.c					\
 	grl-metadata-source.c grl-metadata-source-priv.h	\
 	grl-media-source.c grl-media-source-priv.h		\
-	grl-log.c
+	grl-log.c 						\
+	grl-multiple.c
 
 data_c_sources =		\
 	data/grl-data.c		\
@@ -48,7 +49,8 @@ lib GRL_NAME@inc_HEADERS =	\
 	grl-metadata-key.h	\
 	grl-metadata-source.h	\
 	grl-media-source.h	\
-	grl-log.h
+	grl-log.h 		\
+	grl-multiple.h
 
 data_h_headers =		\
 	data/grl-data.h		\
diff --git a/src/grilo.h b/src/grilo.h
index e65503b..6665a73 100644
--- a/src/grilo.h
+++ b/src/grilo.h
@@ -45,6 +45,7 @@
 #include <grl-media-image.h>
 #include <grl-media-box.h>
 #include <grl-config.h>
+#include <grl-multiple.h>
 
 #undef _GRILO_H_INSIDE_
 
diff --git a/src/grl-multiple.c b/src/grl-multiple.c
new file mode 100644
index 0000000..92a914e
--- /dev/null
+++ b/src/grl-multiple.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2010 Igalia S.L.
+ *
+ * Contact: Iago Toral Quiroga <itoral igalia com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include "grl-multiple.h"
+#include "grl-plugin-registry.h"
+#include "grl-media-source-priv.h"
+
+#undef G_LOG_DOMAIN
+#define G_LOG_DOMAIN "grl-multiple"
+
+struct MultipleSearchData {
+  GHashTable *table;
+  guint remaining;
+  GList *search_ids;
+  guint search_id;
+  GrlMediaSourceResultCb user_callback;
+  gpointer user_data;
+};
+
+static void
+multiple_search_cb (GrlMediaSource *source,
+		    guint search_id,
+		    GrlMedia *media,
+		    guint remaining,
+		    gpointer user_data,
+		    const GError *error)
+{
+  g_debug ("multiple_search_cb");
+
+  struct MultipleSearchData *msd = (struct MultipleSearchData *) user_data;
+  guint source_remaining;
+  guint diff;
+
+  g_debug ("multiple:remaining == %u, source:remaining = %u (%s)",
+	     msd->remaining, remaining,
+	     grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)));
+
+  source_remaining = GPOINTER_TO_INT (g_hash_table_lookup (msd->table,
+							   (gpointer) source));
+  if (remaining != -1) {
+    diff = source_remaining - remaining;
+    g_hash_table_insert (msd->table, source, GINT_TO_POINTER (remaining - 1));
+  } else {
+    diff = 0;
+    g_hash_table_insert (msd->table, source, GINT_TO_POINTER (source_remaining - 1));
+  }
+  
+  msd->remaining -= diff;
+
+  msd->user_callback (source,
+		      msd->search_id,
+		      media,
+		      msd->remaining,
+		      msd->user_data,
+		      NULL);
+  
+  if (msd->remaining == 0) {
+    g_debug ("Multiple operation finished (%u)", msd->search_id);
+    g_hash_table_unref (msd->table);
+    g_free (msd);
+  } else {
+    msd->remaining--;
+  }
+}
+
+guint
+grl_multiple_search (const gchar *text,
+		     const GList *keys,
+		     guint count,
+		     GrlMetadataResolutionFlags flags,
+		     GrlMediaSourceResultCb callback,
+		     gpointer user_data)
+{
+  GrlPluginRegistry *registry;
+  GrlMediaPlugin **sources;
+  guint individual_count, first_count, source_count;
+  guint search_id, n;
+  struct MultipleSearchData *msd;
+
+  g_debug ("grl_multiple_search");
+
+  registry = grl_plugin_registry_get_instance ();
+  sources = grl_plugin_registry_get_sources_by_operations (registry,
+							   GRL_OP_SEARCH,
+							   TRUE);
+
+  /* TODO: handle this properly */
+  if (sources[0] == NULL) {
+    g_free (sources);
+    callback (NULL, 0, NULL, 0, user_data, NULL);
+    return 0;
+  }
+
+  /* Prepare operation */
+  for (n = 0; sources[n] != NULL; n++);
+  individual_count = count / n;
+  first_count = individual_count + count % n;
+  source_count = n;
+
+  msd = g_new0 (struct MultipleSearchData, 1);
+  msd->table = g_hash_table_new (g_direct_hash, g_direct_equal);
+  msd->remaining = count - 1;
+  msd->user_callback = callback;
+  msd->user_data = user_data;
+    
+  /* Execute multiple search */
+  for (n = 0; sources[n]; n++) {
+    GrlMediaSource *source;
+    guint c, id;
+
+    source = GRL_MEDIA_SOURCE (sources[n]);
+
+    if (n == 0) {
+      c = first_count; 
+      msd->search_id = grl_media_source_gen_browse_id (source);
+    } else {
+      c = individual_count;
+    }
+
+    if (c > 0) {
+      g_hash_table_insert (msd->table, source, GINT_TO_POINTER (c - 1));
+      id = grl_media_source_search (source,
+				    text,
+				    keys,
+				    0, c,
+				    flags,
+				    multiple_search_cb,
+				    msd);
+      g_debug ("Operation %u: Searching %u items in %s",
+	       id, c, grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)));
+      msd->search_ids = g_list_prepend (msd->search_ids, GINT_TO_POINTER (id));
+    }
+  }
+
+  g_free (sources);
+
+  return msd->search_id;
+}
diff --git a/src/grl-multiple.h b/src/grl-multiple.h
new file mode 100644
index 0000000..f6842f1
--- /dev/null
+++ b/src/grl-multiple.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2010 Igalia S.L.
+ *
+ * Contact: Iago Toral Quiroga <itoral igalia com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#if !defined (_GRILO_H_INSIDE_) && !defined (GRILO_COMPILATION)
+#error "Only <grilo.h> can be included directly."
+#endif
+
+#ifndef _GRL_MULTIPLE_H_
+#define _GRL_MULTIPLE_H_
+
+#include <glib.h>
+
+#include "grl-media-source.h"
+
+guint grl_multiple_search (const gchar *text,
+			   const GList *keys,
+			   guint count,
+			   GrlMetadataResolutionFlags flags,
+			   GrlMediaSourceResultCb callback,
+			   gpointer user_data);
+
+
+#endif
-- 
1.6.0.4

From c96bde56b3560a702dc02292cabcbb8d740520cd Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 16:49:55 +0200
Subject: [PATCH] [core] Added preconditions to grl_multiple_search

---
 src/grl-multiple.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/grl-multiple.c b/src/grl-multiple.c
index 92a914e..9dabe44 100644
--- a/src/grl-multiple.c
+++ b/src/grl-multiple.c
@@ -98,6 +98,10 @@ grl_multiple_search (const gchar *text,
 
   g_debug ("grl_multiple_search");
 
+  g_return_val_if_fail (count > 0, 0);
+  g_return_val_if_fail (text != NULL, 0);
+  g_return_val_if_fail (callback != NULL, 0);
+
   registry = grl_plugin_registry_get_instance ();
   sources = grl_plugin_registry_get_sources_by_operations (registry,
 							   GRL_OP_SEARCH,
-- 
1.6.0.4

From 166d6fd0c649744b11bf9e3b74f71223cae091e8 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 16:59:09 +0200
Subject: [PATCH] [test-ui] Added support for multiple search()

---
 tools/grilo-test-ui/main.c |   39 +++++++++++++++++++++++++++++++--------
 1 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
index 148cce9..b6b49ff 100644
--- a/tools/grilo-test-ui/main.c
+++ b/tools/grilo-test-ui/main.c
@@ -135,6 +135,7 @@ typedef struct {
   guint offset;
   guint count;
   gchar *text;
+  gboolean multiple;
 } OperationState;
 
 typedef struct {
@@ -958,7 +959,8 @@ search_cb (GrlMediaSource *source,
 
   if (remaining == 0) {
     state->offset += state->count;
-    if (state->count >= BROWSE_CHUNK_SIZE &&
+    if (!state->multiple &&
+	state->count >= BROWSE_CHUNK_SIZE &&
 	state->offset < BROWSE_MAX_COUNT) {
       state->count = 0;
       next_search_id =
@@ -994,13 +996,25 @@ search (GrlMediaSource *source, const gchar *text)
 
   state = g_new0 (OperationState, 1);
   state->text = (gchar *) text;
-  search_id = grl_media_source_search (source,
-                                       text,
-                                       browse_keys (),
-                                       0, BROWSE_CHUNK_SIZE,
-                                       BROWSE_FLAGS,
-                                       search_cb,
-                                       state);
+  if (source) {
+    /* Normal search */
+    search_id = grl_media_source_search (source,
+					 text,
+					 browse_keys (),
+					 0, BROWSE_CHUNK_SIZE,
+					 BROWSE_FLAGS,
+					 search_cb,
+					 state);
+  } else {
+    /* Multiple search (all sources) */
+    state->multiple = TRUE;
+    search_id = grl_multiple_search (text,
+				     browse_keys (),
+				     BROWSE_MAX_COUNT,
+				     BROWSE_FLAGS,
+				     search_cb,
+				     state);
+  }
   clear_panes ();
   operation_started (source, search_id);
 }
@@ -1138,6 +1152,15 @@ search_combo_setup (void)
   }
   g_free (sources);
 
+  /* Add "All" option */
+  gtk_list_store_append (GTK_LIST_STORE (view->search_combo_model), &iter);
+  gtk_list_store_set (GTK_LIST_STORE (view->search_combo_model),
+		      &iter,
+		      SEARCH_MODEL_SOURCE, NULL,
+		      SEARCH_MODEL_NAME, "All",
+		      -1);
+  
+
   gtk_combo_box_set_active (GTK_COMBO_BOX (view->search_combo), 0);
 }
 
-- 
1.6.0.4

From 9255cd2c689675254cedf63918a1b4d26b1bd4a6 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Tue, 1 Jun 2010 17:01:03 +0200
Subject: [PATCH] [test-ui] Fixed leaks because of unnecesary string duppings.

---
 tools/grilo-test-ui/main.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
index b6b49ff..2f652a0 100644
--- a/tools/grilo-test-ui/main.c
+++ b/tools/grilo-test-ui/main.c
@@ -1105,7 +1105,7 @@ query_combo_setup (void)
                                                            FALSE);
   while (sources[i]) {
     gchar *name =
-      g_strdup (grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i])));
+      grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i]));
     gtk_list_store_append (GTK_LIST_STORE (view->query_combo_model), &iter);
     gtk_list_store_set (GTK_LIST_STORE (view->query_combo_model),
 			&iter,
@@ -1141,7 +1141,7 @@ search_combo_setup (void)
                                                            FALSE);
   while (sources[i]) {
     gchar *name =
-      g_strdup (grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i])));
+      grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i]));
     gtk_list_store_append (GTK_LIST_STORE (view->search_combo_model), &iter);
     gtk_list_store_set (GTK_LIST_STORE (view->search_combo_model),
 			&iter,
@@ -1451,8 +1451,7 @@ show_plugins ()
     gchar *name;
     GdkPixbuf *icon;
     icon = load_icon (GTK_STOCK_DIRECTORY);
-    name =
-      g_strdup (grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i])));
+    name = grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i]));
     g_debug ("Loaded source: '%s'", name);
     gtk_list_store_append (GTK_LIST_STORE (view->browser_model), &iter);
     gtk_list_store_set (GTK_LIST_STORE (view->browser_model),
-- 
1.6.0.4

From 3d8f38a9747073bc003239f7345efed36630ecc0 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Wed, 2 Jun 2010 08:27:51 +0200
Subject: [PATCH] [core] Removed trailing whitespaces

---
 src/grl-multiple.c         |    8 ++++----
 tools/grilo-test-ui/main.c |    1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/grl-multiple.c b/src/grl-multiple.c
index 9dabe44..6cec5ca 100644
--- a/src/grl-multiple.c
+++ b/src/grl-multiple.c
@@ -63,7 +63,7 @@ multiple_search_cb (GrlMediaSource *source,
     diff = 0;
     g_hash_table_insert (msd->table, source, GINT_TO_POINTER (source_remaining - 1));
   }
-  
+
   msd->remaining -= diff;
 
   msd->user_callback (source,
@@ -72,7 +72,7 @@ multiple_search_cb (GrlMediaSource *source,
 		      msd->remaining,
 		      msd->user_data,
 		      NULL);
-  
+
   if (msd->remaining == 0) {
     g_debug ("Multiple operation finished (%u)", msd->search_id);
     g_hash_table_unref (msd->table);
@@ -125,7 +125,7 @@ grl_multiple_search (const gchar *text,
   msd->remaining = count - 1;
   msd->user_callback = callback;
   msd->user_data = user_data;
-    
+
   /* Execute multiple search */
   for (n = 0; sources[n]; n++) {
     GrlMediaSource *source;
@@ -134,7 +134,7 @@ grl_multiple_search (const gchar *text,
     source = GRL_MEDIA_SOURCE (sources[n]);
 
     if (n == 0) {
-      c = first_count; 
+      c = first_count;
       msd->search_id = grl_media_source_gen_browse_id (source);
     } else {
       c = individual_count;
diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
index 2f652a0..3a64573 100644
--- a/tools/grilo-test-ui/main.c
+++ b/tools/grilo-test-ui/main.c
@@ -1159,7 +1159,6 @@ search_combo_setup (void)
 		      SEARCH_MODEL_SOURCE, NULL,
 		      SEARCH_MODEL_NAME, "All",
 		      -1);
-  
 
   gtk_combo_box_set_active (GTK_COMBO_BOX (view->search_combo), 0);
 }
-- 
1.6.0.4

From 14e2278e8cc697ceb3dfb88add5edf3de8feb239 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Wed, 2 Jun 2010 08:51:19 +0200
Subject: [PATCH] [core] Handle "no searchable sources available" situation in grl_multiple_search

---
 src/grl-multiple.c |   46 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/grl-multiple.c b/src/grl-multiple.c
index 6cec5ca..8e979fc 100644
--- a/src/grl-multiple.c
+++ b/src/grl-multiple.c
@@ -23,6 +23,7 @@
 #include "grl-multiple.h"
 #include "grl-plugin-registry.h"
 #include "grl-media-source-priv.h"
+#include "grl-error.h"
 
 #undef G_LOG_DOMAIN
 #define G_LOG_DOMAIN "grl-multiple"
@@ -36,6 +37,44 @@ struct MultipleSearchData {
   gpointer user_data;
 };
 
+struct CallbackData {
+  GrlMediaSourceResultCb user_callback;
+  gpointer user_data;
+};
+
+static void
+free_multiple_search_data (struct MultipleSearchData *msd)
+{
+  g_hash_table_unref (msd->table);
+  g_list_free (msd->search_ids);
+  g_free (msd);
+}
+
+static gboolean
+handle_no_searchable_sources_idle (gpointer user_data)
+{
+  GError *error;
+  struct CallbackData *callback_data = (struct CallbackData *) user_data;
+
+  error = g_error_new (GRL_ERROR, GRL_ERROR_SEARCH_FAILED, 
+                       "No searchable sources available");
+  callback_data->user_callback (NULL, 0, NULL, 0, callback_data->user_data, error);
+
+  g_error_free (error);
+  g_free (callback_data);
+
+  return FALSE;
+}
+
+static void
+handle_no_searchable_sources (GrlMediaSourceResultCb callback, gpointer user_data)
+{
+  struct CallbackData *callback_data = g_new0 (struct CallbackData, 1);
+  callback_data->user_callback = callback;
+  callback_data->user_data = user_data;
+  g_idle_add (handle_no_searchable_sources_idle, callback_data);
+}
+
 static void
 multiple_search_cb (GrlMediaSource *source,
 		    guint search_id,
@@ -75,8 +114,7 @@ multiple_search_cb (GrlMediaSource *source,
 
   if (msd->remaining == 0) {
     g_debug ("Multiple operation finished (%u)", msd->search_id);
-    g_hash_table_unref (msd->table);
-    g_free (msd);
+    free_multiple_search_data (msd);
   } else {
     msd->remaining--;
   }
@@ -107,10 +145,10 @@ grl_multiple_search (const gchar *text,
 							   GRL_OP_SEARCH,
 							   TRUE);
 
-  /* TODO: handle this properly */
+  /* No searchable sources? */
   if (sources[0] == NULL) {
     g_free (sources);
-    callback (NULL, 0, NULL, 0, user_data, NULL);
+    handle_no_searchable_sources (callback, user_data);
     return 0;
   }
 
-- 
1.6.0.4

From 939e68088ec4c16fd9091d5ea2dff6126cc2cc17 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Wed, 2 Jun 2010 10:18:55 +0200
Subject: [PATCH] [core] Added grl_multiple_cancel

---
 src/grl-multiple.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++----
 src/grl-multiple.h |    2 +
 2 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/src/grl-multiple.c b/src/grl-multiple.c
index 8e979fc..eb0ac6a 100644
--- a/src/grl-multiple.c
+++ b/src/grl-multiple.c
@@ -32,7 +32,9 @@ struct MultipleSearchData {
   GHashTable *table;
   guint remaining;
   GList *search_ids;
+  GList *sources;
   guint search_id;
+  gboolean cancelled;
   GrlMediaSourceResultCb user_callback;
   gpointer user_data;
 };
@@ -42,6 +44,13 @@ struct CallbackData {
   gpointer user_data;
 };
 
+/* ================= Globals ================= */
+
+static GHashTable *pending_operations = NULL;
+static gint multiple_search_id = 1;
+
+/* ================ Utitilies ================ */
+
 static void
 free_multiple_search_data (struct MultipleSearchData *msd)
 {
@@ -88,11 +97,14 @@ multiple_search_cb (GrlMediaSource *source,
   struct MultipleSearchData *msd = (struct MultipleSearchData *) user_data;
   guint source_remaining;
   guint diff;
+  gboolean emit;
 
   g_debug ("multiple:remaining == %u, source:remaining = %u (%s)",
 	     msd->remaining, remaining,
 	     grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)));
 
+  /* --- Update remaining count --- */
+
   source_remaining = GPOINTER_TO_INT (g_hash_table_lookup (msd->table,
 							   (gpointer) source));
   if (remaining != -1) {
@@ -105,21 +117,53 @@ multiple_search_cb (GrlMediaSource *source,
 
   msd->remaining -= diff;
 
-  msd->user_callback (source,
-		      msd->search_id,
-		      media,
-		      msd->remaining,
-		      msd->user_data,
-		      NULL);
+  /* --- Manage NULL results --- */
+
+  if (remaining == 0 && media == NULL && msd->remaining > 0) {
+    /* A source emitted a NULL result to finish its search operation
+       we don't want to relay this to the client (unless this is the
+       last one in the multiple search) */
+    emit = FALSE;
+  } else {
+    emit = TRUE;
+  }
+
+  /* --- Cancelation management --- */
+
+  if (msd->cancelled) {
+    g_debug ("operation is cancelled or already finished, skipping result!");
+    if (media) {
+      g_object_unref (media);
+      media = NULL;
+    }
+    if (msd->remaining > 0) {
+      /* Do not emit if operation was cancelled, only let the last 
+         result through */
+      emit = FALSE;
+    }
+  }
+
+  /* --- Result emission --- */
+  if (emit) {
+    msd->user_callback (source,
+  		        msd->search_id,
+ 		        media,
+		        msd->remaining,
+		        msd->user_data,
+		        NULL);
+  }
 
   if (msd->remaining == 0) {
     g_debug ("Multiple operation finished (%u)", msd->search_id);
     free_multiple_search_data (msd);
   } else {
+    /* Next remaining value expected is one unit less */
     msd->remaining--;
   }
 }
 
+/* ================ API ================ */
+
 guint
 grl_multiple_search (const gchar *text,
 		     const GList *keys,
@@ -140,6 +184,10 @@ grl_multiple_search (const gchar *text,
   g_return_val_if_fail (text != NULL, 0);
   g_return_val_if_fail (callback != NULL, 0);
 
+  if (!pending_operations) {
+    pending_operations = g_hash_table_new (g_direct_hash, g_direct_equal);
+  }
+
   registry = grl_plugin_registry_get_instance ();
   sources = grl_plugin_registry_get_sources_by_operations (registry,
 							   GRL_OP_SEARCH,
@@ -163,6 +211,7 @@ grl_multiple_search (const gchar *text,
   msd->remaining = count - 1;
   msd->user_callback = callback;
   msd->user_data = user_data;
+  msd->search_id = multiple_search_id++;
 
   /* Execute multiple search */
   for (n = 0; sources[n]; n++) {
@@ -173,7 +222,6 @@ grl_multiple_search (const gchar *text,
 
     if (n == 0) {
       c = first_count;
-      msd->search_id = grl_media_source_gen_browse_id (source);
     } else {
       c = individual_count;
     }
@@ -189,11 +237,48 @@ grl_multiple_search (const gchar *text,
 				    msd);
       g_debug ("Operation %u: Searching %u items in %s",
 	       id, c, grl_metadata_source_get_name (GRL_METADATA_SOURCE (source)));
+
       msd->search_ids = g_list_prepend (msd->search_ids, GINT_TO_POINTER (id));
+      msd->sources = g_list_prepend (msd->sources, source);
     }
   }
 
+  g_hash_table_insert (pending_operations, GINT_TO_POINTER (msd->search_id),
+                       msd);
+
   g_free (sources);
 
   return msd->search_id;
 }
+
+void
+grl_multiple_cancel (guint search_id)
+{
+  g_debug ("grl_multiple_cancel");
+  
+  struct MultipleSearchData *msd;
+  GList *sources, *ids;
+
+  if (!pending_operations) {
+    return;
+  }
+
+  msd = (struct MultipleSearchData *)
+    g_hash_table_lookup (pending_operations, GINT_TO_POINTER (search_id));
+  if (!msd) {
+    g_debug ("Tried to cancel invalid or already cancelled "\
+	     "multiple operation. Skipping...");
+    return;
+  }
+
+  sources = msd->sources;
+  ids = msd->search_ids;
+  while (sources) {
+    grl_media_source_cancel (GRL_MEDIA_SOURCE (sources->data),
+                             GPOINTER_TO_INT (ids->data));
+    sources = g_list_next (sources);
+    ids = g_list_next (ids);
+  }
+
+  msd->cancelled = TRUE;
+}
diff --git a/src/grl-multiple.h b/src/grl-multiple.h
index f6842f1..966c4cf 100644
--- a/src/grl-multiple.h
+++ b/src/grl-multiple.h
@@ -39,4 +39,6 @@ guint grl_multiple_search (const gchar *text,
 			   gpointer user_data);
 
 
+void grl_multiple_cancel (guint search_id);
+
 #endif
-- 
1.6.0.4

From 6b8f274d430df42e123eff26a31596aa2f785b62 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Wed, 2 Jun 2010 10:19:37 +0200
Subject: [PATCH] [test-ui] Call grl_multiple_cancel to cancel multiple search operations.

---
 tools/grilo-test-ui/main.c |   33 ++++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
index 3a64573..e8d6385 100644
--- a/tools/grilo-test-ui/main.c
+++ b/tools/grilo-test-ui/main.c
@@ -126,6 +126,7 @@ typedef struct {
   gboolean op_ongoing;
   GrlMediaSource *cur_op_source;
   guint cur_op_id;
+  gboolean multiple;
 
   /* Keeps track of the URL of the item selected */
   const gchar *last_url;
@@ -135,7 +136,6 @@ typedef struct {
   guint offset;
   guint count;
   gchar *text;
-  gboolean multiple;
 } OperationState;
 
 typedef struct {
@@ -366,7 +366,11 @@ static void
 cancel_current_operation (void)
 {
   if (ui_state->op_ongoing) {
-    grl_media_source_cancel (ui_state->cur_op_source, ui_state->cur_op_id);
+    if (!ui_state->multiple) {
+      grl_media_source_cancel (ui_state->cur_op_source, ui_state->cur_op_id);
+    } else {
+      grl_multiple_cancel (ui_state->cur_op_id);
+    }
     ui_state->op_ongoing = FALSE;
   }
 }
@@ -440,11 +444,13 @@ metadata_cb (GrlMediaSource *source,
 }
 
 static void
-operation_started (GrlMediaSource *source, guint operation_id)
+operation_started (GrlMediaSource *source, guint operation_id,
+                   gboolean multiple)
 {
   ui_state->op_ongoing = TRUE;
-  ui_state->cur_op_source  = source;
+  ui_state->cur_op_source = source;
   ui_state->cur_op_id = operation_id;
+  ui_state->multiple = multiple;
 }
 
 static void
@@ -522,7 +528,7 @@ browse_cb (GrlMediaSource *source,
                                    BROWSE_FLAGS,
                                    browse_cb,
                                    state);
-	operation_started (source, next_browse_id);
+	operation_started (source, next_browse_id, FALSE);
       } else {
 	/* We browsed all requested elements  */
 	goto browse_finished;
@@ -558,7 +564,7 @@ browse (GrlMediaSource *source, GrlMedia *container)
                                          BROWSE_FLAGS,
                                          browse_cb,
                                          state);
-    operation_started (source, browse_id);
+    operation_started (source, browse_id, FALSE);
   } else {
     show_plugins ();
   }
@@ -959,7 +965,7 @@ search_cb (GrlMediaSource *source,
 
   if (remaining == 0) {
     state->offset += state->count;
-    if (!state->multiple &&
+    if (!ui_state->multiple &&
 	state->count >= BROWSE_CHUNK_SIZE &&
 	state->offset < BROWSE_MAX_COUNT) {
       state->count = 0;
@@ -971,7 +977,7 @@ search_cb (GrlMediaSource *source,
                                  BROWSE_FLAGS,
                                  search_cb,
                                  state);
-      operation_started (source, next_search_id);
+      operation_started (source, next_search_id, FALSE);
     } else {
       goto search_finished;
     }
@@ -990,6 +996,7 @@ search (GrlMediaSource *source, const gchar *text)
 {
   OperationState *state;
   guint search_id;
+  gboolean multiple = FALSE;
 
   /* If we have an operation ongoing, let's cancel it first */
   cancel_current_operation ();
@@ -1007,7 +1014,7 @@ search (GrlMediaSource *source, const gchar *text)
 					 state);
   } else {
     /* Multiple search (all sources) */
-    state->multiple = TRUE;
+    multiple = TRUE;
     search_id = grl_multiple_search (text,
 				     browse_keys (),
 				     BROWSE_MAX_COUNT,
@@ -1016,7 +1023,7 @@ search (GrlMediaSource *source, const gchar *text)
 				     state);
   }
   clear_panes ();
-  operation_started (source, search_id);
+  operation_started (source, search_id, multiple);
 }
 
 static void
@@ -1059,7 +1066,7 @@ query (GrlMediaSource *source, const gchar *text)
                                      search_cb,
                                      state);
   clear_panes ();
-  operation_started (source, query_id);
+  operation_started (source, query_id, FALSE);
 }
 
 static void
@@ -1104,7 +1111,7 @@ query_combo_setup (void)
                                                            GRL_OP_QUERY,
                                                            FALSE);
   while (sources[i]) {
-    gchar *name =
+    const gchar *name =
       grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i]));
     gtk_list_store_append (GTK_LIST_STORE (view->query_combo_model), &iter);
     gtk_list_store_set (GTK_LIST_STORE (view->query_combo_model),
@@ -1140,7 +1147,7 @@ search_combo_setup (void)
                                                            GRL_OP_SEARCH,
                                                            FALSE);
   while (sources[i]) {
-    gchar *name =
+    const gchar *name =
       grl_metadata_source_get_name (GRL_METADATA_SOURCE (sources[i]));
     gtk_list_store_append (GTK_LIST_STORE (view->search_combo_model), &iter);
     gtk_list_store_set (GTK_LIST_STORE (view->search_combo_model),
-- 
1.6.0.4

From 991700b325e87875d41a4a1a68a81a5e110c9a85 Mon Sep 17 00:00:00 2001
From: Iago Toral Quiroga <itoral igalia com>
Date: Wed, 2 Jun 2010 10:39:22 +0200
Subject: [PATCH] [core] Revert changes to make grl_media_source_gen_browse_id protected.
 This is not needed any more.

---
 src/Makefile.am             |    5 ++---
 src/grl-media-source-priv.h |   41 -----------------------------------------
 src/grl-media-source.c      |   18 ++++++++++--------
 src/grl-multiple.c          |    1 -
 4 files changed, 12 insertions(+), 53 deletions(-)
 delete mode 100644 src/grl-media-source-priv.h

diff --git a/src/Makefile.am b/src/Makefile.am
index c825479..e26bdb8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -23,7 +23,7 @@ lib GRL_NAME@_la_SOURCES =					\
 	grl-plugin-registry.c					\
 	grl-metadata-key.c					\
 	grl-metadata-source.c grl-metadata-source-priv.h	\
-	grl-media-source.c grl-media-source-priv.h		\
+	grl-media-source.c 					\
 	grl-log.c 						\
 	grl-multiple.c
 
@@ -65,8 +65,7 @@ lib GRL_NAME@inc_HEADERS += $(data_h_headers)
 
 noinst_HEADERS =		\
 	grl-media-plugin-priv.h	\
-	grl-metadata-source-priv.h \
-	grl-media-source-priv.h
+	grl-metadata-source-priv.h
 
 MAINTAINERCLEANFILES =	\
 	*.in		\
diff --git a/src/grl-media-source-priv.h b/src/grl-media-source-priv.h
deleted file mode 100644
index c3c8198..0000000
--- a/src/grl-media-source-priv.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2010 Igalia S.L.
- *
- * Contact: Iago Toral Quiroga <itoral igalia com>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public License
- * as published by the Free Software Foundation; version 2.1 of
- * the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
- *
- */
-
-#ifndef _GRL_MEDIA_SOURCE_PRIV_H_
-#define _GRL_MEDIA_SOURCE_PRIV_H_
-
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
-#include "grl-media-source.h"
-
-#include <glib.h>
-#include <glib-object.h>
-
-G_BEGIN_DECLS
-
-guint grl_media_source_gen_browse_id (GrlMediaSource *source);
-
-G_END_DECLS
-
-#endif /* _GRL_MEDIA_SOURCE_PRIV_H_ */
diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index 61f9670..d2290ec 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -163,6 +163,8 @@ static gboolean
 operation_is_finished (GrlMediaSource *source,
 		       guint operation_id) __attribute__ ((unused)) ;
 
+static guint grl_media_source_gen_browse_id (GrlMediaSource *source);
+
 /* ================ GrlMediaSource GObject ================ */
 
 G_DEFINE_ABSTRACT_TYPE (GrlMediaSource,
@@ -214,6 +216,14 @@ grl_media_source_init (GrlMediaSource *source)
     g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free);
 }
 
+static guint
+grl_media_source_gen_browse_id (GrlMediaSource *source)
+{
+  GrlMediaSourceClass *klass;
+  klass = GRL_MEDIA_SOURCE_GET_CLASS (source);
+  return klass->browse_id++;
+}
+
 static void
 grl_media_source_get_property (GObject *object,
                                guint prop_id,
@@ -1076,14 +1086,6 @@ metadata_full_resolution_ctl_cb (GrlMediaSource *source,
   }
 }
 
-guint
-grl_media_source_gen_browse_id (GrlMediaSource *source)
-{
-  GrlMediaSourceClass *klass;
-  klass = GRL_MEDIA_SOURCE_GET_CLASS (source);
-  return klass->browse_id++;
-}
-
 /* ================ API ================ */
 
 /**
diff --git a/src/grl-multiple.c b/src/grl-multiple.c
index eb0ac6a..aea177d 100644
--- a/src/grl-multiple.c
+++ b/src/grl-multiple.c
@@ -22,7 +22,6 @@
 
 #include "grl-multiple.h"
 #include "grl-plugin-registry.h"
-#include "grl-media-source-priv.h"
 #include "grl-error.h"
 
 #undef G_LOG_DOMAIN
-- 
1.6.0.4



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]