Re: [PATCH 1/3] local-metadata: added a new local source

[Iago Toral <itoral igalia com>]

On Wed, 26 Jan 2011 19:32:26 +0100, Guillaume Emont <gemont igalia com> 
wrote:
(...)
> +/* ================== API Implementation ================ */
(...)
> +static void
> +grl_local_metadata_source_resolve (GrlMetadataSource *source,
> +                                  GrlMetadataSourceResolveSpec *rs)
> +{
> +  const gchar *url;
> +  gchar *scheme;
> +
> +  GRL_DEBUG ("grl_local_metadata_source_resolve");
> +
> +  url = grl_media_get_url (rs->media);
> +  scheme = g_uri_parse_scheme (url);
> +
> +  if (0 != g_strcmp0 (scheme, "file")
> +      || !g_list_find (rs->keys, GRL_METADATA_KEY_THUMBNAIL)) {
> +    /* No can do! */
> +    rs->callback (source, rs->media, rs->user_data, NULL);
> +    goto exit;
> +  }

Shouldn't we emit an error here? client is using the API with invalid 
URI or invalid key...

> +  if (GRL_IS_MEDIA_VIDEO (rs->media)
> +      || GRL_IS_MEDIA_IMAGE (rs->media)) {
> +    resolve_image (rs);
> +  } else if (GRL_IS_MEDIA_AUDIO (rs->media)) {
> +    resolve_album_art (rs);
> +  } else {
> +    /* What's that media type? */
> +    rs->callback (source, rs->media, rs->user_data, NULL);
> +  }

Same here, we should emit an error.

> +exit:
> +  g_free (scheme);
> +}
> +

(...)

> diff --git a/src/local-metadata/grl-local-metadata.xml
> b/src/local-metadata/grl-local-metadata.xml
> new file mode 100644
> index 0000000..11e4d32
> --- /dev/null
> +++ b/src/local-metadata/grl-local-metadata.xml
> @@ -0,0 +1,9 @@
> +<plugin>
> +  <info>
> +    <name>Fake Metadata Provider</name>
> +    <description>A plugin for faking metadata 
> resolution</description>
> +    <author>Igalia S.L.</author>
> +    <license>LGPL</license>
> +    <site>http://www.igalia.com</site>
> +  </info>
> +</plugin>

I guess you copied this from the fake metadata plugin, needs proper 
editing :)

BTW, let's not forget to fix the issues in core that prevent testing 
this through grilo-test-ui, I think that should be high priority.

Iago