Re: [PATCH] core: Allow grouping all changed medias in "content-changed" signal
- From: Lionel Landwerlin <lionel g landwerlin linux intel com>
- To: grilo-list gnome org
- Subject: Re: [PATCH] core: Allow grouping all changed medias in "content-changed" signal
- Date: Mon, 04 Apr 2011 22:38:16 +0100
Hi,
I was thinking to a slightly different signal signature exposed to the
user.
Let's say you have 8 different views looking at the content being
changed and suddenly you add 400 files. Then 8 listeners are going to
process a list of 400 items (o(n)) one way or another, even if they just
copy the list to process it later. And that might impact the UI
displaying the results.
So one way to avoid that could be to pack everything into one object
which would contain the list of added/modified/removed items. That way
the UI would still be able to process the items by set of 10 and give
the hand back to the UI main loop. That object would likely be a gobject
on which we can apply reference counting, so various listeners still
parse just allocate an iterator to traverse the notified items.
The core would be responsible the allocate the objects notifying the
user, and the plugins would still provide a list of items
added/modified/removed as in your first patch.
What do you think ?
--
Lionel Landwerlin
On Fri, 2011-04-01 at 13:05 +0200, Juan A. Suarez Romero 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 | 58 +++++++++++++++++++++++++++----------------
> src/grl-media-source.h | 2 +-
> tools/grilo-test-ui/main.c | 53 ++++++++++++++++++++++------------------
> 4 files changed, 67 insertions(+), 48 deletions(-)
>
> diff --git a/src/grl-marshal.list b/src/grl-marshal.list
> index b524c7a..d9d825e 100644
> --- a/src/grl-marshal.list
> +++ b/src/grl-marshal.list
> @@ -1 +1 @@
> -VOID:OBJECT,ENUM,BOOLEAN
> +VOID:POINTER,ENUM,BOOLEAN
> diff --git a/src/grl-media-source.c b/src/grl-media-source.c
> index 12a8cfa..a657e68 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: pointer to #GList with the medias that changed or one of
> + * their ancestors
> * @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 this medias are #GrlBox,
> + * meaning that the content of that box has changed.
> + *
> + * I @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 to @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__POINTER_ENUM_BOOLEAN,
> G_TYPE_NONE,
> 3,
> - GRL_TYPE_MEDIA,
> + G_TYPE_POINTER,
> GRL_TYPE_MEDIA_SOURCE_CHANGE_TYPE,
> G_TYPE_BOOLEAN);
> }
> @@ -2682,7 +2688,8 @@ 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 none):: the list of
> + * medias that have changed, or @NULL to use a list with only the root box
> * @change_type: the type of change
> * @location_unknown: if change has happpened in @media or any descendant
> *
> @@ -2700,29 +2707,36 @@ grl_media_source_notify_change_stop (GrlMediaSource *source,
> * Since: 0.1.9
> */
> void grl_media_source_notify_change (GrlMediaSource *source,
> - GrlMedia *media,
> + GList *changed_medias,
> GrlMediaSourceChangeType change_type,
> gboolean location_unknown)
> {
> - gboolean free_media = FALSE;
> + const gchar *source_id;
> + gboolean free_medias = FALSE;
>
> g_return_if_fail (GRL_IS_MEDIA_SOURCE (source));
> - g_return_if_fail (!media || GRL_IS_MEDIA (media));
>
> - if (!media) {
> - media = grl_media_box_new();
> - free_media = TRUE;
> + if (!changed_medias) {
> + changed_medias = g_list_append (changed_medias,
> + grl_media_box_new());
> + free_medias = TRUE;
> }
>
> - grl_media_set_source (media,
> - grl_metadata_source_get_id (GRL_METADATA_SOURCE (source)));
> + /* Set the source */
> + source_id = grl_metadata_source_get_id (GRL_METADATA_SOURCE (source));
> + g_list_foreach (changed_medias,
> + (GFunc) grl_media_set_source,
> + (gpointer) source_id);
> +
> g_signal_emit (source,
> registry_signals[SIG_CONTENT_CHANGED],
> 0,
> - media,
> + changed_medias,
> change_type,
> location_unknown);
> - if (free_media) {
> - g_object_unref (media);
> +
> + if (free_medias) {
> + g_list_foreach (changed_medias, (GFunc) g_object_unref, NULL);
> + g_list_free (changed_medias);
> }
> }
> diff --git a/src/grl-media-source.h b/src/grl-media-source.h
> index a37f425..70e6d95 100644
> --- a/src/grl-media-source.h
> +++ b/src/grl-media-source.h
> @@ -533,7 +533,7 @@ gboolean grl_media_source_notify_change_stop (GrlMediaSource *source,
> GError **error);
>
> void grl_media_source_notify_change (GrlMediaSource *source,
> - GrlMedia *media,
> + GList *changed_medias,
> GrlMediaSourceChangeType change_type,
> gboolean location_unknown);
>
> diff --git a/tools/grilo-test-ui/main.c b/tools/grilo-test-ui/main.c
> index 526df4f..4197fea 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,
> + GList *changed_medias,
> GrlMediaSourceChangeType change_type,
> gboolean location_unknown,
> gpointer data);
> @@ -1776,12 +1776,12 @@ remove_notification (gpointer data)
>
> static void
> content_changed_cb (GrlMediaSource *source,
> - GrlMedia *media,
> + GList *changed_medias,
> GrlMediaSourceChangeType change_type,
> gboolean location_unknown,
> gpointer data)
> {
> - const gchar *media_id = grl_media_get_id (media);
> + const gchar *media_id = NULL;
> const gchar *change_type_string = "";
> const gchar *location_string = "";
> gchar *message;
> @@ -1803,29 +1803,34 @@ 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);
> - }
> + while (changed_medias) {
> + media_id = grl_media_get_id (changed_medias->data);
> + if (GRL_IS_MEDIA_BOX (changed_medias->data)) {
> + 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);
> + changed_medias = g_list_next (changed_medias);
> + }
> }
>
> static void
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
