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




Hi Juan,

I'll be reviewing this patches, but give me some 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.

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 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.

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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]