Menu
▾
▴
gstreamer-openmax
|
From: Felipe C. <fel...@gm...> - 2010-04-16 15:10:33
|
On Thu, Mar 18, 2010 at 2:59 AM, Rob Clark <ro...@ti...> wrote: > 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/gstomx_base_audiodec.c b/omx/gstomx_base_audiodec.c > new file mode 100644 > index 0000000..8dc37d7 > --- /dev/null > +++ b/omx/gstomx_base_audiodec.c > @@ -0,0 +1,97 @@ > +/* > + * Copyright (C) 2009 Texas Instruments, Inc - http://www.ti.com/ Why the web url? I don't think that's customary. > + * Description: Base audio decoder element > + * Created on: Aug 2, 2009 > + * Author: Rob Clark <ro...@ti...> I don't see the point of this. Author, yes, but date and description? Perhaps the description would make sense if it's picked by the automatic doc generator. > + * 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. > + */ This copyright notice is not only outdated but doesn't match the rest of the notices (not to mention that it's LGPL 2.1+ and not LGPL 2.1). -- Felipe Contreras |
|
From: Felipe C. <fel...@gm...> - 2010-03-23 22:25:10
|
On Thu, Mar 18, 2010 at 1:58 AM, Rob Clark <ro...@ti...> wrote: > This way omx_handle is always valid, and can be used in get/set_property > methods. > --- > @@ -282,12 +285,22 @@ g_omx_core_new (void *object) > > core->omx_state = OMX_StateInvalid; > > + /* it is a bit awkward to have to pass the element's g_class to > + * g_omx_core_new(), but since this is called from type_inistance_init > + * function, we can't rely on G_OBJECT_TYPE().. > + */ > + gstomx_get_component_info (core, G_TYPE_FROM_CLASS (g_class)); I don't like this. It would be better to pass the GType directly. But also, it's messing up g_omx and gstomx_ functions... Can you move g_omx_core_new to gstomx.c and make it gstomx_core_new() or something like that? Cheers. -- Felipe Contreras |
|
From: Rob C. <ro...@ti...> - 2010-03-29 13:06:12
|
This way omx_handle is always valid, and can be used in get/set_property
methods.
---
v2: add gstomx_core_new() which calls g_omx_core_new(),
gstomx_get_component_info(), and then g_omx_core_init() to avoid a
dependencies on gstomx from g_omx
omx/gstomx.c | 13 +++++++++++++
omx/gstomx.h | 2 ++
omx/gstomx_base_filter.c | 8 +-------
omx/gstomx_base_sink.c | 5 +----
omx/gstomx_base_src.c | 5 +----
omx/gstomx_util.c | 8 ++++++--
omx/gstomx_util.h | 1 -
7 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/omx/gstomx.c b/omx/gstomx.c
index ff232b0..2a3a0b7 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -333,6 +333,19 @@ gstomx_get_component_info (void *core,
return TRUE;
}
+/* for now, to keep gst out of g_omx.. if later g_omx becomes gstomx
+ * then this could be combined with the existing g_omx_core_new()
+ * and g_omx_core_init() could be made static/private
+ */
+void *
+gstomx_core_new (void *object, GType type)
+{
+ GOmxCore *core = g_omx_core_new (object);
+ gstomx_get_component_info (core, type);
+ g_omx_core_init (core);
+ return core;
+}
+
GST_PLUGIN_DEFINE (GST_VERSION_MAJOR,
GST_VERSION_MINOR,
"omx",
diff --git a/omx/gstomx.h b/omx/gstomx.h
index b62773c..4f07fd6 100644
--- a/omx/gstomx.h
+++ b/omx/gstomx.h
@@ -33,6 +33,8 @@ GST_DEBUG_CATEGORY_EXTERN (gstomx_util_debug);
gboolean gstomx_get_component_info (void *core,
GType type);
+void * gstomx_core_new (void *object, GType type);
+
G_END_DECLS
#endif /* GSTOMX_H */
diff --git a/omx/gstomx_base_filter.c b/omx/gstomx_base_filter.c
index e12fcd2..019b52b 100644
--- a/omx/gstomx_base_filter.c
+++ b/omx/gstomx_base_filter.c
@@ -101,7 +101,6 @@ change_state (GstElement *element,
switch (transition)
{
case GST_STATE_CHANGE_NULL_TO_READY:
- g_omx_core_init (core);
if (core->omx_state != OMX_StateLoaded)
{
ret = GST_STATE_CHANGE_FAILURE;
@@ -141,10 +140,6 @@ change_state (GstElement *element,
}
break;
- case GST_STATE_CHANGE_READY_TO_NULL:
- g_omx_core_deinit (core);
- break;
-
default:
break;
}
@@ -875,8 +870,7 @@ type_instance_init (GTypeInstance *instance,
self->use_timestamps = TRUE;
- self->gomx = g_omx_core_new (self);
- gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
+ self->gomx = gstomx_core_new (self, G_TYPE_FROM_CLASS (g_class));
self->in_port = g_omx_core_new_port (self->gomx, 0);
self->out_port = g_omx_core_new_port (self->gomx, 1);
diff --git a/omx/gstomx_base_sink.c b/omx/gstomx_base_sink.c
index b6a8955..f3e116f 100644
--- a/omx/gstomx_base_sink.c
+++ b/omx/gstomx_base_sink.c
@@ -381,8 +381,6 @@ activate_push (GstPad *pad,
static inline gboolean
omx_init (GstOmxBaseSink *self)
{
- g_omx_core_init (self->gomx);
-
if (self->gomx->omx_error)
return FALSE;
@@ -421,8 +419,7 @@ type_instance_init (GTypeInstance *instance,
GST_LOG_OBJECT (self, "begin");
- self->gomx = g_omx_core_new (self);
- gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
+ self->gomx = gstomx_core_new (self, G_TYPE_FROM_CLASS (g_class));
self->in_port = g_omx_core_new_port (self->gomx, 0);
{
diff --git a/omx/gstomx_base_src.c b/omx/gstomx_base_src.c
index fea7631..49ddd54 100644
--- a/omx/gstomx_base_src.c
+++ b/omx/gstomx_base_src.c
@@ -54,7 +54,6 @@ start (GstBaseSrc *gst_base)
GST_LOG_OBJECT (self, "begin");
- g_omx_core_init (self->gomx);
if (self->gomx->omx_error)
return GST_STATE_CHANGE_FAILURE;
@@ -74,7 +73,6 @@ stop (GstBaseSrc *gst_base)
g_omx_core_stop (self->gomx);
g_omx_core_unload (self->gomx);
- g_omx_core_deinit (self->gomx);
if (self->gomx->omx_error)
return GST_STATE_CHANGE_FAILURE;
@@ -406,8 +404,7 @@ type_instance_init (GTypeInstance *instance,
GST_LOG_OBJECT (self, "begin");
- self->gomx = g_omx_core_new (self);
- gstomx_get_component_info (self->gomx, G_TYPE_FROM_CLASS (g_class));
+ self->gomx = gstomx_core_new (self, G_TYPE_FROM_CLASS (g_class));
self->out_port = g_omx_core_new_port (self->gomx, 1);
GST_LOG_OBJECT (self, "end");
diff --git a/omx/gstomx_util.c b/omx/gstomx_util.c
index b1c869a..59e6487 100644
--- a/omx/gstomx_util.c
+++ b/omx/gstomx_util.c
@@ -79,6 +79,8 @@ omx_error_to_str (OMX_ERRORTYPE omx_error);
static inline GOmxPort * get_port (GOmxCore *core, guint index);
+static void core_deinit (GOmxCore *core);
+
static inline void
port_free_buffers (GOmxPort *port);
@@ -288,6 +290,8 @@ g_omx_core_new (void *object)
void
g_omx_core_free (GOmxCore *core)
{
+ core_deinit (core);
+
g_sem_free (core->port_sem);
g_sem_free (core->flush_sem);
g_sem_free (core->done_sem);
@@ -316,8 +320,8 @@ g_omx_core_init (GOmxCore *core)
core->omx_state = OMX_StateLoaded;
}
-void
-g_omx_core_deinit (GOmxCore *core)
+static void
+core_deinit (GOmxCore *core)
{
if (!core->imp)
return;
diff --git a/omx/gstomx_util.h b/omx/gstomx_util.h
index 456f9a8..5c928ed 100644
--- a/omx/gstomx_util.h
+++ b/omx/gstomx_util.h
@@ -120,7 +120,6 @@ void g_omx_deinit (void);
GOmxCore *g_omx_core_new (void *object);
void g_omx_core_free (GOmxCore *core);
void g_omx_core_init (GOmxCore *core);
-void g_omx_core_deinit (GOmxCore *core);
void g_omx_core_prepare (GOmxCore *core);
void g_omx_core_start (GOmxCore *core);
void g_omx_core_pause (GOmxCore *core);
--
1.6.6
|
|
From: Felipe C. <fel...@no...> - 2010-04-16 14:38:45
|
On Mon, Mar 29, 2010 at 03:05:39PM +0200, Rob Clark wrote:
> This way omx_handle is always valid, and can be used in get/set_property
> methods.
> ---
I applied this with some changes; I think when future reorganizations
are made what code can be made static would be obvious.
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -333,10 +333,6 @@ gstomx_get_component_info (void *core,
return TRUE;
}
-/* for now, to keep gst out of g_omx.. if later g_omx becomes gstomx
- * then this could be combined with the existing g_omx_core_new()
- * and g_omx_core_init() could be made static/private
- */
void *
gstomx_core_new (void *object, GType type)
{
diff --git a/omx/gstomx.h b/omx/gstomx.h
index 4f07fd6..e646300 100644
--- a/omx/gstomx.h
+++ b/omx/gstomx.h
@@ -33,7 +33,7 @@ GST_DEBUG_CATEGORY_EXTERN (gstomx_util_debug);
gboolean gstomx_get_component_info (void *core,
GType type);
-void * gstomx_core_new (void *object, GType type);
+void *gstomx_core_new (void *object, GType type);
G_END_DECLS
--
Felipe Contreras
|
|
From: Clark, R. <ro...@ti...> - 2010-03-23 22:36:59
|
On Mar 23, 2010, at 5:25 PM, Felipe Contreras wrote: > On Thu, Mar 18, 2010 at 1:58 AM, Rob Clark <ro...@ti...> wrote: >> This way omx_handle is always valid, and can be used in get/set_property >> methods. >> --- > >> @@ -282,12 +285,22 @@ g_omx_core_new (void *object) >> >> core->omx_state = OMX_StateInvalid; >> >> + /* it is a bit awkward to have to pass the element's g_class to >> + * g_omx_core_new(), but since this is called from type_inistance_init >> + * function, we can't rely on G_OBJECT_TYPE().. >> + */ >> + gstomx_get_component_info (core, G_TYPE_FROM_CLASS (g_class)); > > I don't like this. It would be better to pass the GType directly. > > But also, it's messing up g_omx and gstomx_ functions... Can you move > g_omx_core_new to gstomx.c and make it gstomx_core_new() or something > like that? I could do that for now. But actually I would like to change the g_omx_ stuff to gstomx_.. I think there are some parts of logic, such as handling the share_buffer vs !share_buffer cases, which could be consolidated in GOmxPort (or GstOmxPort). Similar for handling of EOS. And for upstream caps negotiation to work properly, we need to do a pad_alloc() before SendCommand(SetState, Idle) or SendCommand(EnablePort).. which could be more easily done in one place in GOmxPort. This is probably the subject for another patch(set).. but I thought maybe now would be a good time to bring up the idea and see if anyone had any issue with it. BR, -R > > Cheers. > > -- > Felipe Contreras |
|
From: Felipe C. <fel...@gm...> - 2010-03-23 22:49:12
|
On Wed, Mar 24, 2010 at 12:36 AM, Clark, Rob <ro...@ti...> wrote: > On Mar 23, 2010, at 5:25 PM, Felipe Contreras wrote: >> I don't like this. It would be better to pass the GType directly. >> >> But also, it's messing up g_omx and gstomx_ functions... Can you move >> g_omx_core_new to gstomx.c and make it gstomx_core_new() or something >> like that? > > I could do that for now. > > But actually I would like to change the g_omx_ stuff to gstomx_.. I think there are some parts of logic, such as handling the share_buffer vs !share_buffer cases, which could be consolidated in GOmxPort (or GstOmxPort). Similar for handling of EOS. Yeah, I've been meaning to fix the share_buffer stuff like that, but I haven't really looked into it. I don't think there's a need to mix gst stuff though. > And for upstream caps negotiation to work properly, we need to do a pad_alloc() before SendCommand(SetState, Idle) or SendCommand(EnablePort).. which could be more easily done in one place in GOmxPort. Haven't really thought about that... I would have to see the patches. -- Felipe Contreras |
|
From: Felipe C. <fel...@gm...> - 2010-04-16 15:21:55
|
On Thu, Mar 18, 2010 at 2:58 AM, Rob Clark <ro...@ti...> wrote:
> Rebased on top of 'port cleanups' patchset.
>
> Now, OMX component is instantiated when GOmxCore is constructed. So the
> patchset no longer includes the g_omx_core_{get,set}_{config,param} helper
> functions as these were not needed as much anymore. (Although it might be
> nice to add them later, so at least there is a common place to put an error
> traces if any of the OMX_{Get,Set}{Config,Param}() calls returns an error.)
>
> Rob Clark (8):
> core: call OMX_GetHandle in g_omx_core_new
> Add some debug traces
> Add G_OMX_INIT_PARAM utility macro
> Handle properties common to all gstomx base classes with helper
> functions
> Add component-role support
> Add input-buffers/output-buffers properties to the base classes
> Add some debug traces
> Add GstOmxBaseAudioDec base class
I've pushed all of these (as well as previous patch-sets) except the
base audio dec one.
I think you should ask for permission to commit directly to the repo.
I think important changes still should go through the mailing list
though.
Thanks!
--
Felipe Contreras
|
Thanks for helping keep SourceForge clean.
X