Re: [PATCH] media-source: avoid some useless call the grl_metadata_source_resolve()

[Guillaume Emont <gemont igalia com>]

On 14/03/2011 12:31, lionel g landwerlin linux intel com wrote:
> From: Lionel Landwerlin <lionel g landwerlin linux intel com>
> 
> When doing a full metadata resolution, avoid to call
> grl_metadata_source_resolve on sources that can not actually fill new
> metadata keys.
> 

Did you find cases where this happens? If so, it's a bug in
grl_metadata_source_get_additional_sources(), it only return sources
returned by get_additional_source_for_key() for one of the keys passed
as parameter. This latter method  should return a source for which
_may_resolve() returned TRUE.
If you have cases where _may_resolve() returns FALSE for a (key, source)
couple, and _resolve() is called for that same (key, source) couple, I
think it's a bug in either grl_metadata_source_get_additional_sources()
or get_additional_source_for_key(), and it should be fixed there.

Guij

> Signed-off-by: Lionel Landwerlin <lionel g landwerlin linux intel com>
> ---
>  src/grl-media-source.c |   33 ++++++++++++++++++++++++---------
>  1 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/src/grl-media-source.c b/src/grl-media-source.c
> index f32346c..df5a731 100644
> --- a/src/grl-media-source.c
> +++ b/src/grl-media-source.c
> @@ -1339,15 +1339,30 @@ metadata_full_resolution_ctl_cb (GrlMediaSource *source,
>                 grl_metadata_source_get_name (_source));
>  
>      if (grl_metadata_source_supported_operations (_source) & GRL_OP_RESOLVE) {
> -      grl_metadata_source_resolve (_source,
> -                                   /* all keys are asked, metadata sources
> -                                    * should check what's already in media */
> -                                   ctl_info->keys,
> -                                   media,
> -                                   ctl_info->flags,
> -                                   metadata_full_resolution_done_cb,
> -                                   done_info);
> -      done_info->pending_callbacks++;
> +      GList *key = ctl_info->keys;
> +
> +      /* First, let's find out if this source can actually resolve something. */
> +      while (key) {
> +        if (grl_metadata_source_may_resolve (_source,
> +                                             media,
> +                                             key->data,
> +                                             NULL))
> +          break;
> +        key = key->next;
> +      }
> +
> +      /* If it does, then call resolve. */
> +      if (key) {
> +        grl_metadata_source_resolve (_source,
> +                                     /* all keys are asked, metadata sources
> +                                      * should check what's already in media */
> +                                     ctl_info->keys,
> +                                     media,
> +                                     ctl_info->flags,
> +                                     metadata_full_resolution_done_cb,
> +                                     done_info);
> +        done_info->pending_callbacks++;
> +      }
>      }
>    }
>    g_list_free (sources);