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 626533 - Permit to specify configurable properties
Permit to specify configurable properties
Status: RESOLVED OBSOLETE
Product: gnome-video-effects
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME video effects maintainers
GNOME video effects maintainers
Depends on: 626550
Blocks: 627928 638108
 
 
Reported: 2010-08-10 14:55 UTC by Thibault Saunier
Modified: 2021-07-05 12:26 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Thibault Saunier 2010-08-10 14:55:02 UTC
For now, it is not possible to specify GstElement properties which should be configurable.

We should be able to specify them as well as range of possible value.
Comment 1 Brandon Lewis 2010-08-10 15:06:34 UTC
One suggestion: specify properties with paths. 

If your effect description looks something like this:
...
Pipeline = videobalance name=vb

You could refer to the properties on the element with a path:

[Property::saturation]
path = vb::saturation

Or, for nested bins:

Pipeline = ( name="foo" ( name = "bar" videobalance ) )

...

[property::saturaiton]
path = foo::bar::saturation
Comment 2 Brandon Lewis 2010-08-10 15:09:08 UTC
Note: the idea I presented above leaves open the possibility to add other bits of meta-data for each property by specifying additional fields in each property subsection.
Comment 3 Thibault Saunier 2010-08-10 15:21:34 UTC
In the case of hulk we could let the user configure the hue property from -1 to -0.5 (as an example) like that?

[Effect]
Encoding=UTF-8
_Name=Hulk
_Comment=Transform yourself into the amazing Hulk
PipelineDescription=videobalance saturation=1.5 hue=-0.5

[Property::saturation]
path = Hulk::saturation
min = -1
max = -0.5

The type of the property is retrieve trough the GstElement I guess.
Does it make sense?
Comment 4 Brandon Lewis 2010-08-10 15:29:54 UTC
not quite...

1) you need to name the videobalance element so that you can find it later, otherwise the name won't be predictable.

2) you might want to specify the default values for the property in the property subsection, like this:

[Effect]
Encoding=UTF-8
_Name=Hulk
_Comment=Transform yourself into the amazing Hulk
PipelineDescription=videobalance name="Hulk" hue=-0.5

[Property::saturation]
path = Hulk::saturation
min = -1
max = -0.5
default = 1.5
Comment 5 daniel g. siegel 2010-08-10 15:42:53 UTC
what about something like this:

[Effect]
Encoding=UTF-8
_Name=Hulk
_Comment=Transform yourself into the amazing Hulk
PipelineDescription=videobalance name="Hulk" hue=-0.5 saturation=0
Parameters=hue;saturation;
min=-1;-0.5;
max=1;0.5;
default=-0.5;0;

that way you would know exactly which range is for what property
Comment 6 Brandon Lewis 2010-08-10 15:50:04 UTC
what if you have a bin with two elements that both provide an 'alpha' property? For example, videobox and alpha both implement the 'alpha' property.
Comment 7 daniel g. siegel 2010-08-10 15:52:20 UTC
would work if you make both configurable ;)
Comment 8 Brandon Lewis 2010-08-10 16:28:57 UTC
The core of this idea is really pretty simple:

Your pipeline description can be thought of as a hierarchical tree of elements (each element has a parent, and element can be nested inside bins that are arbitrarily deep). Because the elements in each bin must have a unique name, the path gives you a unique way to locate a given element/property.

So two elements in your pipeline can have properties with the same name, and you can still determine which is which because the properties will have different paths. Moreover, it's no problem if the property you need is implemented inside a nested bin. All you have to do is follow the path until you find the element (which will always be the second to last node in the path), then get the property (which is always the last node in the path).
Comment 9 Brandon Lewis 2010-08-10 16:31:49 UTC
also, not all property types will have a min/max value. For example, strings and enums. Your format does not deal with that elegantly, since everything is positional. You need to have a token to signify that a given property doesn't have a min/max/default value which you need to handle as a special case.
Comment 10 daniel g. siegel 2010-08-10 16:42:21 UTC
right. so i think gstreamer is much more keen on figuring out what the properties are. so what about just writing which elements are used and the app can look up them itself?
Comment 11 Thibault Saunier 2010-08-10 16:48:59 UTC
The idea would be to have the ability to change just some properties we choose not all of them for effects that have effect files.

If we look at the first example we could manage so you don't have to handle configurable properties like:

[Effect]
Encoding=UTF-8
_Name=Hulk
_Comment=Transform yourself into the amazing Hulk
PipelineDescription=videobalance name=vb saturation=1.5 hue=-0.5

[Property::saturation]
path = Hulk::saturation
min = -1
max = -0.5
Comment 12 Youness Alaoui 2010-08-11 21:53:49 UTC
Hi,
I've been discussing this already with some people from GStreamer, and it's nice to see a bug report discussing it here. I've proposed something in this bug #626550 (now a dependency of this bug)
Let me copy/paste to you the important bits :

