Re: Caps and Options update

[Iago Toral <itoral igalia com>]

On Mon, 04 Apr 2011 20:14:31 +0200, Guillaume Emont <gemont igalia com> 
wrote:
> Following the discussions we've had, I've advanced the development of
> GrlCaps and GrlOperationOptions.
> I've pushed the current state of things to my gitorious repository 
> [1]
> The stuff is in src/grl-caps.{h,c} and 
> src/grl-operation-options.{h,c}.
> I have also added some helper tools to handle GValues in
> grl-value-helper.{h,c}
>
> I'd be happy to have some feedback on that before I start integrating 
> it
> into grl-metadata-source.c.

Had a quick look at the code but I guess we won't see how good of  a 
solutions this is until we do the integration into grl-metadata-source.
Anyway, some comments below:

> Some notes on it though:
>  - I have put caps and pagination in the caps, even though they might
> not be needed here (because we're likely to want these to be 
> mandatory).
> This might change in the future, and is like that mainly to 
> illustrate
> how caps and options interact.

In that case, ok. FWIW, I think of pagination and flags as something 
that are expected to be there for any source, I expect them to be 
options but not caps.

>  - the _set_value() methods are inconsistent in the way they take
> ownership of their parameters. I plan to change that, probably to 
> always
> copy @value, since taking ownership of it is assuming it was created
> using grl_g_value_new(), which might not always be true.
>  - for the options and caps that are defined, I think we should have
> default values. I'm still hesitating on whether to put that in
> initialisation or in getters.

I don't get to see the reason, in which situation do you think default 
values for options/caps would be interesting? From what I see, when a 
cap is not set it can only mean that the source does not support it (so 
no need for a default value), and when a option is not set it can only 
mean the client is not interested in that option at all (so again, no 
need for a default value).

Also:

I would make the naming of the option keys shorter:
GRL_OPERATION_OPTIONS_KEY_SKIP => GRL_OPERATION_OPTION_SKIP

Remember to pad the public structures to prevent ABI breakage. Even if 
you use GLib's private support for objects I think you still need to pad 
the classes.

Iago