[Gstreamer-openmax] Proposal of new implementation

[Felipe Contreras]

Hi Giulio,

On Wed, Jan 26, 2011 at 5:40 PM, giulio urlini <giulio.urlini at gmail.com> wrote:
>   my two cents from Bellagio, with an opinion of pros and cons of
> Bellagio framework.
>
> The Bellagio framework has been build so complicated in order to cover
> all the definition and use cases of the standard, trying to reflect
> the general opinion of the OpenMAX Khronos group about the behavior of
> core and components in any use case. It covers also all the spec, with
> a full interoperability support.

Sure, but you can achieve the same goals with simpler code.

For example:

DERIVEDCLASS(omx_base_filter_PrivateType, omx_base_component_PrivateType)
#define omx_base_filter_PrivateType_FIELDS
omx_base_component_PrivateType_FIELDS \
  /** @param pPendingOutputBuffer pending Output Buffer pointer */ \
  OMX_BUFFERHEADERTYPE* pPendingOutputBuffer; \
  /** @param BufferMgmtCallback function pointer for algorithm callback */ \
  void (*BufferMgmtCallback)(OMX_COMPONENTTYPE* openmaxStandComp,
OMX_BUFFERHEADERTYPE* inputbuffer, OMX_BUFFERHEADERTYPE*
outputbuffer);
ENDCLASS(omx_base_filter_PrivateType)

This could be simplified a lot by doing something like:

struct omx_base_filter {
  struct omx_base_component parent;
  OMX_BUFFERHEADERTYPE *pending_buffer;
  ...
};

In fact, rather than having these base classes, it would probably be
more useful if the functionality of these bases classes was provided
to the components as utilities, that they might decide to use or not.

Not to mention that the code is overly verbose:
	omx_base_component_PrivateType* omx_base_component_Private;
	omx_base_component_Private =
(omx_base_component_PrivateType*)openmaxStandComp->pComponentPrivate;

Could be easily simplified to:
	omx_base_component_PrivateType *base;
	base = standard->pComponentPrivate;

Or here:
	omx_base_component_Private->state = OMX_StateLoaded;
	omx_base_component_Private->transientState = OMX_TransStateMax;
	omx_base_component_Private->callbacks = NULL;
	omx_base_component_Private->callbackData = NULL;
	omx_base_component_Private->nGroupPriority = 100;
	omx_base_component_Private->nGroupID = 0;
	omx_base_component_Private->pMark.hMarkTargetComponent = NULL;

s/omx_base_component_Private/base/

Theres many many ways in which the code can be simplified, made easier
to read, and more maintainable.

Also, I'm not sure about the "full interoperability" support. I'm
pretty sure if an implementation is compliant, it should work with
gst-openmax without changes. Bellagio did at some point, and then it
suddenly stopped. It might be the the API remains compatible, but the
data streams are not, to me that's a break all the same.

> Finally it has a support for multiple loaders mechanism, necessary to
> allow the necessary flexibility since this part is not covered
> intentionally by the standard.

Well, I don't think it's necessary. If you need another loader, you
use another omx implementation. What is there in an omx core other
than the loader anyway?

I have experimented with this, it's possible to load TI components
(which don't use bellagio base clases) with bellagio loader. And it's
also possible to load bellagio components into TI's omx core. At the
end of the day the only difference of these implementations are the
loaders.

> For sure I'll take a look at your implementation, but my opinion is
> that the spec itself, if considered entirely is complicated, and a
> "reference" implementation would be complicated as well.

Sure, if your objective is to implement as most as the standard as possible.

I would prefer to have a trampoline implementation, which people can
use to develop their own components. And for that purpose simplicity
is key.

> Actually in Linaro Bellagio has been recognized as the reference for
> standard OpenMAX support, so probably concentrating the efforts on it
> and on the improvement of the interaction with gst-omx can be useful
> for everyone.
>
> https://wiki.linaro.org/WorkingGroups/Middleware/Multimedia/Specs/1105/StandardizeBellagioOmxCore
>
> We are totally open to it, and we are increasing the effort spent on
> this project.

Maybe. It would certainly be nice to get bellagio working on
gst-openmax again, but I'm not going to do that any more. I think it's
a better use of my time to implement ffmpeg wrappers for libomxil-g,
and make it more extensible.

Cheers.

-- 
Felipe Contreras