gstreamer-openmax — Development list for GStreamer OpenMax plugins

[Gstreamer-openmax] [PATCH 0/4] Plugin initialization improvements

[Felipe C. <fel...@no...>]

Hi,

The first patches reorganize the way the element_table is initialized, and once
that's done, it's straigh-forward to add support for cache reloads when the
config changes, and plugin cache.

These patches apply on top of the sytem-wide config patch series.

Felipe Contreras (4):
  plugin: reorganize element_table init
  plugin: make element_table const
  plugin: add support dependencies
  plugin: store element_table in plugin cache

 omx/gstomx.c |   76 +++++++++++++++++++++++++++------------------------------
 1 files changed, 36 insertions(+), 40 deletions(-)

[Gstreamer-openmax] [PATCH 4/4] plugin: store element_table in plugin cache

[Felipe C. <fel...@no...>]

Probably more efficient.

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index cc15ecf..11cd18b 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -130,8 +130,6 @@ get_config_path (void)
                              "gst-openmax.conf", NULL);
 }
 
-/* TODO: we can cache table w/ gst_plugin_{get,set}_cache_data..
- */
 static void
 fetch_element_table (GstPlugin *plugin)
 {
@@ -139,6 +137,11 @@ fetch_element_table (GstPlugin *plugin)
     gchar *config, *s;
     GstStructure *new, *element;
 
+    element_table = gst_plugin_get_cache_data (plugin);
+
+    if (element_table)
+        return;
+
     path = get_config_path ();
 
     if (!g_file_get_contents (path, &config, NULL, NULL))
@@ -170,6 +173,8 @@ fetch_element_table (GstPlugin *plugin)
 
     GST_DEBUG ("element_table=%" GST_PTR_FORMAT, new);
 
+    gst_plugin_set_cache_data (plugin, new);
+
     element_table = new;
 }
 
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 2/4] plugin: make element_table const

[Felipe C. <fel...@no...>]

Will be useful in next patches, but it's good in itself. Also, no need
to inialize to zero global variables.

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index e9b0e70..3fbb240 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -63,7 +63,7 @@
 
 GST_DEBUG_CATEGORY (gstomx_debug);
 
-static GstStructure *element_table = NULL;
+const static GstStructure *element_table;
 static GQuark element_name_quark;
 
 extern const gchar *default_config;
