After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686206 - Move generic TMDB metadata keys to libgrilo.
Move generic TMDB metadata keys to libgrilo.
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: core
0.2.x
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2012-10-16 07:30 UTC by Mathias Hasselmann (IRC: tbf)
Modified: 2012-11-01 09:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (18.36 KB, patch)
2012-10-23 21:29 UTC, Mathias Hasselmann (IRC: tbf)
none Details | Review
proposed patch (12.83 KB, patch)
2012-10-23 21:29 UTC, Mathias Hasselmann (IRC: tbf)
none Details | Review
proposed core patch (21.44 KB, patch)
2012-10-26 13:24 UTC, Mathias Hasselmann (IRC: tbf)
committed Details | Review
proposed tmdb patch (16.83 KB, patch)
2012-10-26 13:57 UTC, Mathias Hasselmann (IRC: tbf)
none Details | Review
proposed tmdb patch (17.02 KB, patch)
2012-10-26 14:29 UTC, Mathias Hasselmann (IRC: tbf)
none Details | Review
proposed tmdb patch (17.60 KB, patch)
2012-10-31 11:23 UTC, Mathias Hasselmann (IRC: tbf)
committed Details | Review

Description Mathias Hasselmann (IRC: tbf) 2012-10-16 07:30:36 UTC
The TMDB plugin defines a few new metadata keys of generic interest. They should be moved the libgrilo to make such data easier to use once other sources should provide similiar information.

 * tmdb-backdrops(gchararray): A list of URLs for movie backdrops
 * tmdb-poster(gchararray): A list of URLs for movie posters
 * tmdb-imdb-id(gchararray): ID of this movie at imdb.org
 * tmdb-keywords(gchararray): A list of keywords about the movie
 * tmdb-performer(gchararray): A list of actors taking part in the movie
 * tmdb-producer(gchararray): A list of producers of the movie
 * tmdb-director(gchararray): Director of the movie
 * tmdb-age-certificates(gchararray): Semicolon-separated list of all age certificates
 * tmdb-original-title(gchararray): Original title of a movie

The "tmdb-" prefix would be skipped of course. One probably also should rename "tmdb-keywords" into "keyword", and "tmdb-age-certificates" into "age-certificate". Also "tmdb-age-certificates" should be split into separate values to make it more useful in filters.
Comment 1 Mathias Hasselmann (IRC: tbf) 2012-10-16 07:33:29 UTC
see also: bug 679686
Comment 2 Mathias Hasselmann (IRC: tbf) 2012-10-16 07:34:54 UTC
Actually it proposes GRL_METADATA_KEY_CERTIFICATE there. Must then look again what exactly we report in "tmdb-age-certificates".
Comment 3 Mathias Hasselmann (IRC: tbf) 2012-10-16 07:45:31 UTC
Actually it proposes GRL_METADATA_KEY_CERTIFICATE there. Must then look again what exactly we report in "tmdb-age-certificates".
Comment 4 Mathias Hasselmann (IRC: tbf) 2012-10-16 07:45:40 UTC
Actually it proposes GRL_METADATA_KEY_CERTIFICATE there. Must then look again what exactly we report in "tmdb-age-certificates".
Comment 5 Mathias Hasselmann (IRC: tbf) 2012-10-23 21:29:12 UTC
Created attachment 227097 [details] [review]
proposed patch
Comment 6 Mathias Hasselmann (IRC: tbf) 2012-10-23 21:29:38 UTC
Created attachment 227098 [details] [review]
proposed patch
Comment 7 Mathias Hasselmann (IRC: tbf) 2012-10-26 09:48:37 UTC
So what's done in the patches? The core patch introduces those new keys:

+#define GRL_METADATA_KEY_KEYWORD              45
+#define GRL_METADATA_KEY_BACKDROP             46
+#define GRL_METADATA_KEY_POSTER               47
+#define GRL_METADATA_KEY_PERFORMER            48
+#define GRL_METADATA_KEY_PRODUCER             49
+#define GRL_METADATA_KEY_DIRECTOR             50
+#define GRL_METADATA_KEY_ORIGINAL_TITLE       51

It provides: 
* the GrlKeyID definitions
* updates VAPI and API docs
* provides simple GrlMedia accessors. 

About the keys:
* They all are defined as strings.
* There are no relations between them
* Except for GRL_METADATA_KEY_ORIGINAL_TITLE, all of them are multi-valued. How to actually express that?
* Only GRL_METADATA_KEY_KEYWORD is a generic key, all others are defined as video keys.

About the TMDB patch:
* It simply replaces the custom key definitions by the generic definitions, leaving "tmdb-id" and "tmdb-imdb-id" as the only TMDB specific keys.
Comment 8 Juan A. Suarez Romero 2012-10-26 11:17:31 UTC
Some comments:

- We should add the corrresponding "add()" functionts for keywords and performer. Those two keys are usually a multivalued, so we need convenience API to add keywords and performers.

- I have doubts with including backdrops as a core key. I don't think that key is so common as the others, like poster. I feel that only tmdb would be providing it. What do you think if we keep it as a specific tmdb plugin key?

