Re: [PATCH 0/9] Multi-valued proposal (WIP)

["Juan A." Suárez Romero <jasuarez igalia com>]

On Wed, 2011-02-23 at 07:48 +0000, Iago Toral wrote:
> Hi Juan,
> 
>  I'll be reviewing this patches, but give me some time :)

No problem. I understand this is a difficult topic (believe me, quite
difficult when I was trying on real code), so take your time.

> 
>  One thing that I don't like is GrlDataMulti: I don't see a good reason 
>  to have this, it does not solve any problem and introduces a new 
>  object/concept making things slightly more difficult for no good reason.
> 
>  When we discussed this topic in the past we always talked about adding 
>  additional API to GrlData and making it handle single and multi valued 
>  data transparently, not about subclassing it, and that is still the 
>  approach I think we should follow. Let's keep things simple.


Actually, I had started the idea adding multi-valued API to GrlData, but
everything become lot of complicated, so I tried to split up
single-valued and multi-valued API in two different classes, which
solved most of the problems.

Yes, it introduces a new class, but I don't think it makes things more
complicated. Actually, I think it helps to understand better the API, as
user can focus on single or multi valued API just focusing in one or
another class.


>  The other thing I don't particularly like is the approach to correlated 
>  properties. The relations between the properties are not explicit (we do 
>  not have explicit data types mapping them), so when I get a GrlProperty 
>  I might find thing a set of keys or another depending on the key I 
>  requested and the relationships I established at run-time. I am not sure 
>  if I like this better than having explicit types for each set of 
>  correlated properties, although I admit this approach is more dynamic 
>  since the correlations can be established at run-time. Somehow it feels 
>  like we forced the solution to reuse code and I don't like that feeling.

I feel I'm not understanding the paragrapha, specially the "when I get a
GrlProperty I might find thing a set of keys or another depending on the
key I requested and the relationships I established at run-time".

When creating relations between keys, there is a function to get those
relations, so applications can get which keys are related, for instance
with URI.

But besides this, this relations defined by core are somewhat fixed, in
the same way core register the predefined keys: we can add in
documentation which keys exists, and which keys are related with.

Let me clarify why I added this approach: plugins can define their own
keys on run-time and this keys can correlated with other keys.

As example, Gravatar plugin introduces two new keys, "artist-avatar" and
"author-avatar", and this keys are correlated with "artist" and
"author", respectively.

So I need to have a flexible way to handle this situation.

> 
>  I guess the question here is if the extra flexibility we get from this 
>  approach and the code reuse is worth the additional slight obscurity.
> 
>  The alternative to this, as I explained in an e-mail before would be to 
>  have explicit, fixed relations defined at compile time. Things like:
> 
>  GrlDataVideoURI, GrlDataThumbnail, etc
> 
>  These would be structs:
> 
>  typedef struct {
>    gchar *uri;
>    gchar *mime;
>    guint width;
>    guint height;
>    ...
>  } GrlVideoURI;
> 
>  And then we would have API to set and get these structs from a GrlData 
>  object:
> 
>  GrlMediaVideo *video;
>  GrlVideoUri *uri_data = grl_video_uri_new (uri, mime, width, height);
>  grl_media_video_set_uri (video, uri_data);
>  ...
>  GrlVideoUri *uri_data = grl_media_video_get_uri (video);
>  gchar *uri = grl_video_uri_get_uri (uri_data);
>  gchar *mime = grl_video_uri_get_mime (uri_data);
>  ...
> 
>  As you can see, the correlations here are fixed, they are defined by 
>  structures, so they are explicit and we provide the users with specific 
>  APIs to handle each correlation,, which I think is less obscure than 
>  using a generic API. The drawback is of course that they are fixed, and 
>  that you have to create structures and specific API for each correlation 
>  (more code) and you cannot define new relations at run-time.

Exactly. If Grilo wouldn't allow to create new keys by plugins, then of
course I go with this approach.

But here there is an important point I would like to say: what I sent is
a preliminary work that focus on the core part for adding multi-valued
properties. Based on this work, we can add new api to ease some things,
as we added api in GrlMediaAudio to ease its handle using GrlData api.

Thus, maybe we can add your api proposal to GrlMediaVideo using
GrlDataMulti api to implement it.

The only concern about that is how to cope with new potential keys
defined by plugins and related with URI.


	J.A.