<paste>
I'd like to see some way to define a range of values that can be used for
elements so users can configure them with a slider, or checkbox or something
similar, and also having some kind of specific presets.
Here's a draft of how we could enhance the .effect file format, it might be a
good start for these extended presets :
---------------------------------
[Effect]
Encoding=UTF-8
Name=[The human readable effect name]
Name[LangCode]=[The translated human readable effect name]
...
Comment=[Description of the effect (tooltip?)]
Comment[LangCode]=[Translation of the description of the effect]
...
Category=[The category of your effect, e.g. "Colors".]
PipelineDescription=[A valid GStreamer pipeline]

[Configuration]
Type=[type of configuration, eg."bool", "int", etc..]
Min=[minimal allowed value]
Max=[maximum allowed value]
Element=[name of the element to configure]
Property=[property name to modify for the element]

[Configuration]
....

[Preset]
Name=[name of the preset]
Name[lang]=[translate name of the preset]
Element[Property]=[value for <Property> on <Element>]
Element2[Property2]=[value for <Property2> on <Element2>]
...
---------------------------------
This way, it would be possible to allow user configuration of specific
properties while keeping the configuration in the effect's purpose (example,
changing the hue/saturation of the Hulk effect to change the 'how much green'
but keep the values range inside the right range so that the color doesn't
change from green to something else).
This is just a draft, but I'd like to find some way to link properties
together, for example, if property1 is in the range [x1, y1], then the range of
property2 will be [a1, b1], if it's in the range of [x2, y2], then the range of
property2 will [a2, b2]. This should allow for something like a rotating zoom
where a single slider would affect two properties on two different elements at
the same time.
If someone has a suggestion to achieve this, let me know!
</paste>