@@ -141,7 +141,7 @@ fetch_element_table (void)
 {
     gchar *path;
     gchar *config, *s;
-    GstStructure *element;
+    GstStructure *new, *element;
 
     path = get_config_path ();
 
@@ -155,21 +155,23 @@ fetch_element_table (void)
 
     GST_DEBUG ("parsing config:\n%s", config);
 
-    element_table = gst_structure_empty_new ("element_table");
+    new = gst_structure_empty_new ("element_table");
 
     s = config;
 
     while ((element = gst_structure_from_string (s, &s)))
     {
         const gchar *element_name = gst_structure_get_name (element);
-        gst_structure_set (element_table,
+        gst_structure_set (new,
                            element_name, GST_TYPE_STRUCTURE, element, NULL);
     }
 
     if (config != default_config)
         g_free (config);
 
-    GST_DEBUG ("element_table=%" GST_PTR_FORMAT, element_table);
+    GST_DEBUG ("element_table=%" GST_PTR_FORMAT, new);
+
+    element_table = new;
 }
 
 static GstStructure *
@@ -177,7 +179,7 @@ get_element_entry (const gchar *element_name)
 {
     GstStructure *element;
 
-    if (!gst_structure_get (element_table, element_name,
+    if (!gst_structure_get ((GstStructure *) element_table, element_name,
                             GST_TYPE_STRUCTURE, &element, NULL))
     {
         element = NULL;
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 1/4] plugin: reorganize element_table init

[Felipe C. <fel...@no...>]

plugin_init() is *always* called, so we can assume element_table will
always be initialize once.

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |   60 ++++++++++++++++++++++++---------------------------------
 1 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index a27b0b3..e9b0e70 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -136,54 +136,45 @@ get_config_path (void)
 
 /* TODO: we can cache table w/ gst_plugin_{get,set}_cache_data..
  */
-static GstStructure *
-get_element_table (void)
+static void
+fetch_element_table (void)
 {
-    static volatile gsize gonce_data = 0;
-    if (g_once_init_enter (&gonce_data))
-    {
-        gchar *path;
-        gchar *config, *s;
-        GstStructure *element;
-
-        path = get_config_path ();
-
-        if (!g_file_get_contents (path, &config, NULL, NULL))
-        {
-            g_warning ("could not find config file '%s'.. using defaults!", path);
-            config = (gchar *) default_config;
-        }
-
-        g_free (path);
+    gchar *path;
+    gchar *config, *s;
+    GstStructure *element;
 
-        GST_DEBUG ("parsing config:\n%s", config);
+    path = get_config_path ();
 
-        element_table = gst_structure_empty_new ("element_table");
+    if (!g_file_get_contents (path, &config, NULL, NULL))
+    {
+        g_warning ("could not find config file '%s'.. using defaults!", path);
+        config = (gchar *) default_config;
+    }
 
-        s = config;
+    g_free (path);
 
-        while ((element = gst_structure_from_string (s, &s)))
-        {
-            const gchar *element_name = gst_structure_get_name (element);
-            gst_structure_set (element_table,
-                    element_name, GST_TYPE_STRUCTURE, element, NULL);
-        }
+    GST_DEBUG ("parsing config:\n%s", config);
 
-        if (config != default_config)
-            g_free (config);
+    element_table = gst_structure_empty_new ("element_table");
 
-        GST_DEBUG ("element_table=%" GST_PTR_FORMAT, element_table);
+    s = config;
 
-        g_once_init_leave (&gonce_data, 1);
+    while ((element = gst_structure_from_string (s, &s)))
+    {
+        const gchar *element_name = gst_structure_get_name (element);
+        gst_structure_set (element_table,
+                           element_name, GST_TYPE_STRUCTURE, element, NULL);
     }
 
-    return element_table;
+    if (config != default_config)
+        g_free (config);
+
+    GST_DEBUG ("element_table=%" GST_PTR_FORMAT, element_table);
 }
 
 static GstStructure *
 get_element_entry (const gchar *element_name)
 {
-    GstStructure *element_table = get_element_table ();
     GstStructure *element;
 
     if (!gst_structure_get (element_table, element_name,
@@ -227,7 +218,6 @@ static gboolean
 plugin_init (GstPlugin *plugin)
 {
     gint i, cnt;
-    GstStructure *element_table;
 
     GST_DEBUG_CATEGORY_INIT (gstomx_debug, "omx", 0, "gst-openmax");
     GST_DEBUG_CATEGORY_INIT (gstomx_util_debug, "omx_util", 0, "gst-openmax utility");
@@ -241,7 +231,7 @@ plugin_init (GstPlugin *plugin)
     for (i = 0; i < G_N_ELEMENTS (get_type); i++)
         get_type[i] ();
 
-    element_table = get_element_table ();
+    fetch_element_table ();
 
     g_omx_init ();
 
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 3/4] plugin: add support dependencies

[Felipe C. <fel...@no...>]

So that the cache is reloaded when needed.

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index 3fbb240..cc15ecf 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -130,14 +130,10 @@ get_config_path (void)
                              "gst-openmax.conf", NULL);
 }
 
-/**
- * @todo find a way to call plugin_init() when the config file changes
- */
-
 /* TODO: we can cache table w/ gst_plugin_{get,set}_cache_data..
  */
 static void
-fetch_element_table (void)
+fetch_element_table (GstPlugin *plugin)
 {
     gchar *path;
     gchar *config, *s;
@@ -151,6 +147,9 @@ fetch_element_table (void)
         config = (gchar *) default_config;
     }
 
+    gst_plugin_add_dependency_simple (plugin, "ONX_CONFIG", path, NULL,
+                                      GST_PLUGIN_DEPENDENCY_FLAG_NONE);
+
     g_free (path);
 
     GST_DEBUG ("parsing config:\n%s", config);
@@ -233,7 +232,7 @@ plugin_init (GstPlugin *plugin)
     for (i = 0; i < G_N_ELEMENTS (get_type); i++)
         get_type[i] ();
 
-    fetch_element_table ();
+    fetch_element_table (plugin);
 
     g_omx_init ();
 
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 2/3] plugin: add support for system-wide config

[Felipe C. <fel...@no...>]

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index cb26eef..71bafd9 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -117,13 +117,21 @@ get_config_path (void)
     if (path)
         return path;
 
+    dirs = g_get_system_config_dirs ();
+    for (i = 0; dirs[i]; i++)
+    {
+        path = g_build_filename (dirs[i], "gstreamer-0.10", "gst-openmax.conf", NULL);
+        if (g_file_test (path, G_FILE_TEST_IS_REGULAR))
+            return path;
+        g_free (path);
+    }
+
     return g_build_filename (g_get_user_config_dir (),
                              "gst-openmax.conf", NULL);
 }
 
 /**
  * @todo find a way to call plugin_init() when the config file changes
- * @todo support a system-wide config file
  * @todo provide a recommended system-wide config file
  */
 
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 0/3] More configuration stuff

[Felipe C. <fel...@no...>]

Hi,

Here are some patches to implement system-wide configuration.

For reference, by default the configuration is in:
/etc/xdg/gstreamer-0.10/gst-openmax.conf

I'm also working on gst_plugin_add_dependency() so that the registry is
updating when the configuration changes. I already have a simple patch, but
first I want to investigate how the plugin loading really works; it seems
plugin_init() is always called.

Cheers.

Felipe Contreras (3):
  plugin: reorganize code into get_config_path()
  plugin: add support for system-wide config
  Add example configuration

 example.conf |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 omx/gstomx.c |   34 +++++++++++++++++++++++++-------
 2 files changed, 85 insertions(+), 8 deletions(-)
 create mode 100644 example.conf

[Gstreamer-openmax] [PATCH 3/3] Add example configuration

[Felipe C. <fel...@no...>]

Signed-off-by: Felipe Contreras <fel...@no...>
---
 example.conf |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 omx/gstomx.c |    1 -
 2 files changed, 59 insertions(+), 1 deletions(-)
 create mode 100644 example.conf

diff --git a/example.conf b/example.conf
new file mode 100644
index 0000000..a6f7de2
--- /dev/null
+++ b/example.conf
@@ -0,0 +1,59 @@
+omx_mpeg4dec,
+  type=GstOmxMpeg4Dec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_decoder.mpeg4,
+  rank=256;
+
+omx_h264dec,
+  type=GstOmxH264Dec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_decoder.avc,
+  rank=256;
+
+omx_h263dec,
+  type=GstOmxH263Dec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_decoder.h263,
+  rank=256;
+
+omx_wmvdec,
+  type=GstOmxWmvDec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_decoder.wmv,
+  rank=256;
+
+omx_mpeg4enc,
+  type=GstOmxMpeg4Enc,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_encoder.mpeg4,
+  rank=256;
+
+omx_h264enc,
+  type=GstOmxH264Enc,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_encoder.avc,
+  rank=256;
+
+omx_h263enc,
+  type=GstOmxH263Enc,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.video_encoder.h263,
+  rank=256;
+
+omx_vorbisdec,
+  type=GstOmxVorbisDec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.audio_decoder.ogg.single,
+  rank=128;
+
+omx_mp3dec,
+  type=GstOmxMp3Dec,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.audio_decoder.mp3.mad,
+  rank=256;
+
+omx_audiosink,
+  type=GstOmxAudioSink,
+  library-name=libomxil-bellagio.so.0,
+  component-name=OMX.st.alsa.alsasink,
+  rank=0;
diff --git a/omx/gstomx.c b/omx/gstomx.c
index 71bafd9..a27b0b3 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -132,7 +132,6 @@ get_config_path (void)
 
 /**
  * @todo find a way to call plugin_init() when the config file changes
- * @todo provide a recommended system-wide config file
  */
 
 /* TODO: we can cache table w/ gst_plugin_{get,set}_cache_data..
-- 
1.7.0.2

[Gstreamer-openmax] [PATCH 1/3] plugin: reorganize code into get_config_path()

[Felipe C. <fel...@no...>]

Will be useful in next commits to extend to more config options.

Signed-off-by: Felipe Contreras <fel...@no...>
---
 omx/gstomx.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index cc8f3f8..cb26eef 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -105,6 +105,22 @@ static GType (*get_type[]) (void) = {
     gst_omx_volume_get_type,
 };
 
+static gchar *
+get_config_path (void)
+{
+    gchar *path;
+    const gchar *const *dirs;
+    int i;
+
+    path = g_strdup (g_getenv ("OMX_CONFIG"));
+
+    if (path)
+        return path;
+
+    return g_build_filename (g_get_user_config_dir (),
+                             "gst-openmax.conf", NULL);
+}
+
 /**
  * @todo find a way to call plugin_init() when the config file changes
  * @todo support a system-wide config file
@@ -123,12 +139,7 @@ get_element_table (void)
         gchar *config, *s;
         GstStructure *element;
 
-        path = g_strdup (g_getenv ("OMX_CONFIG"));
-        if (!path)
-        {
-            path = g_build_filename (g_get_user_config_dir (),
-                    "gst-openmax.conf", NULL);
-        }
+        path = get_config_path ();
 
         if (!g_file_get_contents (path, &config, NULL, NULL))
         {
-- 
1.7.0.2

Re: [Gstreamer-openmax] [PATCH 06/10] add g_omx_core_{get, set}_{config, param} helper functions

[Rob C. <ro...@ti...>]

On Mar 8, 2010, at 11:04 AM, Felipe Contreras wrote:

> On Mon, Mar 08, 2010 at 12:14:53AM +0100, Rob Clark wrote:
>> The helper functions consolidate error handling.
>> ---
> 
>> +void
>> +g_omx_core_get_param (GOmxCore *core, OMX_INDEXTYPE idx, OMX_PTR param)
> 
> Wouldn't it make sense to return the error?


I thought about that.. but at the moment there were no places that were handling errors from {Get,Set}{Parameter,Config}.

So I thought the least intrusive change was simply to do a GST_WARNING trace.

It would be a simple change later to also return the error if there was some places where we wanted to handle the error.


> 
>> +{
>> +    OMX_HANDLETYPE omx_handle = g_omx_core_get_handle (core);
>> +    OMX_ERRORTYPE err;
>> +
>> +    if (!omx_handle)
>> +        return;
> 
> I think this check should be done much sooner.


??? I guess I don't understand this point.  The check is the first thing after the g_omx_core_get_handle() call, so I don't see how it could be any sooner?

There is already an error trace in g_omx_core_get_handle(), so in this case we just need to make the function return before we call a null pointer.

The use case here is if you do a gst-inspect on an element which doesn't have a corresponding OMX component..  it is ok that the element does not work properly, but it isn't ok that gst-inspect segfaults.  This becomes relevant in the next patch where input/output-buffers properties are added.


BR,
-R


> 
>> +    err = OMX_GetParameter (omx_handle, idx, param);
>> +
>> +    if (OMX_ErrorNone != err)
> 
> The other way around: err != OMX_ErrorNone
> 
>> +        GST_WARNING_OBJECT (core->object, "OMX_GetParameter(%d) failed: %s",
>> +                idx, omx_error_to_str (err));
>> +}
> 
> -- 
> Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 1/2] use G_PARAM_STATIC_STRINGS

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 04:50:12PM +0100, Rob Clark wrote:
> 
> On Mar 8, 2010, at 9:36 AM, Felipe Contreras wrote:
> 
> > On Sat, Mar 06, 2010 at 10:03:57PM +0100, Rob Clark wrote:
> >> as suggested by Stefan Kost
> >> ---
> > 
> > This patch doesn't apply, and it's weird because it says 1/2, but I
> > don't see the 0/2 patch.
> 
> It should apply on top of your 'remove unused set_property()' patch..
> or at least that is the theory.
> 
> I made the patch on top of your patches which had not been committed
> yet, because I figured they would be committed soon, and that would be
> easier than rebasing..  Have a look at:

Ok, you should mention that kind of stuff. It seems to apply fine there.

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 06/10] add g_omx_core_{get, set}_{config, param} helper functions

[Felipe C. <fel...@No...>]

On Mon, Mar 08, 2010 at 12:14:53AM +0100, Rob Clark wrote:
> The helper functions consolidate error handling.
> ---

> +void
> +g_omx_core_get_param (GOmxCore *core, OMX_INDEXTYPE idx, OMX_PTR param)

Wouldn't it make sense to return the error?

> +{
> +    OMX_HANDLETYPE omx_handle = g_omx_core_get_handle (core);
> +    OMX_ERRORTYPE err;
> +
> +    if (!omx_handle)
> +        return;

I think this check should be done much sooner.

> +    err = OMX_GetParameter (omx_handle, idx, param);
> +
> +    if (OMX_ErrorNone != err)

The other way around: err != OMX_ErrorNone

> +        GST_WARNING_OBJECT (core->object, "OMX_GetParameter(%d) failed: %s",
> +                idx, omx_error_to_str (err));
> +}

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 01/10] construct GOmxPort objects in element constructor

[Rob C. <ro...@ti...>]

On Mar 8, 2010, at 9:57 AM, Felipe Contreras wrote:

> On Mon, Mar 08, 2010 at 12:14:48AM +0100, Rob Clark wrote:
>> The port objects are constructed up front (so they are valid at any point in the element's lifecycle), and bound to a port_index at construction time.
>> 
>> This will be useful later to enable properties that directly set port params.
> 
> It's not clear to me what this patch is doing, and why.


Basically it is creating port in element constructor, instead of later at setup_ports() time..

The internal function within gstomx_util.c used to access ports (g_omx_core_get_port()) is renamed to get_port() because it is not public API.  A new g_omx_core_get_port() which constructs the port if it has not already been constructed is created.  (So maybe more correctly it would be g_omx_core_get_port_or_construct_new_port_if_it_does_not_exist() ;-))

But since this is only used from type_instance_init() fxns, it could be replaced by g_omx_core_new() which does not check if the port already exists yet or not and is simply a constructor function.

BR,
-R


> 
>> ---
>> omx/gstomx_base_filter.c |   13 +++++------
>> omx/gstomx_base_sink.c   |   10 +++-----
>> omx/gstomx_base_src.c    |   10 +++-----
>> omx/gstomx_util.c        |   47 +++++++++++++++++++--------------------------
>> omx/gstomx_util.h        |    4 +-
>> 5 files changed, 36 insertions(+), 48 deletions(-)
>> 
>> diff --git a/omx/gstomx_base_filter.c b/omx/gstomx_base_filter.c
>> index 44841c3..ddb480f 100644
>> --- a/omx/gstomx_base_filter.c
>> +++ b/omx/gstomx_base_filter.c
>> @@ -64,14 +64,14 @@ setup_ports (GstOmxBaseFilter *self)
>> 
>>    param.nPortIndex = 0;
>>    OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
>> -    self->in_port = g_omx_core_setup_port (core, &param);
>> +    g_omx_port_setup (self->in_port, &param);
>>    gst_pad_set_element_private (self->sinkpad, self->in_port);
>> 
>>    /* Output port configuration. */
>> 
>>    param.nPortIndex = 1;
>>    OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
>> -    self->out_port = g_omx_core_setup_port (core, &param);
>> +    g_omx_port_setup (self->out_port, &param);
>>    gst_pad_set_element_private (self->srcpad, self->out_port);
>> 
>>    if (g_getenv ("OMX_ALLOCATE_ON"))
>> @@ -877,11 +877,10 @@ type_instance_init (GTypeInstance *instance,
>>    self->use_timestamps = TRUE;
>> 
>>    /* GOmx */
>> -    {
>> -        GOmxCore *gomx;
>> -        self->gomx = gomx = g_omx_core_new (self);
>> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
>> -    }
>> +    self->gomx = g_omx_core_new (self);
>> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
>> +    self->in_port = g_omx_core_get_port (self->gomx, 0);
>> +    self->out_port = g_omx_core_get_port (self->gomx, 1);
> 
> This code reorganization was not advertised, and it makes the actual
> change more difficult to see.
> 
>> 
>>    self->ready_lock = g_mutex_new ();
>> 
>> diff --git a/omx/gstomx_base_sink.c b/omx/gstomx_base_sink.c
>> index 6838f08..1c7888c 100644
>> --- a/omx/gstomx_base_sink.c
>> +++ b/omx/gstomx_base_sink.c
>> @@ -52,7 +52,7 @@ setup_ports (GstOmxBaseSink *self)
>> 
>>    param.nPortIndex = 0;
>>    OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
>> -    self->in_port = g_omx_core_setup_port (core, &param);
>> +    g_omx_port_setup (self->in_port, &param);
>>    gst_pad_set_element_private (self->sinkpad, self->in_port);
>> }
>> 
>> @@ -417,11 +417,9 @@ type_instance_init (GTypeInstance *instance,
>>    GST_LOG_OBJECT (self, "begin");
>> 
>>    /* GOmx */
>> -    {
>> -        GOmxCore *gomx;
>> -        self->gomx = gomx = g_omx_core_new (self);
>> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
>> -    }
>> +    self->gomx = g_omx_core_new (self);
>> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
>> +    self->in_port = g_omx_core_get_port (self->gomx, 0);
> 
> Unneeded reorganization.
> 
>> 
>>    {
>>        GstPad *sinkpad;
>> diff --git a/omx/gstomx_base_src.c b/omx/gstomx_base_src.c
>> index dd2be9e..a19c08f 100644
>> --- a/omx/gstomx_base_src.c
>> +++ b/omx/gstomx_base_src.c
>> @@ -43,7 +43,7 @@ setup_ports (GstOmxBaseSrc *self)
>> 
>>    param.nPortIndex = 0;
>>    OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
>> -    self->out_port = g_omx_core_setup_port (core, &param);
>> +    g_omx_port_setup (self->out_port, &param);
>> 
>>    if (self->setup_ports)
>>    {
>> @@ -402,11 +402,9 @@ type_instance_init (GTypeInstance *instance,
>>    GST_LOG_OBJECT (self, "begin");
>> 
>>    /* GOmx */
>> -    {
>> -        GOmxCore *gomx;
>> -        self->gomx = gomx = g_omx_core_new (self);
>> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
>> -    }
>> +    self->gomx = g_omx_core_new (self);
>> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
>> +    self->out_port = g_omx_core_get_port (self->gomx, 1);
> 
> Unneeded reorganization.
> 
>> 
>>    GST_LOG_OBJECT (self, "end");
>> }
>> diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
>> index b2a96c8..d8cc7f0 100644
>> --- a/omx/gstomx_util.c
>> +++ b/omx/gstomx_util.c
>> @@ -76,9 +76,7 @@ omx_state_to_str (OMX_STATETYPE omx_state);
>> static inline const char *
>> omx_error_to_str (OMX_ERRORTYPE omx_error);
>> 
>> -static inline GOmxPort *
>> -g_omx_core_get_port (GOmxCore *core,
>> -                     guint index);
>> +static inline GOmxPort * get_port (GOmxCore *core, guint index);
>> 
>> static inline void
>> port_free_buffers (GOmxPort *port);
>> @@ -133,7 +131,7 @@ core_for_each_port (GOmxCore *core,
>>    {
>>        GOmxPort *port;
>> 
>> -        port = g_omx_core_get_port (core, index);
>> +        port = get_port (core, index);
> 
> g_omx_core_get_port() != get_port()
> 
> So this theoretically is changing the behavior, and that wasn't
> mentioned in the commit message.
> 
> I think moving from g_omx_core_get_port() to get_port() is a good idea,
> but perhaps should be a separate patch.
> 
>> 
>>        if (port)
>>            func (port);
>> @@ -396,37 +394,30 @@ g_omx_core_unload (GOmxCore *core)
>>    g_ptr_array_clear (core->ports);
>> }
>> 
>> -GOmxPort *
>> -g_omx_core_setup_port (GOmxCore *core,
>> -                       OMX_PARAM_PORTDEFINITIONTYPE *omx_port)
>> +static inline GOmxPort *
>> +get_port (GOmxCore *core, guint index)
>> {
>> -    GOmxPort *port;
>> -    guint index;
>> -
>> -    index = omx_port->nPortIndex;
>> -    port = g_omx_core_get_port (core, index);
>> -
>> -    if (!port)
>> +    if (G_LIKELY (index < core->ports->len))
>>    {
>> -        port = g_omx_port_new (core);
>> -        g_ptr_array_insert (core->ports, index, port);
>> +        return g_ptr_array_index (core->ports, index);
>>    }
>> 
>> -    g_omx_port_setup (port, omx_port);
>> -
>> -    return port;
>> +    return NULL;
>> }
>> 
>> -static inline GOmxPort *
>> +GOmxPort *
>> g_omx_core_get_port (GOmxCore *core,
>>                     guint index)
>> {
>> -    if (G_LIKELY (index < core->ports->len))
>> +    GOmxPort *port = get_port (core, index);
>> +
>> +    if (!port)
>>    {
>> -        return g_ptr_array_index (core->ports, index);
>> +        port = g_omx_port_new (core, index);
>> +        g_ptr_array_insert (core->ports, index, port);
> 
> You are changing the semantics of the function; now it's not getting a
> port; it's creating a port. It's more like g_omx_core_new_port().
> 
> -- 
> Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 05/10] add G_OMX_INIT_PARAM utility macro

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 12:14:52AM +0100, Rob Clark wrote:
> ---
>  omx/gstomx_aacdec.c        |    7 +------
>  omx/gstomx_aacenc.c        |   22 ++++------------------
>  omx/gstomx_adpcmdec.c      |    7 +------
>  omx/gstomx_adpcmenc.c      |   12 ++----------
>  omx/gstomx_amrnbdec.c      |    7 +------
>  omx/gstomx_amrnbenc.c      |   12 ++----------
>  omx/gstomx_amrwbdec.c      |    7 +------
>  omx/gstomx_amrwbenc.c      |   12 ++----------
>  omx/gstomx_audiosink.c     |    7 +------
>  omx/gstomx_base_filter.c   |    7 ++-----
>  omx/gstomx_base_sink.c     |    7 ++-----
>  omx/gstomx_base_src.c      |    7 ++-----
>  omx/gstomx_base_videodec.c |   18 +++---------------
>  omx/gstomx_base_videoenc.c |   13 ++++---------
>  omx/gstomx_g711dec.c       |    7 ++-----
>  omx/gstomx_g711enc.c       |    7 ++-----
>  omx/gstomx_g729enc.c       |    7 +------
>  omx/gstomx_h263enc.c       |    7 +------
>  omx/gstomx_h264enc.c       |    7 +------
>  omx/gstomx_jpegenc.c       |   20 ++++----------------
>  omx/gstomx_mp2dec.c        |    7 +------
>  omx/gstomx_mp3dec.c        |    7 +------
>  omx/gstomx_mpeg4enc.c      |    7 +------
>  omx/gstomx_util.h          |    9 +++++++++
>  omx/gstomx_videosink.c     |   17 ++++-------------
>  omx/gstomx_volume.c        |    7 +------
>  omx/gstomx_vorbisdec.c     |    7 +------
>  27 files changed, 57 insertions(+), 204 deletions(-)

Looks fine to me,

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 04/10] add some debug traces

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 12:14:51AM +0100, Rob Clark wrote:
> ---
>  omx/gstomx_util.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
> index efe953d..b6633e4 100644
> --- a/omx/gstomx_util.c
> +++ b/omx/gstomx_util.c
> @@ -157,6 +157,9 @@ imp_new (const gchar *name)
>          void *handle;
>  
>          imp->dl_handle = handle = dlopen (name, RTLD_LAZY);
> +
> +        GST_DEBUG ("dlopen(%s) -> %p", name, handle);

I think this should be done before the dlopen(). Suppose dlopen()
crashes; we want the debug to appear before it does.

The rest looks ok.

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 03/10] add accessor for OMX_HANDLE

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 12:14:50AM +0100, Rob Clark wrote:
> A method for GOmxCore object which can be called at any time to access
> OMX component handle.  If the OMX component is not yet instantiated,
> this will trigger OMX_GetHandle().  This makes it safe to use in
> places like get/set_property methods which could be called before the
> OMX component would otherwise be instantiated.
> ---

NAK! I like self-documenting code, and a g_omx_core_get_handle() that
also initializes stuff is not really a 'get'; if anything it's
g_omx_core_get_and_initialize_if_not_initialized_handle(), and nedless
to say, that doesn't look nice.

I think I already said this before; if the core needs to be initialized
before; it should be initialized before, manually, and visibly. I think
that was the point of the config stuff; so that we can initialize the
core sooner.

Cheers.

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 01/10] construct GOmxPort objects in element constructor

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 12:14:48AM +0100, Rob Clark wrote:
> The port objects are constructed up front (so they are valid at any point in the element's lifecycle), and bound to a port_index at construction time.
> 
> This will be useful later to enable properties that directly set port params.

It's not clear to me what this patch is doing, and why.

> ---
>  omx/gstomx_base_filter.c |   13 +++++------
>  omx/gstomx_base_sink.c   |   10 +++-----
>  omx/gstomx_base_src.c    |   10 +++-----
>  omx/gstomx_util.c        |   47 +++++++++++++++++++--------------------------
>  omx/gstomx_util.h        |    4 +-
>  5 files changed, 36 insertions(+), 48 deletions(-)
> 
> diff --git a/omx/gstomx_base_filter.c b/omx/gstomx_base_filter.c
> index 44841c3..ddb480f 100644
> --- a/omx/gstomx_base_filter.c
> +++ b/omx/gstomx_base_filter.c
> @@ -64,14 +64,14 @@ setup_ports (GstOmxBaseFilter *self)
>  
>      param.nPortIndex = 0;
>      OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
> -    self->in_port = g_omx_core_setup_port (core, &param);
> +    g_omx_port_setup (self->in_port, &param);
>      gst_pad_set_element_private (self->sinkpad, self->in_port);
>  
>      /* Output port configuration. */
>  
>      param.nPortIndex = 1;
>      OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
> -    self->out_port = g_omx_core_setup_port (core, &param);
> +    g_omx_port_setup (self->out_port, &param);
>      gst_pad_set_element_private (self->srcpad, self->out_port);
>  
>      if (g_getenv ("OMX_ALLOCATE_ON"))
> @@ -877,11 +877,10 @@ type_instance_init (GTypeInstance *instance,
>      self->use_timestamps = TRUE;
>  
>      /* GOmx */
> -    {
> -        GOmxCore *gomx;
> -        self->gomx = gomx = g_omx_core_new (self);
> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
> -    }
> +    self->gomx = g_omx_core_new (self);
> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
> +    self->in_port = g_omx_core_get_port (self->gomx, 0);
> +    self->out_port = g_omx_core_get_port (self->gomx, 1);

This code reorganization was not advertised, and it makes the actual
change more difficult to see.

>  
>      self->ready_lock = g_mutex_new ();
>  
> diff --git a/omx/gstomx_base_sink.c b/omx/gstomx_base_sink.c
> index 6838f08..1c7888c 100644
> --- a/omx/gstomx_base_sink.c
> +++ b/omx/gstomx_base_sink.c
> @@ -52,7 +52,7 @@ setup_ports (GstOmxBaseSink *self)
>  
>      param.nPortIndex = 0;
>      OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
> -    self->in_port = g_omx_core_setup_port (core, &param);
> +    g_omx_port_setup (self->in_port, &param);
>      gst_pad_set_element_private (self->sinkpad, self->in_port);
>  }
>  
> @@ -417,11 +417,9 @@ type_instance_init (GTypeInstance *instance,
>      GST_LOG_OBJECT (self, "begin");
>  
>      /* GOmx */
> -    {
> -        GOmxCore *gomx;
> -        self->gomx = gomx = g_omx_core_new (self);
> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
> -    }
> +    self->gomx = g_omx_core_new (self);
> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
> +    self->in_port = g_omx_core_get_port (self->gomx, 0);

Unneeded reorganization.

>  
>      {
>          GstPad *sinkpad;
> diff --git a/omx/gstomx_base_src.c b/omx/gstomx_base_src.c
> index dd2be9e..a19c08f 100644
> --- a/omx/gstomx_base_src.c
> +++ b/omx/gstomx_base_src.c
> @@ -43,7 +43,7 @@ setup_ports (GstOmxBaseSrc *self)
>  
>      param.nPortIndex = 0;
>      OMX_GetParameter (core->omx_handle, OMX_IndexParamPortDefinition, &param);
> -    self->out_port = g_omx_core_setup_port (core, &param);
> +    g_omx_port_setup (self->out_port, &param);
>  
>      if (self->setup_ports)
>      {
> @@ -402,11 +402,9 @@ type_instance_init (GTypeInstance *instance,
>      GST_LOG_OBJECT (self, "begin");
>  
>      /* GOmx */
> -    {
> -        GOmxCore *gomx;
> -        self->gomx = gomx = g_omx_core_new (self);
> -        gstomx_get_component_info (gomx, G_TYPE_FROM_CLASS (g_class));
> -    }
> +    self->gomx = g_omx_core_new (self);
> +    gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
> +    self->out_port = g_omx_core_get_port (self->gomx, 1);

Unneeded reorganization.

>  
>      GST_LOG_OBJECT (self, "end");
>  }
> diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
> index b2a96c8..d8cc7f0 100644
> --- a/omx/gstomx_util.c
> +++ b/omx/gstomx_util.c
> @@ -76,9 +76,7 @@ omx_state_to_str (OMX_STATETYPE omx_state);
>  static inline const char *
>  omx_error_to_str (OMX_ERRORTYPE omx_error);
>  
> -static inline GOmxPort *
> -g_omx_core_get_port (GOmxCore *core,
> -                     guint index);
> +static inline GOmxPort * get_port (GOmxCore *core, guint index);
>  
>  static inline void
>  port_free_buffers (GOmxPort *port);
> @@ -133,7 +131,7 @@ core_for_each_port (GOmxCore *core,
>      {
>          GOmxPort *port;
>  
> -        port = g_omx_core_get_port (core, index);
> +        port = get_port (core, index);

g_omx_core_get_port() != get_port()

So this theoretically is changing the behavior, and that wasn't
mentioned in the commit message.

I think moving from g_omx_core_get_port() to get_port() is a good idea,
but perhaps should be a separate patch.

>  
>          if (port)
>              func (port);
> @@ -396,37 +394,30 @@ g_omx_core_unload (GOmxCore *core)
>      g_ptr_array_clear (core->ports);
>  }
>  
> -GOmxPort *
> -g_omx_core_setup_port (GOmxCore *core,
> -                       OMX_PARAM_PORTDEFINITIONTYPE *omx_port)
> +static inline GOmxPort *
> +get_port (GOmxCore *core, guint index)
>  {
> -    GOmxPort *port;
> -    guint index;
> -
> -    index = omx_port->nPortIndex;
> -    port = g_omx_core_get_port (core, index);
> -
> -    if (!port)
> +    if (G_LIKELY (index < core->ports->len))
>      {
> -        port = g_omx_port_new (core);
> -        g_ptr_array_insert (core->ports, index, port);
> +        return g_ptr_array_index (core->ports, index);
>      }
>  
> -    g_omx_port_setup (port, omx_port);
> -
> -    return port;
> +    return NULL;
>  }
>  
> -static inline GOmxPort *
> +GOmxPort *
>  g_omx_core_get_port (GOmxCore *core,
>                       guint index)
>  {
> -    if (G_LIKELY (index < core->ports->len))
> +    GOmxPort *port = get_port (core, index);
> +
> +    if (!port)
>      {
> -        return g_ptr_array_index (core->ports, index);
> +        port = g_omx_port_new (core, index);
> +        g_ptr_array_insert (core->ports, index, port);

You are changing the semantics of the function; now it's not getting a
port; it's creating a port. It's more like g_omx_core_new_port().

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 1/2] use G_PARAM_STATIC_STRINGS

[Clark, R. <ro...@ti...>]

On Mar 8, 2010, at 9:36 AM, Felipe Contreras wrote:

> On Sat, Mar 06, 2010 at 10:03:57PM +0100, Rob Clark wrote:
>> as suggested by Stefan Kost
>> ---
> 
> This patch doesn't apply, and it's weird because it says 1/2, but I
> don't see the 0/2 patch.

It should apply on top of your 'remove unused set_property()' patch..  or at least that is the theory.

I made the patch on top of your patches which had not been committed yet, because I figured they would be committed soon, and that would be easier than rebasing..  Have a look at:

  http://gitorious.org/robclark-gstreamer/gst-openmax/commits/patches-1c

to see the order of patches

BR,
-R


> 
>> --- a/omx/gstomx_base_filter.c
>> +++ b/omx/gstomx_base_filter.c
>> @@ -260,17 +260,17 @@ type_class_init (gpointer g_class,
>>         g_object_class_install_property (gobject_class, ARG_COMPONENT_NAME,
>>                                          g_param_spec_string ("component-name", "Component name",
>>                                                               "Name of the OpenMAX IL component to use",
>> -                                                              NULL, G_PARAM_READABLE));
>> +                                                              NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
>> 
>>         g_object_class_install_property (gobject_class, ARG_LIBRARY_NAME,
>>                                          g_param_spec_string ("library-name", "Library name",
>>                                                               "Name of the OpenMAX IL implementation library to use",
>> -                                                              NULL, G_PARAM_READABLE));
>> +                                                              NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
>> 
>>         g_object_class_install_property (gobject_class, ARG_USE_TIMESTAMPS,
>>                                          g_param_spec_boolean ("use-timestamps", "Use timestamps",
>>                                                                "Whether or not to use timestamps",
>> -                                                               TRUE, G_PARAM_READWRITE));
>> +                                                               TRUE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
>>     }
>> }
>> 
> 
> 
> -- 
> Felipe Contreras

Re: [Gstreamer-openmax] Trick Mode / Pause-Resume issues

[Felipe C. <fel...@no...>]

On Mon, Mar 08, 2010 at 12:36:47PM +0100, ext Nitin PAI wrote:
> Does the current gst-openmax stack support seeking / play-pause?

It depends for which elements; decoders, yes; sinks/sources; don't know.

> I wrote a small gstreamer based test application that plays a
> audio/video file.  This tries to do play/pause and seeking.  There
> were some problems with pause/resume but with some fixes in
> gstomx_base_sink.c (handling PAUSE_TO_RESUME state change)
> pause/resume worked fine.

I never spent much time with sinks/sources, I implemented a few just as
proof-of-concept.

> I am still facing trouble doing seeking. Basically the app hangs in
> the gstreamer code in the waiting for preroll to be done.  Can you
> tell me if the gst-openmax stack is tested for all this?

Nope, it's not tested for that. In Nokia we tested only
encoders/decoders, and they worked fine for the main use-cases, but
nowadays we are concentrating on gst-dsp.

About trick-mode; I've no idea, but my guess it that wouldn't work.

Patches are welcome, though :)

Cheers.

-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH 1/2] use G_PARAM_STATIC_STRINGS

[Felipe C. <fel...@no...>]

On Sat, Mar 06, 2010 at 10:03:57PM +0100, Rob Clark wrote:
> as suggested by Stefan Kost
> ---

This patch doesn't apply, and it's weird because it says 1/2, but I
don't see the 0/2 patch.

> --- a/omx/gstomx_base_filter.c
> +++ b/omx/gstomx_base_filter.c
> @@ -260,17 +260,17 @@ type_class_init (gpointer g_class,
>          g_object_class_install_property (gobject_class, ARG_COMPONENT_NAME,
>                                           g_param_spec_string ("component-name", "Component name",
>                                                                "Name of the OpenMAX IL component to use",
> -                                                              NULL, G_PARAM_READABLE));
> +                                                              NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
> 
>          g_object_class_install_property (gobject_class, ARG_LIBRARY_NAME,
>                                           g_param_spec_string ("library-name", "Library name",
>                                                                "Name of the OpenMAX IL implementation library to use",
> -                                                              NULL, G_PARAM_READABLE));
> +                                                              NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
> 
>          g_object_class_install_property (gobject_class, ARG_USE_TIMESTAMPS,
>                                           g_param_spec_boolean ("use-timestamps", "Use timestamps",
>                                                                 "Whether or not to use timestamps",
> -                                                               TRUE, G_PARAM_READWRITE));
> +                                                               TRUE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
>      }
>  }
> 


-- 
Felipe Contreras

Re: [Gstreamer-openmax] [PATCH] fixes for out-of-tree build

[Felipe C. <fel...@no...>]

On Sat, Mar 06, 2010 at 07:20:47PM +0100, Rob Clark wrote:
> Normally with autotools, you should be able to create a directory outside of the srctree, and then run something like:
> 
>   ../path/to/gst-openmax/configure ....
>   make check install
> 
> but a couple small tweaks were needed to make check work properly.  Mainly replacing Makefile with a Makefile.in, and avoiding relative paths to files in the src tree.

Minor nitpicks about the commit message:

 * If there's no submodule prefix, begin the summary with a capital
   letter.
 * The summary should be at most 50 characters long, while the rest of
   the message 80.
 * If possible, explain in the summary what does the patch *do*, not
   what it is

This ensures that it looks fine across different git tools, and email.

Something like:

 * Fix out-of-tree build
 * build: fixes for out-of-tree

Anyway, I'll fix that and commit. Thanks!

-- 
Felipe Contreras

[Gstreamer-openmax] Trick Mode / Pause-Resume issues

[Nitin P. <nit...@gm...>]

Hi Felipe,

Does the current gst-openmax stack support seeking / play-pause?

I wrote a small gstreamer based test application that plays a audio/video
file.  This tries to do play/pause and seeking.
There were some problems with pause/resume but with some fixes in
gstomx_base_sink.c (handling PAUSE_TO_RESUME state change) pause/resume
worked fine.

I am still facing trouble doing seeking. Basically the app hangs in the
gstreamer code in the waiting for preroll to be done.
Can you tell me if the gst-openmax stack is tested for all this?

Thanks,
Nitin

[Gstreamer-openmax] [PATCH 10/10] Add GstOmxBaseAudioDec base class

[Rob C. <ro...@ti...>]

Refactor some common functionality, in particular the settings_changed_cb, into an abstract base class.
---
 omx/Makefile.am            |    1 +
 omx/gstomx_aacdec.c        |   45 +--------------------
 omx/gstomx_aacdec.h        |    6 +-
 omx/gstomx_adpcmdec.c      |    1 +
 omx/gstomx_amrnbdec.c      |   48 +---------------------
 omx/gstomx_amrnbdec.h      |    6 +-
 omx/gstomx_amrwbdec.c      |   48 +---------------------
 omx/gstomx_amrwbdec.h      |    6 +-
 omx/gstomx_base_audiodec.c |   97 ++++++++++++++++++++++++++++++++++++++++++++
 omx/gstomx_base_audiodec.h |   53 ++++++++++++++++++++++++
 omx/gstomx_g711dec.c       |    1 +
 omx/gstomx_g729dec.c       |    4 +-
 omx/gstomx_g729dec.h       |    6 +-
 omx/gstomx_ilbcdec.c       |    1 +
 omx/gstomx_mp2dec.c        |   55 +------------------------
 omx/gstomx_mp2dec.h        |    6 +-
 omx/gstomx_mp3dec.c        |   56 +-------------------------
 omx/gstomx_mp3dec.h        |    6 +-
 omx/gstomx_vorbisdec.c     |   45 +--------------------
 omx/gstomx_vorbisdec.h     |    6 +-
 20 files changed, 183 insertions(+), 314 deletions(-)
 create mode 100644 omx/gstomx_base_audiodec.c
 create mode 100644 omx/gstomx_base_audiodec.h

diff --git a/omx/Makefile.am b/omx/Makefile.am
index 80e7d9a..7e9f81f 100644
--- a/omx/Makefile.am
+++ b/omx/Makefile.am
@@ -6,6 +6,7 @@ libgstomx_la_SOURCES = gstomx.c gstomx.h \
 		       gstomx_base_filter.c gstomx_base_filter.h \
 		       gstomx_base_videodec.c gstomx_base_videodec.h \
 		       gstomx_base_videoenc.c gstomx_base_videoenc.h \
+		       gstomx_base_audiodec.c gstomx_base_audiodec.h \
 		       gstomx_dummy.c gstomx_dummy.h \
 		       gstomx_volume.c gstomx_volume.h \
 		       gstomx_mpeg4dec.c gstomx_mpeg4dec.h \
diff --git a/omx/gstomx_aacdec.c b/omx/gstomx_aacdec.c
index 782ec2f..1916216 100644
--- a/omx/gstomx_aacdec.c
+++ b/omx/gstomx_aacdec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_aacdec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxAacDec, gst_omx_aacdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxAacDec, gst_omx_aacdec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -127,46 +126,6 @@ type_class_init (gpointer g_class,
 {
 }
 
-static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "channels", G_TYPE_INT, channels,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
 static gboolean
 sink_setcaps (GstPad *pad,
               GstCaps *caps)
@@ -204,7 +163,5 @@ type_instance_init (GTypeInstance *instance,
 
     omx_base = GST_OMX_BASE_FILTER (instance);
 
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
-
     gst_pad_set_setcaps_function (omx_base->sinkpad, sink_setcaps);
 }
diff --git a/omx/gstomx_aacdec.h b/omx/gstomx_aacdec.h
index b53b030..1b431fd 100644
--- a/omx/gstomx_aacdec.h
+++ b/omx/gstomx_aacdec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxAacDec GstOmxAacDec;
 typedef struct GstOmxAacDecClass GstOmxAacDecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxAacDec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxAacDecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_aacdec_get_type (void);
diff --git a/omx/gstomx_adpcmdec.c b/omx/gstomx_adpcmdec.c
index 8826387..f665bf0 100644
--- a/omx/gstomx_adpcmdec.c
+++ b/omx/gstomx_adpcmdec.c
@@ -23,6 +23,7 @@
 #include "gstomx_base_filter.h"
 #include "gstomx.h"
 
+/* should this class extend GstOmxBaseAudioDec? */
 GSTOMX_BOILERPLATE (GstOmxAdpcmDec, gst_omx_adpcmdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
 
 static GstCaps *
diff --git a/omx/gstomx_amrnbdec.c b/omx/gstomx_amrnbdec.c
index 600a513..0362731 100644
--- a/omx/gstomx_amrnbdec.c
+++ b/omx/gstomx_amrnbdec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_amrnbdec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxAmrNbDec, gst_omx_amrnbdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxAmrNbDec, gst_omx_amrnbdec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -101,52 +100,7 @@ type_class_init (gpointer g_class,
 }
 
 static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "channels", G_TYPE_INT, channels,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
-static void
 type_instance_init (GTypeInstance *instance,
                     gpointer g_class)
 {
-    GstOmxBaseFilter *omx_base;
-
-    omx_base = GST_OMX_BASE_FILTER (instance);
-
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
 }
diff --git a/omx/gstomx_amrnbdec.h b/omx/gstomx_amrnbdec.h
index 3ec66c2..781e4ed 100644
--- a/omx/gstomx_amrnbdec.h
+++ b/omx/gstomx_amrnbdec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxAmrNbDec GstOmxAmrNbDec;
 typedef struct GstOmxAmrNbDecClass GstOmxAmrNbDecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxAmrNbDec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxAmrNbDecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_amrnbdec_get_type (void);
diff --git a/omx/gstomx_amrwbdec.c b/omx/gstomx_amrwbdec.c
index 2e77188..581b3da 100644
--- a/omx/gstomx_amrwbdec.c
+++ b/omx/gstomx_amrwbdec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_amrwbdec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxAmrWbDec, gst_omx_amrwbdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxAmrWbDec, gst_omx_amrwbdec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -101,52 +100,7 @@ type_class_init (gpointer g_class,
 }
 
 static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "channels", G_TYPE_INT, channels,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
-static void
 type_instance_init (GTypeInstance *instance,
                     gpointer g_class)
 {
-    GstOmxBaseFilter *omx_base;
-
-    omx_base = GST_OMX_BASE_FILTER (instance);
-
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
 }
diff --git a/omx/gstomx_amrwbdec.h b/omx/gstomx_amrwbdec.h
index ae0a2cf..61870b8 100644
--- a/omx/gstomx_amrwbdec.h
+++ b/omx/gstomx_amrwbdec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxAmrWbDec GstOmxAmrWbDec;
 typedef struct GstOmxAmrWbDecClass GstOmxAmrWbDecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxAmrWbDec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxAmrWbDecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_amrwbdec_get_type (void);
diff --git a/omx/gstomx_base_audiodec.c b/omx/gstomx_base_audiodec.c
new file mode 100644
index 0000000..45037a6
--- /dev/null
+++ b/omx/gstomx_base_audiodec.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2009 Texas Instruments, Inc - http://www.ti.com/
+ *
+ * Description: Base audio decoder element
+ *  Created on: Aug 2, 2009
+ *      Author: Rob Clark <ro...@ti...>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#include "gstomx_base_audiodec.h"
+#include "gstomx.h"
+
+GSTOMX_BOILERPLATE (GstOmxBaseAudioDec, gst_omx_base_audiodec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+
+static void
+type_base_init (gpointer g_class)
+{
+}
+
+static void
+type_class_init (gpointer g_class,
+                 gpointer class_data)
+{
+}
+
+static void
+settings_changed_cb (GOmxCore *core)
+{
+    GstOmxBaseFilter *omx_base;
+    guint rate;
+    guint channels;
+
+    omx_base = core->object;
+
+    GST_DEBUG_OBJECT (omx_base, "settings changed");
+
+    {
+        OMX_AUDIO_PARAM_PCMMODETYPE param;
+
+        G_OMX_INIT_PARAM (param);
+
+        param.nPortIndex = omx_base->out_port->port_index;
+        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
+
+        rate = param.nSamplingRate;
+        channels = param.nChannels;
+        if (rate == 0)
+        {
+            /** @todo: this shouldn't happen. */
+            GST_WARNING_OBJECT (omx_base, "Bad samplerate");
+            rate = 44100;
+        }
+    }
+
+    {
+        GstCaps *new_caps;
+
+        new_caps = gst_caps_new_simple ("audio/x-raw-int",
+                                        "width", G_TYPE_INT, 16,
+                                        "depth", G_TYPE_INT, 16,
+                                        "rate", G_TYPE_INT, rate,
+                                        "signed", G_TYPE_BOOLEAN, TRUE,
+                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
+                                        "channels", G_TYPE_INT, channels,
+                                        NULL);
+
+        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
+        gst_pad_set_caps (omx_base->srcpad, new_caps);
+    }
+}
+
+static void
+type_instance_init (GTypeInstance *instance,
+                    gpointer g_class)
+{
+    GstOmxBaseFilter *omx_base;
+
+    omx_base = GST_OMX_BASE_FILTER (instance);
+
+    GST_DEBUG_OBJECT (omx_base, "start");
+
+    omx_base->gomx->settings_changed_cb = settings_changed_cb;
+}
diff --git a/omx/gstomx_base_audiodec.h b/omx/gstomx_base_audiodec.h
new file mode 100644
index 0000000..83472e6
--- /dev/null
+++ b/omx/gstomx_base_audiodec.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2009 Texas Instruments, Inc - http://www.ti.com/
+ *
+ * Description: Base audio decoder element
+ *  Created on: Aug 2, 2009
+ *      Author: Rob Clark <ro...@ti...>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#ifndef GSTOMX_BASE_AUDIODEC_H
+#define GSTOMX_BASE_AUDIODEC_H
+
+#include <gst/gst.h>
+
+G_BEGIN_DECLS
+
+#define GST_OMX_BASE_AUDIODEC(obj) (GstOmxBaseAudioDec *) (obj)
+#define GST_OMX_BASE_AUDIODEC_TYPE (gst_omx_base_audiodec_get_type ())
+
+typedef struct GstOmxBaseAudioDec GstOmxBaseAudioDec;
+typedef struct GstOmxBaseAudioDecClass GstOmxBaseAudioDecClass;
+
+#include "gstomx_base_filter.h"
+
+struct GstOmxBaseAudioDec
+{
+    GstOmxBaseFilter omx_base;
+};
+
+struct GstOmxBaseAudioDecClass
+{
+    GstOmxBaseFilterClass parent_class;
+};
+
+GType gst_omx_base_audiodec_get_type (void);
+
+G_END_DECLS
+
+#endif /* GSTOMX_BASE_AUDIODEC_H */
diff --git a/omx/gstomx_g711dec.c b/omx/gstomx_g711dec.c
index cd11e4a..c6c7db0 100644
--- a/omx/gstomx_g711dec.c
+++ b/omx/gstomx_g711dec.c
@@ -25,6 +25,7 @@
 
 #include <string.h> /* for strcmp */
 
+/* should this class extend GstOmxBaseAudioDec? */
 GSTOMX_BOILERPLATE (GstOmxG711Dec, gst_omx_g711dec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
 
 static GstCaps *
diff --git a/omx/gstomx_g729dec.c b/omx/gstomx_g729dec.c
index 666d5ab..a558011 100644
--- a/omx/gstomx_g729dec.c
+++ b/omx/gstomx_g729dec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_g729dec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxG729Dec, gst_omx_g729dec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxG729Dec, gst_omx_g729dec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -105,6 +104,7 @@ type_class_init (gpointer g_class,
 {
 }
 
+/* should we be overriding the settings_changed_cb from parent class like this?? */
 static void
 settings_changed_cb (GOmxCore *core)
 {
diff --git a/omx/gstomx_g729dec.h b/omx/gstomx_g729dec.h
index abe5504..49e525f 100644
--- a/omx/gstomx_g729dec.h
+++ b/omx/gstomx_g729dec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxG729Dec GstOmxG729Dec;
 typedef struct GstOmxG729DecClass GstOmxG729DecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxG729Dec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxG729DecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_g729dec_get_type (void);
diff --git a/omx/gstomx_ilbcdec.c b/omx/gstomx_ilbcdec.c
index 9d03f01..ff899ac 100644
--- a/omx/gstomx_ilbcdec.c
+++ b/omx/gstomx_ilbcdec.c
@@ -23,6 +23,7 @@
 #include "gstomx_base_filter.h"
 #include "gstomx.h"
 
+/* should this class extend GstOmxBaseAudioDec? */
 GSTOMX_BOILERPLATE (GstOmxIlbcDec, gst_omx_ilbcdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
 
 static GstCaps *
diff --git a/omx/gstomx_mp2dec.c b/omx/gstomx_mp2dec.c
index 7f7c312..319ef8e 100644
--- a/omx/gstomx_mp2dec.c
+++ b/omx/gstomx_mp2dec.c
@@ -23,7 +23,7 @@
 #include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxMp2Dec, gst_omx_mp2dec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxMp2Dec, gst_omx_mp2dec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -104,60 +104,7 @@ type_class_init (gpointer g_class,
 }
 
 static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-        if (rate == 0)
-        {
-            /** @todo: this shouldn't happen. */
-            GST_WARNING_OBJECT (omx_base, "Bad samplerate");
-            rate = 44100;
-        }
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "channels", G_TYPE_INT, channels,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
-static void
 type_instance_init (GTypeInstance *instance,
                     gpointer g_class)
 {
-    GstOmxBaseFilter *omx_base;
-
-    omx_base = GST_OMX_BASE_FILTER (instance);
-
-    GST_DEBUG_OBJECT (omx_base, "start");
-
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
 }
diff --git a/omx/gstomx_mp2dec.h b/omx/gstomx_mp2dec.h
index 877d3c5..c6c517e 100644
--- a/omx/gstomx_mp2dec.h
+++ b/omx/gstomx_mp2dec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxMp2Dec GstOmxMp2Dec;
 typedef struct GstOmxMp2DecClass GstOmxMp2DecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxMp2Dec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxMp2DecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_mp2dec_get_type (void);
diff --git a/omx/gstomx_mp3dec.c b/omx/gstomx_mp3dec.c
index e7bf6ec..8a1f465 100644
--- a/omx/gstomx_mp3dec.c
+++ b/omx/gstomx_mp3dec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_mp3dec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxMp3Dec, gst_omx_mp3dec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxMp3Dec, gst_omx_mp3dec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -104,60 +103,7 @@ type_class_init (gpointer g_class,
 }
 
 static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-        if (rate == 0)
-        {
-            /** @todo: this shouldn't happen. */
-            GST_WARNING_OBJECT (omx_base, "Bad samplerate");
-            rate = 44100;
-        }
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "channels", G_TYPE_INT, channels,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
-static void
 type_instance_init (GTypeInstance *instance,
                     gpointer g_class)
 {
-    GstOmxBaseFilter *omx_base;
-
-    omx_base = GST_OMX_BASE_FILTER (instance);
-
-    GST_DEBUG_OBJECT (omx_base, "start");
-
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
 }
diff --git a/omx/gstomx_mp3dec.h b/omx/gstomx_mp3dec.h
index f353ee5..c0a7d78 100644
--- a/omx/gstomx_mp3dec.h
+++ b/omx/gstomx_mp3dec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxMp3Dec GstOmxMp3Dec;
 typedef struct GstOmxMp3DecClass GstOmxMp3DecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxMp3Dec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxMp3DecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_mp3dec_get_type (void);
diff --git a/omx/gstomx_vorbisdec.c b/omx/gstomx_vorbisdec.c
index d405dfa..afd53fd 100644
--- a/omx/gstomx_vorbisdec.c
+++ b/omx/gstomx_vorbisdec.c
@@ -20,10 +20,9 @@
  */
 
 #include "gstomx_vorbisdec.h"
-#include "gstomx_base_filter.h"
 #include "gstomx.h"
 
-GSTOMX_BOILERPLATE (GstOmxVorbisDec, gst_omx_vorbisdec, GstOmxBaseFilter, GST_OMX_BASE_FILTER_TYPE);
+GSTOMX_BOILERPLATE (GstOmxVorbisDec, gst_omx_vorbisdec, GstOmxBaseAudioDec, GST_OMX_BASE_AUDIODEC_TYPE);
 
 static GstCaps *
 generate_src_template (void)
@@ -99,46 +98,6 @@ type_class_init (gpointer g_class,
 }
 
 static void
-settings_changed_cb (GOmxCore *core)
-{
-    GstOmxBaseFilter *omx_base;
-    guint rate;
-    guint channels;
-
-    omx_base = core->object;
-
-    GST_DEBUG_OBJECT (omx_base, "settings changed");
-
-    {
-        OMX_AUDIO_PARAM_PCMMODETYPE param;
-
-        G_OMX_INIT_PARAM (param);
-
-        param.nPortIndex = omx_base->out_port->port_index;
-        g_omx_core_get_param (omx_base->gomx, OMX_IndexParamAudioPcm, &param);
-
-        rate = param.nSamplingRate;
-        channels = param.nChannels;
-    }
-
-    {
-        GstCaps *new_caps;
-
-        new_caps = gst_caps_new_simple ("audio/x-raw-int",
-                                        "rate", G_TYPE_INT, rate,
-                                        "signed", G_TYPE_BOOLEAN, TRUE,
-                                        "channels", G_TYPE_INT, channels,
-                                        "endianness", G_TYPE_INT, G_BYTE_ORDER,
-                                        "width", G_TYPE_INT, 16,
-                                        "depth", G_TYPE_INT, 16,
-                                        NULL);
-
-        GST_INFO_OBJECT (omx_base, "caps are: %" GST_PTR_FORMAT, new_caps);
-        gst_pad_set_caps (omx_base->srcpad, new_caps);
-    }
-}
-
-static void
 type_instance_init (GTypeInstance *instance,
                     gpointer g_class)
 {
@@ -149,6 +108,4 @@ type_instance_init (GTypeInstance *instance,
     GST_DEBUG_OBJECT (omx_base, "start");
 
     omx_base->use_timestamps = FALSE;
-
-    omx_base->gomx->settings_changed_cb = settings_changed_cb;
 }
diff --git a/omx/gstomx_vorbisdec.h b/omx/gstomx_vorbisdec.h
index 05ce070..c7737a3 100644
--- a/omx/gstomx_vorbisdec.h
+++ b/omx/gstomx_vorbisdec.h
@@ -32,16 +32,16 @@ G_BEGIN_DECLS
 typedef struct GstOmxVorbisDec GstOmxVorbisDec;
 typedef struct GstOmxVorbisDecClass GstOmxVorbisDecClass;
 
-#include "gstomx_base_filter.h"
+#include "gstomx_base_audiodec.h"
 
 struct GstOmxVorbisDec
 {
-    GstOmxBaseFilter omx_base;
+    GstOmxBaseAudioDec omx_base;
 };
 
 struct GstOmxVorbisDecClass
 {
-    GstOmxBaseFilterClass parent_class;
+    GstOmxBaseAudioDecClass parent_class;
 };
 
 GType gst_omx_vorbisdec_get_type (void);
-- 
1.6.3.2

[Gstreamer-openmax] [PATCH 09/10] add component-role support

[Rob C. <ro...@ti...>]

---
 omx/gstomx.c      |   14 +++++++++++++-
 omx/gstomx.h      |    1 +
 omx/gstomx_util.c |   23 +++++++++++++++++++++--
 omx/gstomx_util.h |    1 +
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/omx/gstomx.c b/omx/gstomx.c
index e35ef3d..e199cce 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -233,7 +233,7 @@ plugin_init (GstPlugin *plugin)
         const gchar *element_name = gst_structure_nth_field_name (element_table, i);
         GstStructure *element = get_element_entry (element_name);
         const gchar *type_name, *parent_type_name;
-        const gchar *component_name, *library_name;
+        const gchar *component_name, *component_role, *library_name;
         GType type;
         gint rank;
 
@@ -242,6 +242,7 @@ plugin_init (GstPlugin *plugin)
         parent_type_name = gst_structure_get_string (element, "parent-type");
         type_name = gst_structure_get_string (element, "type");
         component_name = gst_structure_get_string (element, "component-name");
+        component_role = gst_structure_get_string (element, "component-role");
         library_name = gst_structure_get_string (element, "library-name");
 
         if (!type_name || !component_name || !library_name)
@@ -316,6 +317,9 @@ gstomx_get_component_info (void *core,
     str = gst_structure_get_string (element, "component-name");
     rcore->component_name = g_strdup (str);
 
+    str = gst_structure_get_string (element, "component-role");
+    rcore->component_role = g_strdup (str);
+
     return TRUE;
 }
 
@@ -328,6 +332,11 @@ gstomx_install_property_helper (GObjectClass *gobject_class)
                                                           "Name of the OpenMAX IL component to use",
                                                           NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
 
+    g_object_class_install_property (gobject_class, ARG_COMPONENT_ROLE,
+                                     g_param_spec_string ("component-role", "Component role",
+                                                          "Role of the OpenMAX IL component",
+                                                          NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
+
     g_object_class_install_property (gobject_class, ARG_LIBRARY_NAME,
                                      g_param_spec_string ("library-name", "Library name",
                                                           "Name of the OpenMAX IL implementation library to use",
@@ -343,6 +352,9 @@ gstomx_get_property_helper (void *core, guint prop_id, GValue *value)
         case ARG_COMPONENT_NAME:
             g_value_set_string (value, gomx->component_name);
             return TRUE;
+        case ARG_COMPONENT_ROLE:
+            g_value_set_string (value, gomx->component_role);
+            return TRUE;
         case ARG_LIBRARY_NAME:
             g_value_set_string (value, gomx->library_name);
             return TRUE;
diff --git a/omx/gstomx.h b/omx/gstomx.h
index 7557fd4..da66c81 100644
--- a/omx/gstomx.h
+++ b/omx/gstomx.h
@@ -34,6 +34,7 @@ enum
 {
     GSTOMX_ARG_0,
     ARG_COMPONENT_NAME,
+    ARG_COMPONENT_ROLE,
     ARG_LIBRARY_NAME,
     GSTOMX_LAST_COMMON_PROP
 };
diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
index 565bdda..513ab6d 100644
--- a/omx/gstomx_util.c
+++ b/omx/gstomx_util.c
@@ -310,8 +310,10 @@ g_omx_core_init (GOmxCore *core)
     if (core->omx_handle)
         return;
 
-    GST_DEBUG_OBJECT (core->object, "loading: %s (%s)",
-            core->component_name, core->library_name);
+    GST_DEBUG_OBJECT (core->object, "loading: %s %s (%s)",
+            core->component_name,
+            core->component_role ? core->component_role : "",
+            core->library_name);
 
     core->imp = request_imp (core->library_name);
 
@@ -327,7 +329,23 @@ g_omx_core_init (GOmxCore *core)
             core->omx_handle, core->omx_error);
 
     if (!core->omx_error)
+    {
         core->omx_state = OMX_StateLoaded;
+
+        if (core->component_role)
+        {
+            OMX_PARAM_COMPONENTROLETYPE param;
+
+            GST_DEBUG_OBJECT (core->object, "setting component role: %s",
+                    core->component_role);
+
+            G_OMX_INIT_PARAM (param);
+
+            strcpy((char*)param.cRole, core->component_role);
+
+            g_omx_core_set_param (core, OMX_IndexParamStandardComponentRole, &param);
+        }
+    }
 }
 
 void
@@ -354,6 +372,7 @@ g_omx_core_deinit (GOmxCore *core)
 
     g_free (core->library_name);
     g_free (core->component_name);
+    g_free (core->component_role);
 
     release_imp (core->imp);
     core->imp = NULL;
diff --git a/omx/gstomx_util.h b/omx/gstomx_util.h
index c647998..64360b9 100644
--- a/omx/gstomx_util.h
+++ b/omx/gstomx_util.h
@@ -94,6 +94,7 @@ struct GOmxCore
 
     gchar *library_name;
     gchar *component_name;
+    gchar *component_role;
 };
 
 struct GOmxPort
-- 
1.6.3.2

[Gstreamer-openmax] [PATCH 08/10] add some debug traces

[Rob C. <ro...@ti...>]

---
 omx/gstomx_base_filter.c |   12 ++++++++++++
 omx/gstomx_util.c        |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/omx/gstomx_base_filter.c b/omx/gstomx_base_filter.c
index 02cacaa..7f3e536 100644
--- a/omx/gstomx_base_filter.c
+++ b/omx/gstomx_base_filter.c
@@ -75,6 +75,7 @@ setup_ports (GstOmxBaseFilter *self)
 
     if (g_getenv ("OMX_ALLOCATE_ON"))
     {
+        GST_DEBUG_OBJECT (self, "OMX_ALLOCATE_ON");
         self->in_port->omx_allocate = TRUE;
         self->out_port->omx_allocate = TRUE;
         self->share_input_buffer = FALSE;
@@ -82,14 +83,25 @@ setup_ports (GstOmxBaseFilter *self)
     }
     else if (g_getenv ("OMX_SHARE_HACK_ON"))
     {
+        GST_DEBUG_OBJECT (self, "OMX_SHARE_HACK_ON");
         self->share_input_buffer = TRUE;
         self->share_output_buffer = TRUE;
     }
     else if (g_getenv ("OMX_SHARE_HACK_OFF"))
     {
+        GST_DEBUG_OBJECT (self, "OMX_SHARE_HACK_OFF");
         self->share_input_buffer = FALSE;
         self->share_output_buffer = FALSE;
     }
+    else
+    {
+        GST_DEBUG_OBJECT (self, "default sharing and allocation");
+    }
+
+    GST_DEBUG_OBJECT (self, "omx_allocate: in: %d, out: %d",
+            self->in_port->omx_allocate, self->out_port->omx_allocate);
+    GST_DEBUG_OBJECT (self, "share_buffer: in: %d, out: %d",
+            self->share_input_buffer, self->share_output_buffer);
 }
 
 static GstStateChangeReturn
diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
index 128f06a..565bdda 100644
--- a/omx/gstomx_util.c
+++ b/omx/gstomx_util.c
@@ -628,6 +628,7 @@ port_allocate_buffers (GOmxPort *port)
     {
         if (port->omx_allocate)
         {
+            GST_DEBUG_OBJECT (port->core->object, "%d: OMX_AllocateBuffer(), size=%"G_GSIZE_FORMAT, i, size);
             OMX_AllocateBuffer (port->core->omx_handle,
                                 &port->buffers[i],
                                 port->port_index,
@@ -638,6 +639,7 @@ port_allocate_buffers (GOmxPort *port)
         {
             gpointer buffer_data;
             buffer_data = g_malloc (size);
+            GST_DEBUG_OBJECT (port->core->object, "%d: OMX_UseBuffer(), size=%"G_GSIZE_FORMAT, i, size);
             OMX_UseBuffer (port->core->omx_handle,
                            &port->buffers[i],
                            port->port_index,
-- 
1.6.3.2