> 
>  As I said, I still have not made up my mind on what approach I like 
>  better. I guess Juan likes what he proposed, so what do other think?
> 
>  Iago
> 
>  On Mon, 21 Feb 2011 10:36:26 +0100, "Juan A. Suarez Romero" 
>  <jasuarez igalia com> wrote:
> > Hello.
> >
> > Some time ago, Guillaume sent an proposal about how to handle 
> > multivalued
> > elements in Grilo. It got some interesting issues about some
> > drawbacks it had.
> >
> > So here there is a revamped version based on Guillaume proposal, that 
> > also
> > tries to fix the issues mentioned in that thread. Note that it
> > probably needs a
> > polishment, like choosing better names, and maybe adding some new 
> > functions.
> >
> > First of all, some important points to take in mind to understand the
> > proposal.
> >
> > - Being able to handle multi-valued keys should not complicate the 
> > life of
> >   people just interested in single-valued properties.
> > - Applications handling multi-valued properties should not be worry 
> > about
> >   plugins providing only single-valued properties. That is, an
> > application can
> >   be able to handle multi-valued properties, while the plugin it uses
> > can only
> >   handle single-valued properties.
> > - The other way around: applications handling single-valued 
> > properties should
> >   be able to work with plugins providing multi-valued properties in 
> > the same
> >   way as working with single-valued plugins.
> > - Very likely, most of applications are just interesting in 
> > single-valued
> >   properties. Do not complicate their life.
> >
> > Based on above assumptions, the proposal here extend GrlData, which
> > is able to
> > handle single-valued elements only, with a new API specifically 
> > designed to
> > handle multivalued elements. This new API is provided in a new class,
> > GrlDataMulti, that extends GrlData.
> >
> > GrlMedia and children now inherits from GrlDataMulti, instead of
> > GrlData. It is
> > important to note that I keep GrlMedia API as it is, that is, it only 
> > handles
> > single-valued elements. When an application/plugin wants to use 
> > multi-valued
> > API, it just casts the media to GrlMediaMulti and uses the right API 
> > (I will
> > expose an example later).
> >
> > One of the issues mentioned in Guillaume's proposal was the fact that
> > some keys
> > are somewhat correlated. That is, depending on the keys, there must 
> > be a
> > mathematical function between values of those two keys. For instance, 
> > having
> > several values for "url" key and several values for "mime" key means
> > that each
> > mime value corresponds to a value in url key.
> >
> > For this reason, I have introduced a new function in plugin registry,
> > grl_plugin_register_register_metadata_relation(), that basically 
> > creates a
> > relation between keys. In the core, right now keys "url", 
> > "mime-type",
> > "bitrate", "framerate", "height" and "width" are correlated. It means
> > that for
> > a specific url value, it has a specific mime-type, bitrate, and so
> > forth (or no
> > values for some of them).
> >
> > To make easier the things, whenever a new relation between 2 keys is 
> > created,
> > all the keys that were related with those 2 become part of the 
> > relationship
> > too. We can say we create a relation-ring. In example above, url and
> > the other
> > keys are in the same relation.
> >
> > It is important to mention that this relationship concept only makes
> > sense when
> > handling multi-valued keys: if application just use single-valued 
> > keys, that
> > relation is of no interest.
> >
> > So, how it works the multi-valued stuff? GrlDataMulti provides api to 
> > add,
> > remove, update and get the value of a key from a specific position. 
> > So we can
> > ask for 2nd value of mime-type. Instead of returning only the asked 
> > value, it
> > returns also the values of correlated keys. Therefore, it will return
> > also the
> > appropriate value for uri, bitrate, and so on. Note that maybe some 
> > of that
> > values are actually NULL, but they are still returned.
> >
> > These correlated keys are returned in a GrlProperty, which is 
> > basically a
> > GrlData containing the keys with this values. It doesn't provide any 
> > specific
> > API, as GrlData API is enough to get it.
> >
> > Therefore, as an example, for a plugin providing several values for
> > "url" key,
> > to get the 3rd value and the corresponding mime-type we can write:
> >
> > GrlProperty *prop = grl_data_multi_get (GRL_DATA_MULTI (media),
> > GRL_METADATA_KEY_URL, 2); /* Starts to count at 0 */
> > const gchar *url = grl_data_get_string (GRL_DATA (prop),
> > GRL_METADATA_KEY_URL);
> > const gchar *mime = grl_data_get_string (GRL_DATA (prop),
> > GRL_METADATA_KEY_MIME);
> >
> > For applications that only wants the values of a specific key,
> > forgetting about
> > other keys, there is API in GrlDataMulti to get them:
> > grl_data_multi_get_single_all() returns a list of values for that 
> > key.
> >
> > As I told at first, this is a work in progress, that probably needs 
> > some
> > polishment. In any case, here is to get some feedback.
> >
> >
> >           J.A.
> >
> >
> >
> > Juan A. Suarez Romero (9):
> >   core: Create relations between keys
> >   core: Add a new class to handle multivalued properties.
> >   core: Add place to store related keys and values.
> >   core: Add API to handle multivalued data
> >   core: Create relation with key "url"
> >   core: Set an order when creating a relation ship
> >   core: Create property with an initial set of keys
> >   core: Use a representative element when handling multivalued data
> >   core: Add API to get single multivalued elements
> >
> >  src/Makefile.am           |    4 +
> >  src/data/grl-data-multi.c |  511
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  src/data/grl-data-multi.h |  112 ++++++++++
> >  src/data/grl-media.c      |    2 +-
> >  src/data/grl-media.h      |    6 +-
> >  src/data/grl-property.c   |   84 ++++++++
> >  src/data/grl-property.h   |  101 +++++++++
> >  src/grl-metadata-key.c    |   17 ++
> >  src/grl-plugin-registry.c |   89 ++++++++
> >  src/grl-plugin-registry.h |    7 +
> >  10 files changed, 929 insertions(+), 4 deletions(-)
> >  create mode 100644 src/data/grl-data-multi.c
> >  create mode 100644 src/data/grl-data-multi.h
> >  create mode 100644 src/data/grl-property.c
> >  create mode 100644 src/data/grl-property.h
> >
> > _______________________________________________
> > grilo-list mailing list
> > grilo-list gnome org
> > http://mail.gnome.org/mailman/listinfo/grilo-list
> 
> _______________________________________________
> grilo-list mailing list
> grilo-list gnome org
> http://mail.gnome.org/mailman/listinfo/grilo-list
>