Re: [PATCH 2/2] local-metadata: add series support

[Lionel Landwerlin <lionel g landwerlin linux intel com>]

Ok, will do.

Regards,

--
Lionel Landwerlin

On Thu, 2011-03-17 at 17:52 +0100, Guillaume Emont wrote:
> Hi Lionel!
> 
> Thanks for the new version.
> As we discussed on IRC, can you add a configuration option to give the
> opportunity to disable series recognition? For the record, the rationale
> is that the information provided is only a guess and cannot be 100%
> reliable. Some people/applications might want to disable it for that
> reason. Obviously, _supported_keys() and _may_resolve() should take that
> (de)activation into account.
> Since it's still a cool feature ot have I feel the default configuration
> should be to enable this.
> 
> I also have some comments on the code, inline in the patch below.
> 
> Cheers,
> 
> Guillaume
> 
> 
> On 16/03/2011 18:48, lionel g landwerlin linux intel com wrote:
> > From: Lionel Landwerlin <lionel g landwerlin linux intel com>
> > 
> > Based on a patch for Tracker by Iain Holmes :
> > 
> > http://build.meego.com/package/view_file?file=0002-Tracker-extract-Parse-the-video-filename-to-obtain-e.patch&package=tracker&project=devel%3Acontentfw&srcmd5=c91b724488ee9ae476a9ce65755d8152
> > 
> > Signed-off-by: Lionel Landwerlin <lionel g landwerlin linux intel com>
> > ---
> >  configure.ac                                     |    3 +
> >  src/metadata/local-metadata/grl-local-metadata.c |  393 ++++++++++++++++++++--
> >  2 files changed, 373 insertions(+), 23 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 39c2955..54fe141 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -709,6 +709,9 @@ AC_ARG_ENABLE(localmetadata,
> >                          if test "x$HAVE_GIO" = "xno"; then
> >                             AC_MSG_ERROR([gio-2.0 not found, install it or use --disable-localmetadata])
> >                          fi
> > +                        if test "x$HAVE_GLIB_2_26" = "xno"; then
> > +                           AC_MSG_ERROR([glib-2.26.0 or above not found, install it or use --disable-localmetadata])
> > +                        fi
> >                          ;;
> >                  esac
> >          ],
> > diff --git a/src/metadata/local-metadata/grl-local-metadata.c b/src/metadata/local-metadata/grl-local-metadata.c
> > index 62737ba..e1ae9e0 100644
> > --- a/src/metadata/local-metadata/grl-local-metadata.c
> > +++ b/src/metadata/local-metadata/grl-local-metadata.c
> > (...)
> > @@ -115,6 +155,191 @@ G_DEFINE_TYPE (GrlLocalMetadataSource,
> >                 GRL_TYPE_METADATA_SOURCE);
> > (...)
> > +/* tidies strings before we run them through the regexes */
> > +static gchar *
> > +video_uri_to_metadata (const gchar *uri)
> > +{
> 
> Like I asked in the first review: could you rename this function with a
> name that describes more what it actually does?
> 
> > (...)
> > @@ -233,13 +568,18 @@ grl_local_metadata_source_may_resolve (GrlMetadataSource *source,
> >                                         GrlKeyID key_id,
> >                                         GList **missing_keys)
> >  {
> > -  if (key_id != GRL_METADATA_KEY_THUMBNAIL
> > -      || GRL_IS_MEDIA_AUDIO (media)
> > -      || GRL_IS_MEDIA_BOX (media))
> > -    return FALSE;
> > -
> > -  if (media && grl_data_key_is_known (GRL_DATA (media), GRL_METADATA_KEY_URL))
> > -    return has_compatible_media_url (media);
> > +  if (media) {
> > +    if (grl_data_key_is_known (GRL_DATA (media), GRL_METADATA_KEY_URL) &&
> > +        has_compatible_media_url (media)) {
> > +      if (key_id == GRL_METADATA_KEY_THUMBNAIL ||
> > +          key_id == GRL_METADATA_KEY_TITLE ||
> > +          key_id == GRL_METADATA_KEY_SHOW ||
> > +          key_id == GRL_METADATA_KEY_DATE ||
> > +          key_id == GRL_METADATA_KEY_SEASON ||
> > +          key_id == GRL_METADATA_KEY_EPISODE)
> > +        return TRUE;
> > +    }
> > +  }
> I think we can be more fine-grained here, and check for the media type
> as well:
> Box,Audio -> FALSE
> Image -> TRUE if thumbnail was asked
> Video -> TRUE if at least one of thumbnail, title, show, date, season,
> episode was asked
> 
> _______________________________________________
> grilo-list mailing list
> grilo-list gnome org
> http://mail.gnome.org/mailman/listinfo/grilo-list