We're having two parts here - conventions and extra methods:

= Conventions =

I'd suggest to use the same approach like the preset interface in core uses regarding the locations also with regard how local copies shadow system wide ones.

I further suggest that we would distribute a set of profiles as the gstreamer community (separate package). Distributors can customize that for their needs.

The naming suggestions for "<category>-<name>.profile" sounds good. Having separate sub directories for each category might also work nicely.

= API =

If we enforce the file naming, we could quickly produce a list of profiles for a category from the names. Otherwise having a profile cache would be good.

(In reply to comment #1)
> We're having two parts here - conventions and extra methods:
> 
> = Conventions =
> 
> I'd suggest to use the same approach like the preset interface in core uses
> regarding the locations also with regard how local copies shadow system wide
> ones.

  I'm basing most of my work on the preset system already :)

> 
> I further suggest that we would distribute a set of profiles as the gstreamer
> community (separate package). Distributors can customize that for their needs.

  Agreed. Although it would also make sense to have some distributed in existing gst-plugins-* packages.

> The naming suggestions for "<category>-<name>.profile" sounds good. Having
> separate sub directories for each category might also work nicely.

  Actually <category>/<name>.profile is more flexible. It reduces the limitations of the naming scheme considerably and will make it faster to retrieve all targets for a given category.

> 
> = API =
> 
> If we enforce the file naming, we could quickly produce a list of profiles for
> a category from the names. Otherwise having a profile cache would be good.

  See comment above about directories :) A cache will be nice in the future, but as long as a proper API is designed, we can change what happens under the hood later on.

A first round of patches for review are available in my profileregistry branch here :

http://cgit.freedesktop.org/~bilboed/gst-plugins-base/

I tried to follow all comments above (directories for categories for ex).

In even works on the command line (encodebin profile="target-name/profile-name")

Any comments on this ? Would be nice to have merged before freeze.

Are you using this already somewhere? It still feels a bit unpolished somehow, but can't quite put my finger on it :)

Couple of comments:

 - (existing header): the GST_ENCODING_CATEGORY_* defines
   need a comment block explaining a bit more what they're
   meant to be used for (one or two examples)

 - any API that takes a category_name argument should reference
   one of the GST_ENCODING_CATEGORY_* defines in the gtk-doc
   blurb as example

 - new gst_encoding_target_get_profile() API: couldn't it happen
   that there are profiles with the same name under different
   categories?

 - as an example of what I mean by "unpolished", see the code
   example in the gtk-doc comment which is modified in the
   "encoding-target: Add method to get a profile by name"
   commit: it just seems an odd way to be using this API in the
   first place (very narrowly defined). I would expect almost all
   applications to want to get a list of targets, either for all
   categories or for one specific category, and then expose
   that somehow in a selector, no?

 - there's no give-me-a-list-of-all-targets API, which would be
    the most interesting API here

 - another reason why it seems unpolished is maybe the
   implicitness of the registry stuff - you don't have a
   central object or API to query for targets, that then
   also handles the filesystem stuff, it's all hidden in the
   API. The filesystem is the registry, sorta. Which is fine
   I guess, but we end up having gst_encoding_{profile,target}_*()
   API that's not linked to a profile/target object, which
   easily feels unclean if it's too much :)

 - I think the datadir sub-directory should be called "encoding-profiles"
   (ie. plural)

 - don't we need some kind of environment-variable override, or
   way to allow additional paths, e.g. for an uninstalled setup?

 - not a fan of the ".gstprofile" suffix, should be either something
   with three letters (.gep? .dat?) or just ".profile" IMHO. The path
   already takes care of the 'gst' thing.

 - the description for gst_encoding_target_save() says it will try
   to save in the system directory, but doesn't actually (not sure
   it should either)

 - wonder if load_from/save_to should also have 'file' in the
   function name (it's not clear if path is a directory here or
   a filepath/name); also, people will later likely ask for
   saving to and loading from memory/data instead of files.

Sorry, forgot one:

> "encoding-profile: Add convenience method to find a profileprofileregistry
> Also register GValue functions to be able to use profiles in gst-launch"

 - it's cool to be able to use it from gst-launch I think, and
   it seems ok to abuse the serialisation function here for
   that trick even though we don't actually serialise the
   object itself, but I'm not sure if we should expose
   gst_encoding_profile_find() with the single profilename
   argument where the caller should pass "target/profile-name" -
   wouldn't it be more natural to have two arguments for this and
   let the deserialisation function do the splitting in two?

 - I think the argument name in the gtk-doc blurb is different
   than in the function declaration and/or definition

