Re: [core v2] core: Allow grouping all changed medias in "content-changed" signal




Hi,

a few comments below...

On Thu, 7 Apr 2011 17:00:43 +0200, "Juan A. Suarez Romero" <jasuarez igalia com> wrote:
When several multimedia files change at the same time, it can be
useful to send
just one signal containing all the changed medias, instead of sending one
signal per change.

This commit changes the "content-changed" signal and related
functions to send
a list of changed medias instead of one media.

Signed-off-by: Juan A. Suarez Romero <jasuarez igalia com>
---
 src/grl-marshal.list       |    2 +-
 src/grl-media-source.c     |   96
+++++++++++++++++++++++++++++++++----------
 src/grl-media-source.h     |    7 +++-
 tools/grilo-test-ui/main.c |   55 ++++++++++++++-----------
 4 files changed, 110 insertions(+), 50 deletions(-)

diff --git a/src/grl-marshal.list b/src/grl-marshal.list
index b524c7a..3472bef 100644
--- a/src/grl-marshal.list
+++ b/src/grl-marshal.list
@@ -1 +1 @@
-VOID:OBJECT,ENUM,BOOLEAN
+VOID:BOXED,ENUM,BOOLEAN
diff --git a/src/grl-media-source.c b/src/grl-media-source.c
index d700660..c516509 100644
--- a/src/grl-media-source.c
+++ b/src/grl-media-source.c
@@ -227,20 +227,26 @@ grl_media_source_class_init
(GrlMediaSourceClass *media_source_class)
   /**
    * GrlMediaSource::content-changed:
    * @source: source that has changed
-   * @media: the media that changed or one of its ancestors
+ * @changed_medias: a #GPtrArray with the medias that changed or one of
+   * their ancestors

...with the media that changed or a common ancestor of them of type #GrlBox.

    * @change_type: the kind of change that ocurred
    * @location_unknown: @TRUE if the change happened in @media
itself or in one
    * of its direct children (when @media is a #GrlMediaBox). @FALSE
otherwise
    *
- * Signals that the content in the source has changed. Usually @media is a
-   * #GrlBox, meaning that the content of that box has changed. if
-   * @location_unknown is @TRUE it means the source cannot establish
where the
- * change happened: could be either in the box, in any child, or in any
-   * other descendant of the box in the hierarchy.
+   * Signals that the content in the source has changed.
@changed_medias is the
+   * list of elements that have been changed. Usually these medias
are of type

....list of elements that have changed....

+   * #GrlBox, meaning that the content of that box has changed.
+   *
+   * If @location_unknown is @TRUE it means the source cannot
establish where the
+   * change happened: could be either in the box, in any child, or
in any other
+   * descendant of the box in the hierarchy.
+   *
+   * Both @change_type and @location_unknown are applied to all
elements in the
+   * list.
    *
    * For the cases where the source can only signal that a change
happened, but
-   * not where, it would use the root box (@NULL id) and set
location_unknown as
-   * to @TRUE.
+ * not where, it would use a list with the the root box (@NULL id) and set
+   * location_unknown as @TRUE.
    *
    * Since: 0.1.9
    */
@@ -251,10 +257,10 @@ grl_media_source_class_init
(GrlMediaSourceClass *media_source_class)
                  0,
                  NULL,
                  NULL,
-                 grl_marshal_VOID__OBJECT_ENUM_BOOLEAN,
+                 grl_marshal_VOID__BOXED_ENUM_BOOLEAN,
                  G_TYPE_NONE,
                  3,
-                 GRL_TYPE_MEDIA,
+                 G_TYPE_PTR_ARRAY,
                  GRL_TYPE_MEDIA_SOURCE_CHANGE_TYPE,
                  G_TYPE_BOOLEAN);
 }