The thing with presets is that it's already a GStreamer feature
(http://gstreamer.net/data/doc/gstreamer/head/gstreamer/html/GstPreset.html)
and you could use it to directly load/parse presets from the .effect file.
Currently presets only allow for storing properties of a single element, but it
will hopefully change with this new extended preset bug.
Here's an example of the current GstPreset feature :
http://cgit.freedesktop.org/gstreamer/gst-plugins-ugly/tree/ext/x264/GstX264Enc.prs
The idea would be to have specific presets stored in the .effect file and maybe
allow the UI to just show a combobox so the user can select the preset he
wants, for example, for Hulk, we could get "Nice Hulk"/"Angry Hulk"/"Very angry
Hulk", etc.. which would change the hue/saturation to make it light/dark green.
It will also solve the possible issue of properties that require strings (or
something similar to the 'pattern' property of videotestsrc).
By reusing the same preset format as GstPreset, we'll avoid code duplication
and we'll be able to reuse GstPreset in whatever implementation uses the
.effect files.
Tell me what you think!
Comment 13 Youness Alaoui 2010-08-26 20:35:44 UTC
Hi again,
Daniel suggested to paste an example of this 'new extended format' here so others can see more clearly what I mean, and in order to continue this discussion and brainstorm a bit more on how to improve the format.
I decided to take the Hulk example and extend it (note, the ranges/values for hue/saturation may not be correct, I just put pseudo-random values in there) :
---------------------------------------------------------------
[Effect]
Encoding=UTF-8
Name=Hulk
Name[es]=Hulk
Comment=Transform yourself into the amazing Hulk
Comment[es]=Transformarse en el Increíble Hulk
PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
Configurations=Configuration01
Presets=Preset01, Preset02, Preset03

[Configuration01]
Name=Angriness
Name[fr]=Colère
Comment=Change the angriness of Hulk by making yourself more or less green.
Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
Type=double
Min=1.2
Max=1.6
Element=balance1
Property=saturation
 
[Preset01]
Name=Calm
Name[fr]=Calme
balance1[hue]=-0.3
balance1[saturation]=1.2
 
[Preset02]
Name=Angry
Name[fr]=Enérvé
balance1[hue]=-0.5
balance1[saturation]=1.5
 
[Preset03]
Name=Very angry
Name[fr]=Très énervé
balance1[hue]=-0.7
balance1[saturation]=1.6
-----------------------------------------------------------------

So this is pretty much the same format as I wrote before, but Daniel yesterday told me that we can't have multiple groups in the file with the same name, so I added a 'Presets' and "Configurations' keys in the [Effect] group, where you can list the group names of all configurations/presets.

The configuration is meant to allow the UI to have a slider, a checkbox, whatever, to allow to configure a specific option. The name is to be shown on the UI next to the config, the Comment would be the tooltip to show on mouse over.

The presets are meant to allow the UI to have radio buttons, or a dropdown box, to let the user choose between preset configurations.
A UI could implement/integrate both Configurations and Presets, or just have the slider, or just have the combobox, or completely ignore these and let the effect be used in its default setting.
If you see issues, or want to make it more feature-complete/complex/whatever, please comment!
Comment 14 daniel g. siegel 2010-08-27 11:42:58 UTC
(In reply to comment #13)
> Hi again,
> Daniel suggested to paste an example of this 'new extended format' here so
> others can see more clearly what I mean, and in order to continue this
> discussion and brainstorm a bit more on how to improve the format.
> I decided to take the Hulk example and extend it (note, the ranges/values for
> hue/saturation may not be correct, I just put pseudo-random values in there) :
> ---------------------------------------------------------------
> [Effect]
> Encoding=UTF-8
> Name=Hulk
> Name[es]=Hulk
> Comment=Transform yourself into the amazing Hulk
> Comment[es]=Transformarse en el Increíble Hulk
> PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
> Configurations=Configuration01
> Presets=Preset01, Preset02, Preset03

this is wrong, you would have to use:

Presets=Preset01;Preset02;Preset03;

see http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.0.html#value-types

> 
> [Configuration01]
> Name=Angriness
> Name[fr]=Colère
> Comment=Change the angriness of Hulk by making yourself more or less green.
> Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
> Type=double
> Min=1.2
> Max=1.6
> Element=balance1
> Property=saturation
> 
> [Preset01]
> Name=Calm
> Name[fr]=Calme
> balance1[hue]=-0.3
> balance1[saturation]=1.2
> 
> [Preset02]
> Name=Angry
> Name[fr]=Enérvé
> balance1[hue]=-0.5
> balance1[saturation]=1.5
> 
> [Preset03]
> Name=Very angry
> Name[fr]=Très énervé
> balance1[hue]=-0.7
> balance1[saturation]=1.6
> -----------------------------------------------------------------
> 
> So this is pretty much the same format as I wrote before, but Daniel yesterday
> told me that we can't have multiple groups in the file with the same name, so I
> added a 'Presets' and "Configurations' keys in the [Effect] group, where you
> can list the group names of all configurations/presets.

according to the standard you can:

http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.0.html#group-header

 Multiple groups may not have the same name. 

i just dont know, if glib supports that. i didnt find anything looking at the api

> 
> The configuration is meant to allow the UI to have a slider, a checkbox,
> whatever, to allow to configure a specific option. The name is to be shown on
> the UI next to the config, the Comment would be the tooltip to show on mouse
> over.
> 
> The presets are meant to allow the UI to have radio buttons, or a dropdown box,
> to let the user choose between preset configurations.
> A UI could implement/integrate both Configurations and Presets, or just have
> the slider, or just have the combobox, or completely ignore these and let the
> effect be used in its default setting.
> If you see issues, or want to make it more feature-complete/complex/whatever,
> please comment!

imho, this gets pretty complex right now. if you want to handle dependencies and such (as you do with configurations and presets), im thinking if xml would serve us better here.

also, would there be a way, to get the min/max values automatically by looking up the element?
Comment 15 Youness Alaoui 2010-08-27 18:44:08 UTC
(In reply to comment #14)
> this is wrong, you would have to use:
> 
> Presets=Preset01;Preset02;Preset03;
> 
> see
> http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.0.html#value-types

yes, sure, sorry, I'm a bit new to this format :)

> 
> > So this is pretty much the same format as I wrote before, but Daniel yesterday
> > told me that we can't have multiple groups in the file with the same name, so I
> > added a 'Presets' and "Configurations' keys in the [Effect] group, where you
> > can list the group names of all configurations/presets.
> 
> according to the standard you can:
> 
> http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.0.html#group-header
> 
>  Multiple groups may not have the same name. 
> 
> i just dont know, if glib supports that. i didnt find anything looking at the
> api

Maybe you misread, it says 'may NOT have the same name', so we can't.
> 
> > please comment!
> 
> imho, this gets pretty complex right now. if you want to handle dependencies
> and such (as you do with configurations and presets), im thinking if xml would
> serve us better here.
> 

I'm not sure XML would be a good idea. Although XML is excellent for writing stuff like this, it's a bit more complicated, would force people to use an xml library, and it doesn't make the files very human readable, etc.. this format keeps it human readable and you can parse it easily with glib and like I said, if you don't care about presets/configs, then you can ignore them when parsing the file.

> also, would there be a way, to get the min/max values automatically by looking
> up the element?
Yes, I suppose, but most probably the range will need to be limited by the effect.. but I can imagine some effects that would need to take the full range of the element (like for the timedelay effect). In that case, it could be as simple as :
[Configuration]
type=proxy
Element=timedelay
Property=delay

And by setting the type to 'proxy' it would mean that you would just proxy the property directly into a configuration (so get the real type and range/default/etc.. directory from the element itself).
I suck at names, so 'proxy' might not be good, maybe 'native', or something.. do you suggest a better solution? or another name ?
Comment 16 Luciana Fujii 2010-12-17 01:41:27 UTC
I plan to work on this bug during the GNOME Outreach Program (very soon actually).

Is the format proposed by Youness agreed upon?

I was looking for other ways to name group headers instead of using ConfigurationXX. And I thought we could use the format proposed before - using [Property::Name] - for the group header. The problem I see with that is just that we wouldn't have the key "Name" anymore, which would be bad to fall back for localized values (if I understood how this works).

On the other hand, we can keep the key "Name" which is the Human readable name of the effect and which will be localized and we can use the group header as [Property::ID], where ID can be either the name or something else. When we parse this, ID will be used to refer to the property inside the application while Name can be shown to the user and there is no problem is they are equal.

I still prefer the name Property over Configuration, since Configuration is more vague - Presets are also configurations.

Also, I think having a "proxy" type of property is great. I can't think of a better name for it though.

Transformed example:

[Effect]
Encoding=UTF-8
Name=Hulk
Name[es]=Hulk
Comment=Transform yourself into the amazing Hulk
Comment[es]=Transformarse en el Increíble Hulk
PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1

[Property::angriness]
Name=Angriness
Name[fr]=Colère
Comment=Change the angriness of Hulk by making yourself more or less green.
Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
Type=double
Min=1.2
Max=1.6
Element=balance1
Property=saturation
Comment 17 Youness Alaoui 2010-12-17 02:24:03 UTC
(In reply to comment #16)
> I plan to work on this bug during the GNOME Outreach Program (very soon
> actually).
> 
Great! I'm glad someone has time to work on this :)

> Is the format proposed by Youness agreed upon?
I don't know, noone agreed and noone disagreed as far as I know.

> 
> I was looking for other ways to name group headers instead of using
> ConfigurationXX. And I thought we could use the format proposed before - using
> [Property::Name] - for the group header. The problem I see with that is just
> that we wouldn't have the key "Name" anymore, which would be bad to fall back
> for localized values (if I understood how this works).
> 
> On the other hand, we can keep the key "Name" which is the Human readable name
> of the effect and which will be localized and we can use the group header as
> [Property::ID], where ID can be either the name or something else. When we
> parse this, ID will be used to refer to the property inside the application
> while Name can be shown to the user and there is no problem is they are equal.

Good idea, looks like there is a g_key_file_get_groups, so this would work great and is a much better solution than what I suggested. :)
I like the [Property::ID] much more than [Property::Name] actually, just like you showed in your example, that's perfect.

> 
> I still prefer the name Property over Configuration, since Configuration is
> more vague - Presets are also configurations.

The 'Property' is kind of confusing with gstreamer/gobject properties, so I'm not sure it's really a good thing to use that. Also, Configuration != Preset. The way I see it, a Configuration equals a slider/checkbox to 'configure' the effect, while a Preset equals a combobox (containing all presets).
Presets also set a specific value on element properties, they don't give a range or something like that, and presets can also modify/set properties that aren't/shouldn't be available to the user as a slider configuration. In my example, A preset was modifying the hue and saturation, but the user only had access to change the saturation property of the element.


> 
> Also, I think having a "proxy" type of property is great. I can't think of a
> better name for it though.
> 
> Transformed example:
> 
> [Effect]
> Encoding=UTF-8
> Name=Hulk
> Name[es]=Hulk
> Comment=Transform yourself into the amazing Hulk
> Comment[es]=Transformarse en el Increíble Hulk
> PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
> 
> [Property::angriness]
> Name=Angriness
> Name[fr]=Colère
> Comment=Change the angriness of Hulk by making yourself more or less green.
> Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
> Type=double
> Min=1.2
> Max=1.6
> Element=balance1
> Property=saturation

Much cleaner, thanks, I like the new example.

By the way, do you plan on writing a small library that would come with gnome-video-effects and would do all this parsing for us? this way every application that uses gnome-video-effects won't have to duplicate/rewrite the code to parse the .effect files, and this would also allow modifications to the format to not break other people's implementations of the file parsing if they use the library shipped with g-v-e.
Comment 18 Luciana Fujii 2010-12-17 02:42:19 UTC
Yes, I plan to write a library to parse this and include it in g-v-e. I would have to write it anyway to use new g-v-e on Cheese. I should probably open a new bug report later to discuss if this is desired in g-v-e and discuss my ideas on how to implement it.
Comment 19 Youness Alaoui 2010-12-17 03:03:37 UTC
Good! When you open the bug report, could you make it depend on this one, this way I can see it and add myself in CC to it? I also might be able to help design the API (although it should be fairly simple I think).
Comment 20 daniel g. siegel 2010-12-17 09:42:06 UTC
(In reply to comment #16)

> Transformed example:
> 
> [Effect]
> Encoding=UTF-8
> Name=Hulk
> Name[es]=Hulk
> Comment=Transform yourself into the amazing Hulk
> Comment[es]=Transformarse en el Increíble Hulk
> PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
> 
> [Property::angriness]
> Name=Angriness
> Name[fr]=Colère
> Comment=Change the angriness of Hulk by making yourself more or less green.
> Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
> Type=double
> Min=1.2
> Max=1.6
> Element=balance1
> Property=saturation

is the only way to get such a property then with g_key_file_get_groups? as i probably don't know the name of the property. so maybe we need another
field like youness suggested, where we put all properties:

Presets=angriness;Preset02;Preset03;
Comment 21 Youness Alaoui 2010-12-17 23:17:56 UTC
(In reply to comment #20)
> 
> is the only way to get such a property then with g_key_file_get_groups? as i
> probably don't know the name of the property. so maybe we need another
> field like youness suggested, where we put all properties:
> 
> Presets=angriness;Preset02;Preset03;

Any specific reason why you don't like g_key_file_get_groups? Is it that you just don't like the idea of looping through all the groups and strncmp the 'starts with' "Property::" ?
Comment 22 daniel g. siegel 2010-12-18 01:25:58 UTC
exactly, i would have to iterate through all. although i don't know if this is bad ;)
Comment 23 Luciana Fujii 2010-12-27 13:20:50 UTC
I think if we have properties, but no presets, it's not that bad to iterate through them, you would do something very similar to that even with an extra field. The only case I think would be much better with an extra field would be having many presets and properties when the user only wants properties. If there is an extra field, then it wouldn't need to go through all preset group headers to check if they are properties.

Would anyone use only properties and no presets? I guess in Cheese we would use both.

In another subject, would it be ok to have the Default key to define the default value of a property instead of adding that to the pipeline directly. Would be easier to parse and I even think it's more clear.
Comment 24 Youness Alaoui 2010-12-28 10:51:27 UTC
Well, I'm not sure it would be such a problem even if you have many presets and properties, you won't open the groups or fetch keys or whatever, you'll just do a if (g_str_has_prefix(groups[i], "Property::")) in your loop and it's not something that would make it cpu intensive or anything.

About the Default key, it's a difficult question, my original idea was "if you don't want customizable UI, then ignore those groups, but having the default value not in the pipeline description means that you can't just gst_bin_parse the pipeline, you'll need to parse it, then get elements, then iterate and set their default value, etc.. make things complicated.
I would suggest leaving the pipeline description self-sustainable, with all the proper default values, but have an additional Default key to make it easier for those implementing the customization features to have a "reset to defaults" button for example.. problem this adds is that effect developers need to maintain the default value in both the pipeline description and in the configuration options, and it adds the possibility of having different default values depending on how you parse the thing.. so like I said, difficult question and i'm not sure what the right answer should be.. maybe someone else has a better idea ?
Comment 25 Luciana Fujii 2010-12-30 00:35:49 UTC
(In reply to comment #24)
> About the Default key, it's a difficult question, my original idea was "if you
> don't want customizable UI, then ignore those groups, but having the default
> value not in the pipeline description means that you can't just gst_bin_parse
> the pipeline, you'll need to parse it, then get elements, then iterate and set
> their default value, etc.. make things complicated.

I haven't considered that. It's a good reason to have the default on the pipeline for sure. I think default only in the pipeline is enough. We can parse it and keep it if we want a "set to default" option or something like that.

About having to iterate through groups, I have no strong opinion. I don't think it's a big problem to iterate through groups, but I don't think it's a big problem to add a 'Properties' key to list the properties either (and then you can name the groups anything you want, wouldn't need to start with 'Property::'). Does anyone have strong opinions pro or against each of them?

More details to decide: In the library I'm using types: bool, int, double, string. Is there something missing or wrong with those?

What do you think of changing the Min + Max keys to a single Range key. Would be like: Range=0;1;
Comment 26 Youness Alaoui 2010-12-30 05:47:45 UTC
(In reply to comment #25)
> (In reply to comment #24)
> > About the Default key, it's a difficult question, my original idea was "if you
> > don't want customizable UI, then ignore those groups, but having the default
> > value not in the pipeline description means that you can't just gst_bin_parse
> > the pipeline, you'll need to parse it, then get elements, then iterate and set
> > their default value, etc.. make things complicated.
> 
> I haven't considered that. It's a good reason to have the default on the
> pipeline for sure. I think default only in the pipeline is enough. We can parse
> it and keep it if we want a "set to default" option or something like that.

Actually, you gave me an idea, how about "if you want a "set to default" option, you need a "Preset::default" group". By forcing the name of the group to the 'default' keyword, you can make a preset into the default values to use. What do you think ?

p.s: In which case, not sure if the 'Name' key would still be useful for the default preset, maybe for UIs without a 'default' button?

> 
> About having to iterate through groups, I have no strong opinion. I don't think
> it's a big problem to iterate through groups, but I don't think it's a big
> problem to add a 'Properties' key to list the properties either (and then you
> can name the groups anything you want, wouldn't need to start with
> 'Property::'). Does anyone have strong opinions pro or against each of them?

I don't have a "strong opinion" against that, but I think that iteration on get_groups is better than having a key list all the properties/presets, my reasons being :
- having a prefix makes it easier to read the .effect and there is no confusion on what's a property and what's a preset (no need to read a group name, then go see what they are in the main group, then go back)
- avoid wasting time "why isn't this working" because you forgot to add/change the Properties/Presets key.
- avoid redundancy, why force the maintainance of a key value (list of properties/presets) if we can avoid it.

If there's an "auto generate effect" from a nice UI, then maybe it wouldn't matter, but since the .effect files will be manually written, I think it's important to keep the file format as human-readable/human-maintainable as possible.


> 
> More details to decide: In the library I'm using types: bool, int, double,
> string. Is there something missing or wrong with those?
> 
> What do you think of changing the Min + Max keys to a single Range key. Would
> be like: Range=0;1;

I'm not sure about the types, there would probably be fraction too.. maybe look at what types can be set on a gstreamer element property and make sure to include all of those.
The Range idea seems good, I don't see any reason why not to do it this way.
Comment 27 Luciana Fujii 2011-01-06 20:32:34 UTC
I have talked to Daniel about configurable properties in GNOME Video Effects, let me first summarize what we know, correct me if I'm wrong:

Applications that could use configurable properties include: chat applications, video editing application and libraries, streaming softwares and video players. Even though some applications would use more properties than others, all of them would probably be satisfied with some chosen properties per effect.

All effect properties are mapped to a GStreamer element property, although the min and max values may vary. For instance, angrinness in hulk is mapped to saturation in videobalance element. In this case, the range of angriness is not the same as the range of the saturation property in videobalance, since we want to keep the green. Other examples would be property delaytime in timedelay effect, amplitude and frequency in distortion effect.

Right now we have a key "Type" to define the type of the property. Since the property always corresponds to an element property, I think it would be better to get the type from the element property. Doing that would avoid parsing the type name and would be easier to write effects without knowing the exact type of a property. (If that's a good idea, I accept suggestions on how to do this. I could only think of creating a bin with gst_parse_launch_from_description, than getting the element, its class and the property.)

Daniel was saying he doesn't like the need to name the element in the pipeline description very much. I agree it is not very elegant, but I don't have a better suggestion. Does anyone have?

Changed example so far:

[Effect]
Encoding=UTF-8
Name=Hulk
Name[es]=Hulk
Comment=Transform yourself into the amazing Hulk
Comment[es]=Transformarse en el Increíble Hulk
PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
 
[Property::angriness]
Name=Angriness
Name[fr]=Colère
Comment=Change the angriness of Hulk by making yourself more or less green.
Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
Range=1.2;1.6;
Element=balance1
Property=saturation
Comment 28 Youness Alaoui 2011-01-07 07:20:06 UTC
(In reply to comment #27)
> I have talked to Daniel about configurable properties in GNOME Video Effects,
> let me first summarize what we know, correct me if I'm wrong:
> 
> Applications that could use configurable properties include: chat applications,
> video editing application and libraries, streaming softwares and video players.
> Even though some applications would use more properties than others, all of
> them would probably be satisfied with some chosen properties per effect.
> 
> All effect properties are mapped to a GStreamer element property, although the
> min and max values may vary. For instance, angrinness in hulk is mapped to
> saturation in videobalance element. In this case, the range of angriness is not
> the same as the range of the saturation property in videobalance, since we want
> to keep the green. Other examples would be property delaytime in timedelay
> effect, amplitude and frequency in distortion effect.
Yep, very good summary. But you forgot about the presets which are fixed configurations of all the properties.

> 
> Right now we have a key "Type" to define the type of the property. Since the
> property always corresponds to an element property, I think it would be better
> to get the type from the element property. Doing that would avoid parsing the
> type name and would be easier to write effects without knowing the exact type
> of a property. (If that's a good idea, I accept suggestions on how to do this.
> I could only think of creating a bin with gst_parse_launch_from_description,
> than getting the element, its class and the property.)

Humm.. it's a good point that it can be taken directly from the element, and I say make the type 'proxy' by default if it's not specified... but what about types that you want to specify manually? For example, a string type where you specify "boolean" and then you could have the OnValue and OffValue as strings ?
The type could then mean what kind of widget to use (a bool means a checkbox) rather than the type of property the element accepts, but I don't think it would be useful for anything other than string properties actually. I don't know also how to specify multiple choices apart from the on/offvalue. I have an idea but it seems complicated.. And we definitely don't want to make this format complicated...
Other question to think about is.. is there even a filter element that actually uses strings for its properties?

There's also the idea of having one configuration affect multiple elements, like the rotating-zoom example, you move the slider and it rotates *and* zooms on the image... this could be done by specifying the keys in the property like this :
element1[property1] = min;max;
element1[property2] = min;max;
element2[property3] = min;max;
But again, it will make the format even more complicated and hard to read (by humans).. also how to handle non double/int properties... also, how about if we want to do things like :
element2[property1] = 5 * element1[property1] + 100;
Anyways...

Sorry about the rambling, I just wanted to throw these ideas to cover all the aspects of what we could do with this format, but I do understand that this stuff I just said adds a lot of (probably unnecessary) complication, and since one of the requirements (at least to me) is to keep the format simple, then I'd say, we might as well have a more limited file format and drop these features (multi element property and specifiable Type).

So to summarize, yes, it's all good, remove the Type key, but don't forget about the Presets.

> 
> Daniel was saying he doesn't like the need to name the element in the pipeline
> description very much. I agree it is not very elegant, but I don't have a
> better suggestion. Does anyone have?
> 

Yes, it's not great, but really, I don't see any other solution.. especially considering that the same element can appear multiple times in the pipeline and you may want to modify only one of the two, so you can't just use the element plugin name... I really don't think there is any other way for doing this.
Comment 29 daniel g. siegel 2011-01-07 09:37:33 UTC
(In reply to comment #27)
> 
> [Effect]
> Encoding=UTF-8
> Name=Hulk
> Name[es]=Hulk
> Comment=Transform yourself into the amazing Hulk
> Comment[es]=Transformarse en el Increíble Hulk
> PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
> 
> [Property::angriness]
> Name=Angriness
> Name[fr]=Colère
> Comment=Change the angriness of Hulk by making yourself more or less green.
> Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
> Range=1.2;1.6;
> Element=balance1
> Property=saturation

i don't understand why you want to use Property:: ? afaik you will have a hard time finding all properties without knowing their names. imho it would be much better to do something like this:

[Effect]
Encoding=UTF-8
Name=Hulk
Comment=Transform yourself into the amazing Hulk
PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
Properties=Angriness
 
[Angriness]
Name=Angriness
Comment=Change the angriness of Hulk by making yourself more or less green.
Range=1.2;1.6;
Element=balance1
Property=saturation


second point is what youness mentioned: what should we do with multiple elements/properties? e.g. i can imagine that the angriness of hulk would need saturation _and_ hue.
Comment 30 Luciana Fujii 2011-01-07 11:26:31 UTC
(In reply to comment #29)
> (In reply to comment #27)
> > 
> > [Effect]
> > Encoding=UTF-8
> > Name=Hulk
> > Name[es]=Hulk
> > Comment=Transform yourself into the amazing Hulk
> > Comment[es]=Transformarse en el Increíble Hulk
> > PipelineDescription=videobalance saturation=1.5 hue=-0.5 name=balance1
> > 
> > [Property::angriness]
> > Name=Angriness
> > Name[fr]=Colère
> > Comment=Change the angriness of Hulk by making yourself more or less green.
> > Comment[fr]=Changer la colère de Hulk en changeant votre niveau de vert.
> > Range=1.2;1.6;
> > Element=balance1
> > Property=saturation
> 
> i don't understand why you want to use Property:: ? afaik you will have a hard
> time finding all properties without knowing their names. imho it would be much
> better to do something like this:

In comment 26 Youness described why he thinks this format is better than the other. Would you comment his arguments so you can decide the better way. As I said, I have no strong opinion about it, but I did find his arguments compelling and I don't have any arguments the other way around. Right now I'm using anything in this regard until it's decided.

> second point is what youness mentioned: what should we do with multiple
> elements/properties? e.g. i can imagine that the angriness of hulk would need
> saturation _and_ hue.

Yes. This has been mentioned before and we haven't discussed it really. Should we? I don't think Hulk is a good example for that. Would we change Hue to change the type of green? If so how would this be done inside of angriness? Would it be like, changing angrinees from 0 to 1 would change hue in 0.1 and saturation in 0.2 for instance? We wouldn't be able to have a green with more saturation without changing the hue, right?
Comment 31 Luciana Fujii 2011-01-07 11:42:55 UTC
(In reply to comment #28)
> Yep, very good summary. But you forgot about the presets which are fixed
> configurations of all the properties.

That's true. I'm really focusing on properties first and forgot about it. I hope this will be the easy part.
 
> Humm.. it's a good point that it can be taken directly from the element, and I
> say make the type 'proxy' by default if it's not specified

It wouldn't be exactly what you described in the proxy type, because the range could still be set.

> ... but what about
> types that you want to specify manually? For example, a string type where you
> specify "boolean" and then you could have the OnValue and OffValue as strings ?
> The type could then mean what kind of widget to use (a bool means a checkbox)
> rather than the type of property the element accepts, but I don't think it
> would be useful for anything other than string properties actually.

I didn't really understand how would this be done by the application. It would have to know the types used on the UI and used in the element and map them?

I think the type should be the type of the property itself, not related to the widget to use. Different applications might want to use different UIs, but I don't think they should use different effects just for that.

> I don't
> know also how to specify multiple choices apart from the on/offvalue. I have an
> idea but it seems complicated.. And we definitely don't want to make this
> format complicated...
> Other question to think about is.. is there even a filter element that actually
> uses strings for its properties?

textoverlay is the only example I could think of (no effect in g-v-e so far).
Comment 32 Youness Alaoui 2011-01-07 23:23:10 UTC
(In reply to comment #31)
> (In reply to comment #28)
> > Yep, very good summary. But you forgot about the presets which are fixed
> > configurations of all the properties.
> 
> That's true. I'm really focusing on properties first and forgot about it. I
> hope this will be the easy part.
> 
Sure, focusing on properties first is obvious, I just thought presets should be mentioned in the summary :)

> > Humm.. it's a good point that it can be taken directly from the element, and I
> > say make the type 'proxy' by default if it's not specified
> 
> It wouldn't be exactly what you described in the proxy type, because the range
> could still be set.
Ah, you're right.. so how about, no Type means 'proxy' as default, and no Range/Value specified means use the element propertie's values.

> 
> > ... but what about
> > types that you want to specify manually? For example, a string type where you
> > specify "boolean" and then you could have the OnValue and OffValue as strings ?
> > The type could then mean what kind of widget to use (a bool means a checkbox)
> > rather than the type of property the element accepts, but I don't think it
> > would be useful for anything other than string properties actually.
> 
> I didn't really understand how would this be done by the application. It would
> have to know the types used on the UI and used in the element and map them?
> 
> I think the type should be the type of the property itself, not related to the
> widget to use. Different applications might want to use different UIs, but I
> don't think they should use different effects just for that.

No, that's not what I meant, I meant, for example "bool" would be a checkbox, and "int"/"double" or even better, simply "range" would be for the sliders, "enum" would be for a combobox, etc.. you don't specify the widget type, but rather how the property needs to be modified.
You could have an element's property that is a double, but in the UI you only want a checkbox that sets the property to 0.5 or 0.8 (for example).. so you could say Type=bool, and that's what the UI would show, and that would be independent of what the element's property type actually is.
Was I clear enough? Let me know if it's still unclear.

> 
> > I don't
> > know also how to specify multiple choices apart from the on/offvalue. I have an
> > idea but it seems complicated.. And we definitely don't want to make this
> > format complicated...
> > Other question to think about is.. is there even a filter element that actually
> > uses strings for its properties?
> 
> textoverlay is the only example I could think of (no effect in g-v-e so far).

Good point, then I guess we could just have Type=string which would give an entrybox to the user in the UI.. would be useful for this case.. 
And if we take into account what I just said above about the Type being "how the user can configure this property" rather than being linked to the element's property type, then you could have Type=string for the textoverlay, so an textbox is shown in the UI to configure this property, and for some other hypothetical effect, we could have a 'bool' type (or 'enum') with on/off values (or enum values) specifying specific strings to set.

By the way, I still don't like the name Property, it's really getting confusing here when we talk about element properties versus effect properties.
Also, since the effect properties aren't really properties (they have their own type, one 'effect property' could be manipulating multiple element properties, etc..), I'd say the initial 'Configuration' naming is more appropriate since they represent ways to configure the effect.
Unless you have another name to suggest that isn't confusing, or any reason why "Properties" should still used for naming these configurations.
Comment 33 daniel g. siegel 2011-01-14 15:40:41 UTC
i am generally still not quite happy with the current ideas, as it really get's complicated to write new effects.

let me propose a different way to add configuration to the effects: as most apps probably want to use luciana's parsing library anyway, the idea is this:

we leave the format like it is right now with no configurations whatsoever. in the library we then add the configurations to each effect. so one can simply get the effect, and for example call effect.get_properties() which could be a list containing all properties. this way the format would be still very simple and the complexity could live in a .so for each effect.

what do you think about this?
Comment 34 Youness Alaoui 2011-01-14 16:26:29 UTC
(In reply to comment #33)
> i am generally still not quite happy with the current ideas, as it really get's
> complicated to write new effects.
> 
> let me propose a different way to add configuration to the effects: as most
> apps probably want to use luciana's parsing library anyway, the idea is this:
> 
> we leave the format like it is right now with no configurations whatsoever. in
> the library we then add the configurations to each effect. so one can simply
> get the effect, and for example call effect.get_properties() which could be a
> list containing all properties. this way the format would be still very simple
> and the complexity could live in a .so for each effect.
> 
> what do you think about this?

Humm.. no, I don't think that's a good idea, because then writing new effects will be even more complicated.. write the 'base effect' in the .effect file, then modify the C code to add the configurations there somehow..
also then, you'd need to specify how the code should be handling this... right now the library would only be parsing the .effect file, but if you want the configs in the .so, then you'd need some structure or some way to specify the configuration per effect inside the C code, so you'd still need to write extra code following the 'C format' in order to create new effects.. and then how do you want to library to match them? a strcmp on the filename? or on the effect's Name ? 
Also, the library is to help people parse, but someone might want to write his own parsing library in whatever language he likes, you would make the task impossible for maintaining all that.
Again, like i said, from the start the design of this 'extended format' was to not break the previous format, so if you don't want any configurations, if you just want to write a simple effect, then you don't need to bother at all with this extended format.. the format would not change at all, it would be identical to the current format if you don't add configurations to it, so I don't see where the problem is.
This extended format would only be used by those who want to add it to their effects, so writing a new effect will be just as simple as before.. but writing complex effects becomes a bit more complex.
Comment 35 GNOME Infrastructure Team 2021-07-05 12:26:28 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-video-effects/-/issues/

Thank you for your understanding and your help.