- I have also doubts if we should introduce the "poster" key or rather re-utilize the thumbnail key. Note that we use thumbnail key as a generic graphical representation of the content: in the case of music, for instance, we use the cover art as a thumbnail. So when we have a movie, and we do not have any visual element to show it, probably using the poster as thumbnail would be a good idea.
Still, if we at the end consider that having poster as a specific key is a good idea, I think the TMDb should offer it also as a thumbnail; this would make life easier for developers, because when they want to show a content in screen with a thumbnail, for movies it would be using the poster as that thumbnail; no need to ask specific for the poster.
Comment 9 Mathias Hasselmann (IRC: tbf) 2012-10-26 13:24:31 UTC
Created attachment 227354 [details] [review]
proposed core patch
Comment 10 Mathias Hasselmann (IRC: tbf) 2012-10-26 13:57:54 UTC
Created attachment 227362 [details] [review]
proposed tmdb patch
Comment 11 Mathias Hasselmann (IRC: tbf) 2012-10-26 14:29:38 UTC
Created attachment 227365 [details] [review]
proposed tmdb patch
Comment 12 Juan A. Suarez Romero 2012-10-29 08:35:31 UTC
You have posted two patches for TMDb plugin, but none for core.
Comment 13 Mathias Hasselmann (IRC: tbf) 2012-10-29 09:20:46 UTC
(In reply to comment #12)
> You have posted two patches for TMDb plugin, but none for core.

Seems I hit the wrong "obsoletes" checkbox ;-)
Everything proper now.
Comment 14 Juan A. Suarez Romero 2012-10-29 11:02:18 UTC
Everything works fine, except for a small detail: backdrop is returned as thumbnail, instead of poster.

Is this the right behaviour? Because I think a poster is more suitable than backdrop, as it usually contains the title.
Comment 15 Mathias Hasselmann (IRC: tbf) 2012-10-29 12:00:21 UTC
(In reply to comment #14)
> Everything works fine, except for a small detail: backdrop is returned as
> thumbnail, instead of poster.
> 
> Is this the right behaviour? Because I think a poster is more suitable than
> backdrop, as it usually contains the title.

Oh, seems I forgot switching order of those resolver steps. Will change after lunch. Together with a fat note.
Comment 16 Mathias Hasselmann (IRC: tbf) 2012-10-31 11:23:26 UTC
Created attachment 227718 [details] [review]
proposed tmdb patch
Comment 17 Juan A. Suarez Romero 2012-11-01 09:00:26 UTC
Comment on attachment 227354 [details] [review]
proposed core patch

commit ae6327ba4a73bf426b6ec230491e9e7f6bf63121
Author: Mathias Hasselmann <mathias@openismus.com>
Date:   Tue Oct 23 22:44:10 2012 +0200

    core: Add generic metadata keys from TMDB
    
    The TMDB plugin defines a few new metadata keys of generic
    interest. They should be moved to Grilo's core to make such
    data easier to use once other sources should provide
    similiar information.
    
    This introduces following new keys together with generated
    GrlMedia accessors:
    
      * GRL_METADATA_KEY_KEYWORD
      * GRL_METADATA_KEY_PERFORMER
      * GRL_METADATA_KEY_PRODUCER
      * GRL_METADATA_KEY_DIRECTOR
      * GRL_METADATA_KEY_ORIGINAL_TITLE
    
    All keys are defined as strings and there are no relations
    between them. Except for "original-title" all of those keys
    are multi-valued. All keys but "keyword" are video specific
    keys.
    
    This fixes https://bugzilla.gnome.org/show_bug.cgi?id=686206
    
    Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com>

 bindings/vala/grilo-0.2-custom.vala |  10 ++++
 doc/grilo/grilo-sections.txt        |  18 ++++++++
 src/data/grl-media-video.c          | 264 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/data/grl-media-video.h          |  38 ++++++++++++++++
 src/data/grl-media.c                |  79 ++++++++++++++++++++++++++++++++
 src/data/grl-media.h                |   8 ++++
 src/grl-metadata-key.c              |  47 ++++++++++++++++++-
 src/grl-metadata-key.h              |   5 ++
 8 files changed, 468 insertions(+), 1 deletion(-)
Comment 18 Juan A. Suarez Romero 2012-11-01 09:07:21 UTC
commit af26db35ed7dbc103c07b3ef3a0a0d78a614b0a7
Author: Mathias Hasselmann <mathias@openismus.com>
Date:   Tue Oct 23 23:28:13 2012 +0200

    tmdb: Use new metadata keys from grilo core
    
    With https://bugzilla.gnome.org/show_bug.cgi?id=686206
    several metadata keys got moved from this plugin to grilo
    core. This commit replaces the custom key definitions by the
    generic definitions, leaving "tmdb-backdrop", "tmdb-poster",
    "tmdb-id" and "tmdb-imdb-id" as the only TMDB specific keys.

 src/tmdb/grl-tmdb.c |  226 ++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------
 1 file changed, 86 insertions(+), 140 deletions(-)