@@ -2699,13 +2705,17 @@ grl_media_source_notify_change_stop
(GrlMediaSource *source,
 /**
  * grl_media_source_notify_change:
  * @source: a media source
- * @media: (allow-none): the media which has changed, or @NULL to
use the root box.
+ * @changed_medias: (element-type Grl.Media) (transfer full):: the list of
+ * medias that have changed
  * @change_type: the type of change
* @location_unknown: if change has happpened in @media or any descendant
  *
  * Emits "content-changed" signal to notify subscribers that a
change ocurred
  * in @source.
  *
+ * Do not unref @changed_medias array nor the content after invoking this
+ * function. Function will take care of that.

I think the message should be more general:

The function will take ownership of @changed_medias and it should not be manipulated in any way by the caller after invoking this function. If that is needed, the caller must ref the array in advance.

+ *
  * See GrlMediaSource::content-changed signal.
  *
  * <note>
@@ -2717,29 +2727,69 @@ grl_media_source_notify_change_stop
(GrlMediaSource *source,
  * Since: 0.1.9
  */
 void grl_media_source_notify_change (GrlMediaSource *source,
-                                     GrlMedia *media,
+                                     GPtrArray *changed_medias,
GrlMediaSourceChangeType change_type,
                                      gboolean location_unknown)
 {
-  gboolean free_media = FALSE;
+  const gchar *source_id;

   g_return_if_fail (GRL_IS_MEDIA_SOURCE (source));
-  g_return_if_fail (!media || GRL_IS_MEDIA (media));
+  g_return_if_fail (changed_medias);

-  if (!media) {
-    media = grl_media_box_new();
-    free_media = TRUE;
-  }
+  /* Set the source */
+ source_id = grl_metadata_source_get_id (GRL_METADATA_SOURCE (source));
+  g_ptr_array_foreach (changed_medias,
+                       (GFunc) grl_media_set_source,
+                       (gpointer) source_id);
+
+  /* Add hook to free content when freeing the array */
+  g_ptr_array_set_free_func (changed_medias, (GDestroyNotify)
g_object_unref);

-  grl_media_set_source (media,
-                        grl_metadata_source_get_id
(GRL_METADATA_SOURCE (source)));
   g_signal_emit (source,
                  registry_signals[SIG_CONTENT_CHANGED],
                  0,
-                 media,
+                 changed_medias,
                  change_type,
                  location_unknown);
-  if (free_media) {
-    g_object_unref (media);
+
+  g_ptr_array_unref (changed_medias);
+}
+
+/**
+ * grl_media_source_notify_change_single:

A nitpick, but if we really want to have two APIs to ease the single-change case I'd rather use this naming convention:
grl_media_source_notify_change for single changes
grl_media_source_notify_change_list for multiple changes.

I think it is more consistent.

+ * @source: a media source
+ * @changed_media: (allow none): the changed media, or @NULL for root box
+ * @change_type: the type of change
+ * @location_unknown: if change has happpened in @media or any descendant

happppened => happened

+ * Emits "content-changed" signal to notify subscribers that a
change ocurred
+ * in @source.
+ *
+ * See #grl_media_source_notify_change() function.
+ *
+ * <note>
+ *  <para>
+ *    This function is intended to be used only by plugins.
+ *  </para>
+ * </note>
+ */
+void grl_media_source_notify_change_single (GrlMediaSource *source,
+                                            GrlMedia *changed_media,
+                                            GrlMediaSourceChangeType
change_type,
+ gboolean location_unknown)
+{
+  GPtrArray *ptr_array;
+
+  g_return_if_fail (GRL_IS_MEDIA_SOURCE (source));
+
+  if (!changed_media) {
+    changed_media = grl_media_box_new ();
+  } else {
+    g_object_ref (changed_media);
   }
+
+  ptr_array = g_ptr_array_sized_new (1);
+  g_ptr_array_add (ptr_array, changed_media);
+
+  grl_media_source_notify_change (source, ptr_array, change_type,
location_unknown);
 }
diff --git a/src/grl-media-source.h b/src/grl-media-source.h
index a37f425..adc5331 100644
--- a/src/grl-media-source.h
+++ b/src/grl-media-source.h
@@ -533,10 +533,15 @@ gboolean grl_media_source_notify_change_stop
(GrlMediaSource *source,
                                               GError **error);

 void grl_media_source_notify_change (GrlMediaSource *source,
-                                     GrlMedia *media,
+                                     GPtrArray *changed_medias,
GrlMediaSourceChangeType change_type,
                                      gboolean location_unknown);

+void grl_media_source_notify_change_single (GrlMediaSource *source,
+                                            GrlMedia *changed_media,
+                                            GrlMediaSourceChangeType
change_type,
+ gboolean location_unknown);
+
 G_END_DECLS

 #endif /* _GRL_MEDIA_SOURCE_H_ */
diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
index 8c8230d..b6a5462 100644
--- a/tools/grilo-test-ui/main.c
+++ b/tools/grilo-test-ui/main.c
@@ -203,7 +203,7 @@ static void shutdown_plugins (void);

 static void changes_notification_cb (GtkToggleAction *action);
 static void content_changed_cb (GrlMediaSource *source,
-                                GrlMedia *media,
+                                GPtrArray *changed_medias,
GrlMediaSourceChangeType change_type,
                                 gboolean location_unknown,
                                 gpointer data);
@@ -1776,16 +1776,17 @@ remove_notification (gpointer data)

 static void
 content_changed_cb (GrlMediaSource *source,
-                    GrlMedia *media,
+                    GPtrArray *changed_medias,
                     GrlMediaSourceChangeType change_type,
                     gboolean location_unknown,
                     gpointer data)
 {
-  const gchar *media_id = grl_media_get_id (media);
+  GrlMedia *media;
+  const gchar *media_id = NULL;
   const gchar *change_type_string = "";
   const gchar *location_string = "";
   gchar *message;
-  guint id;
+  guint id, i;

   switch (change_type) {
   case GRL_CONTENT_CHANGED:
@@ -1803,29 +1804,33 @@ content_changed_cb (GrlMediaSource *source,
     location_string = "(unknown place)";
   }

-  if (GRL_IS_MEDIA_BOX (media)) {
-    message =
-      g_strdup_printf ("%s: container '%s' has %s%s",
-                       grl_metadata_source_get_name
(GRL_METADATA_SOURCE (source)),
-                       media_id? media_id: "root",
-                       change_type_string,
-                       location_string);
-  } else {
-    message =
-      g_strdup_printf ("%s: element '%s' has %s",
-                       grl_metadata_source_get_name
(GRL_METADATA_SOURCE (source)),
-                       media_id,
-                       change_type_string);
-  }
+  for (i = 0; i < changed_medias->len; i++) {
+    media = g_ptr_array_index (changed_medias, i);
+    media_id = grl_media_get_id (media);
+    if (GRL_IS_MEDIA_BOX (media)) {
+      message =
+        g_strdup_printf ("%s: container '%s' has %s%s",
+                         grl_metadata_source_get_name
(GRL_METADATA_SOURCE (source)),
+                         media_id? media_id: "root",
+                         change_type_string,
+                         location_string);
+    } else {
+      message =
+        g_strdup_printf ("%s: element '%s' has %s",
+                         grl_metadata_source_get_name
(GRL_METADATA_SOURCE (source)),
+                         media_id,
+                         change_type_string);
+    }

-  id = gtk_statusbar_push (GTK_STATUSBAR (view->statusbar),
-                           view->statusbar_context_id,
-                           message);
+    id = gtk_statusbar_push (GTK_STATUSBAR (view->statusbar),
+                             view->statusbar_context_id,
+                             message);

-  g_timeout_add_seconds (NOTIFICATION_TIMEOUT,
-                         remove_notification,
-                         GUINT_TO_POINTER (id));
-  g_free (message);
+    g_timeout_add_seconds (NOTIFICATION_TIMEOUT,
+                           remove_notification,
+                           GUINT_TO_POINTER (id));
+    g_free (message);
+  }
 }

 static void



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