[Gstreamer-openmax] [PATCH 2/2] add gst-openmax registry (config file)

[Clark, Rob]

On Feb 26, 2010, at 3:52 PM, Felipe Contreras wrote:

> On Fri, Feb 26, 2010 at 3:15 PM, Clark, Rob <rob at ti.com> wrote:
>> 
>> On Feb 26, 2010, at 6:05 AM, Felipe Contreras wrote:
>> 
>>> On Thu, Feb 25, 2010 at 3:56 AM, Rob Clark <rob at ti.com> wrote:
>>>> Configurable mapping between gst-openmax elements and OMX library-name and component-name via config file.  The config file may even create multiple different elements with unique names
>>>> using the same gst-openmax class but different OMX library-name and/or component-name.  For example:
>>> 
>>> In general the patch looks ok, but:
>>> 
>>> 1) I don't like this new class creation, probably the only reason it's
>>> needed is because the library-name and component-name are set in the
>>> GType, which is not needed. We can set these into the element itself
>>> without modifying the original GType.
>> 
>> 
>> We can set them as properties on the GstElement, but we still need some way to differentiate when the same element is used for two different OMX components in order to set the correct property values..  we can use the name property on the element, because this can be set to anything by the user (and at a minimum, would end up something like "omx_dummy0")
>> 
>> I originally tried without creating new classes, but ran into this problem in the case that we had two different elements with same GType but different OMX components.  So I don't see any better solution.
> 
> Yeah, now I see that it can be problematic. But I still don't like
> omx_dummy as a GType, I think the proper GType should also be
> specified on the config: GstOmxDummy.


so something like:

-----
omx_dummy,
  type=GstOmxDummy,
  derived-type=GstOmxDummyOne,
  library-name=libomxil-bellagio.so,
  component-name=OMX.st.dummy,
  rank=0;

omx_dummy_2,
  type=GstOmxDummy,
  derived-type=GstOmxDummyTwo,
  library-name=libomxil-bellagio.so,
  component-name=OMX.bellagio.dummy,
  rank=256;
-----

it does make the config file more complex (one more thing to misconfigure), and then we need a bit more error handling, in case user specifies a type name that already exists..  so I'm not super-excited about that.

Also, then we need an additional hash-table to map back to element name (or something like g_type_set_qdata())..  both is possible, but it makes the code slightly less simple.

using the element name as the type name seems a simple solution to both.


> 
>>> 2) gstomx_conf.c looks a bit tricky, I was thinking on a plain text
>>> file, just like gst-openmax.conf. I guess the .c file is ok for now,
>>> but I don't like this tricky Makefile rule, let's just have a
>>> gstomx_conf.c instead.
>> 
>> I think we need something that ends up as a .c file, for the #ifndef EXPERIMENTAL stuff.. and also so we have some default config embedded in libgstomx.so.
>> 
>> That said, it could be a plain gstomx_conf.c file..  I just thought the plain text .conf file generating the .c file was easier to edit.  But I can change it back to plain .c file if you don't like my clever makefile rule ;-)
> 
> Now that you show the plain .c file I understand why it's not nice to
> use that. I feel there must be a better way to handle this, but for
> now I think you original proposal is ok (except the license notice,
> which I think is overkill).


ok, I can go back to original version of default config .conf file

BR,
-R



> 
> -- 
> Felipe Contreras