(In reply to comment #5)
> Are you using this already somewhere? It still feels a bit unpolished somehow,
> but can't quite put my finger on it :)
> 
> Couple of comments:
> 
>  - (existing header): the GST_ENCODING_CATEGORY_* defines
>    need a comment block explaining a bit more what they're
>    meant to be used for (one or two examples)

  Added docs locally.

> 
>  - any API that takes a category_name argument should reference
>    one of the GST_ENCODING_CATEGORY_* defines in the gtk-doc
>    blurb as example

  Added docs locally

> 
>  - new gst_encoding_target_get_profile() API: couldn't it happen
>    that there are profiles with the same name under different
>    categories?

  Yes, but here you're asking a profile from a specific target... which can not contain more than one profile with the same name.

> 
>  - as an example of what I mean by "unpolished", see the code
>    example in the gtk-doc comment which is modified in the
>    "encoding-target: Add method to get a profile by name"
>    commit: it just seems an odd way to be using this API in the
>    first place (very narrowly defined). I would expect almost all
>    applications to want to get a list of targets, either for all
>    categories or for one specific category, and then expose
>    that somehow in a selector, no?

  Updated the example accordingly.

> 
>  - there's no give-me-a-list-of-all-targets API, which would be
>     the most interesting API here

  Add two new methods:
  gst_encoding_target_list_all
  gst_encoding_target_list_available_categories

> 
>  - another reason why it seems unpolished is maybe the
>    implicitness of the registry stuff - you don't have a
>    central object or API to query for targets, that then
>    also handles the filesystem stuff, it's all hidden in the
>    API. The filesystem is the registry, sorta. Which is fine
>    I guess, but we end up having gst_encoding_{profile,target}_*()
>    API that's not linked to a profile/target object, which
>    easily feels unclean if it's too much :)

  Don't see why that would make it 'unpolished'. It could definitely be improved later on, with an extra set of API which takes a 'central' registry object for performance reasons.
  I just went straight to the convenience methods. The addition of the two methods above filled the missing gap.

> 
>  - I think the datadir sub-directory should be called "encoding-profiles"
>    (ie. plural)

  Changed.

> 
>  - don't we need some kind of environment-variable override, or
>    way to allow additional paths, e.g. for an uninstalled setup?
> 
>  - not a fan of the ".gstprofile" suffix, should be either something
>    with three letters (.gep? .dat?) or just ".profile" IMHO. The path
>    already takes care of the 'gst' thing.

  Changed to .gep

> 
>  - the description for gst_encoding_target_save() says it will try
>    to save in the system directory, but doesn't actually (not sure
>    it should either)

  I'll remove the doc part about saving to system directory.

> 
>  - wonder if load_from/save_to should also have 'file' in the
>    function name (it's not clear if path is a directory here or
>    a filepath/name); also, people will later likely ask for
>    saving to and loading from memory/data instead of files.

  Then we will likely later add methods for that if the need arises. Doesn't feel like something important.

(In reply to comment #6)
> Sorry, forgot one:
> 
> > "encoding-profile: Add convenience method to find a profileprofileregistry
> > Also register GValue functions to be able to use profiles in gst-launch"
> 
>  - it's cool to be able to use it from gst-launch I think, and
>    it seems ok to abuse the serialisation function here for
>    that trick even though we don't actually serialise the
>    object itself, but I'm not sure if we should expose
>    gst_encoding_profile_find() with the single profilename
>    argument where the caller should pass "target/profile-name" -
>    wouldn't it be more natural to have two arguments for this and
>    let the deserialisation function do the splitting in two?

  Changed to take two arguments.

> 
>  - I think the argument name in the gtk-doc blurb is different
>    than in the function declaration and/or definition

  fixed


Pushed all updates to my personal git repo

Ok, cool.

>   Add two new methods:
>   gst_encoding_target_list_all

How about

  gst_encoding_target_list_targets()

or

  gst_encoding_target_list_targets_for_category()

instead?

>   gst_encoding_target_list_available_categories

(Still think it's a bit weird to have these functions prefixed with gst_encoding_target, since they're not operating on encoding target objects, but hey).


> >  - wonder if load_from/save_to should also have 'file' in the
> >    function name (it's not clear if path is a directory here or
> >    a filepath/name); also, people will later likely ask for
> >    saving to and loading from memory/data instead of files.
> 
>   Then we will likely later add methods for that if the need arises. Doesn't
> feel like something important.

Still think _load_from_file() + save_to_file() is nicer, but not insisting :)

Let's get this in then!

commit ff92c53ec55b3039b796f9d1cd2f5481eafcbaa5
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Wed Jan 5 20:40:39 2011 +0100

    docs: Add various new symbols

commit f458662ab976c7630ae3891f1607f1c651378e69
Author: Arun Raghavan <arun.raghavan@collabora.co.uk>
Date:   Wed Jan 5 01:50:34 2011 +0530

    encoding-profile: Minor documentation updates

commit 5a8858b3bcba9f45b02c37785f813d1820c7f8f6
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 19:07:45 2011 +0100

    encoding-profile: Give a better usage example

commit 777f816ff162aa7c2d6bd2dbf3b123615e8f91e5
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 18:52:00 2011 +0100

    encoding-target: Fixup loading/saving methods

commit 536849bce572e3272c68e70511665f807a6e33bc
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 18:51:22 2011 +0100

    encoding-target: more docs cleanups

commit 3b32566dd40c6e39ba8a461696f659adb6dc1eba
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 16:07:49 2011 +0100

    encoding-target: Change target suffix to .gep
    
    Along with a bunch of other internal cleanups

commit 520eb442ce714fba31a94172c370295260d7d9da
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 13:21:26 2011 +0100

    encoding-target: Add more docs regarding categories

commit a65faf2f3c687a66f72213d965555c8a4631e2e2
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jan 3 13:20:19 2011 +0100

    encoding-target: Add API for list all categories and targets
    
    API: gst_encoding_list_available_categories
    API: gst_encoding_list_all_targets

commit deea1eb83f5bd07ff45a44a460dc6af590185fc3
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Wed Dec 22 18:18:00 2010 +0100

    encoding-profile: Add convenience method to find a profile
    
    API: gst_encoding_profile_find

commit d8f5b6322fc12d91a15eccfce28365cc7942e573
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Wed Dec 22 18:16:33 2010 +0100

    encoding-target: Implement save/load feature
    
    Fixes #637735

commit c8fa8085ba72538109b1e0a7af1ca399443b5e47
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Wed Dec 22 11:41:41 2010 +0100

    encoding-target: Add method to get a profile by name
    
    API: gst_encoding_target_get_profile