[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-datatype-dev
Subject:    Re: [datatype-dev] CR: Adding mpeg4 omx codec wrapper
From:       Sheldon Fu <sfu () real ! com>
Date:       2009-03-20 4:11:03
Message-ID: 1237522263.5996.67.camel () sfu6400
[Download RAW message or body]

Most OMX core implementations, if not all, are shared libs anyway so
there is no memory overhead by linking to them from multiple places.
Even if OMX core is a static lib, we would only link to it from two
places - universal video decoder wrapper and universal audio decoder
wrapper. There is also no OMX Core object (OMX_Init doesn't return a
handle). The 'instance' is really just the shared lib itself. 

And MediaPlatform is not for outside object wrappers I think. There may
be some interesting use cases though if we query client context for the
omx core methods, basically letting the TLC decide which OMX Core, and
in turn which decoder, to use. Even that may not worth to be implemented
now, especially if we still hard-code the decoder name in the pcf or cf
file. 

fxd 



On Fri, 2009-03-20 at 08:52 +0530, Gurpreet wrote:
> Thanks Sheldon for your comments.
> I completely agree.
> One more thing I would like to add is:
> OMX Core should be loaded from MediaPlatform so that same instance could 
> be used for all codecs.
> Each codec can query for OMX core from context(media platform).
> This will move all omx library linking to mediaplatform.
> Let me know your thoughts on this.
> 
> Best Regards,
> Gurpreet
> 
> Sheldon Fu wrote:
> > Gurpreet,
> >
> > The code looks ok, although since these omx video decoder wrappers are
> > naturally so similar, I think we should merge them all together. If we
> > check in this module and the h264 wrapper module, 90%+ will be duplicate
> > code. 
> >
> > Instead of having separate wrappers for each video codec type, we should
> > have a single /datatype/omx/video/codec/decoder module that handles all
> > of them (h.264, mp4v and h263 at least). That module could build
> > to ,e.g, omxvd.so (or if we have to stay within 4 letters, omvd.so) and
> > the renderers (mp4 or rm) should be modify to know to load this single
> > omx video decoder wrapper for any of the codecs it supports. The actual
> > codec flavor can be signaled in the _Init call or through a SetProperty
> > call.
> >
> > There should also be software codec fallback logic in the renderer.
> > Renderer should try to load the omx codec wrapper first and if its Init
> > method fails, falls back to the software decoder. This is necessary
> > because hardware codec are not always available. These HW decoders
> > normally can have a single client at any given time so the request from
> > us can fail. 
> >
> > All these should be controlled by helix feature defines. e.g, only when
> > HELIX_FEATURE_ENABLE_OMX_VIDEO_DECODER is on should the renderer try to
> > load the omx decoder first. And within the omxvd.so module, there should
> > be things like HELIX_FEATURE_OMX_VIDEO_DECODER_H264 and
> > HELIX_FEATURE_OMX_VIDEO_DEDOER_MP4V, etc to control whether the specific
> > code (there would be a small amount of such code) for that codec should
> > be compiled in. 
> >
> > Feel free to take the h264 decoder wrapper and merge it with this.
> >
> > Same principle should apply to omx audio decoder wrappers. To give you
> > an example, the OpenCore engine has only two wrapper nodes for omx
> > decoder, one for all omx video decoders, another for all omx audio
> > decoders.
> >
> > And a couple of comments on the code itself:
> >
> > 1. why do you have to keep a list of incoming data timestamps? The omx
> > decoder is supposed to carry the timestamp from input buffer to output
> > buffer internally.
> >
> > 2. Please don't use PV's software decoder as default decoder name! Also
> > the decoder name should be configured either in the platform .cf or .pcf
> > file, not hard-coded in the source code. 
> >
> > fxd 
> >  
> >
> > On Thu, 2009-03-19 at 15:54 +0530, Gurpreet wrote:
> >   
> >> Synopsis:
> >> Adding mpeg4 omx codec wrapper.
> >>     
> >
> >   
> >> Overview:
> >> Adding mpeg4 omx codec wrapper.
> >> Its based on Sheldon Fu h264 omx codec wrapper.
> >>
> >> Files Added:
> >> datatype/rm/video/codec/omxmpg4/Umakefil
> >> datatype/rm/video/codec/omxmpg4/android.pcf
> >> datatype/rm/video/codec/omxmpg4/mpeg4.h
> >> datatype/rm/video/codec/omxmpg4/mpeg4dstm.h
> >> datatype/rm/video/codec/omxmpg4/guid.cpp
> >> datatype/rm/video/codec/omxmpg4/mpeg4.cpp
> >> datatype/rm/video/codec/omxmpg4/mpeg4dstm.cpp
> >> datatype/rm/video/codec/omxmpg4/mpeg4api.cpp
> >>
> >> Image Size and Heap Use impact (Client -Only):
> >> None
> >>
> >> Platforms and Profiles Build Verified:
> >> BIF branch      -> atlas 310
> >> Target(s)          -> splay
> >> Profile              -> helix-client-all-defines
> >> SYSTEM_ID  -> linux-2.2-libc6-gcc32-i586
> >>
> >> Files Attached::
> >> omxmpg4.zip
> >>
> >> Thanks & Best Regards,
> >> Gurpreet
> >> _______________________________________________
> >> Datatype-dev mailing list
> >> Datatype-dev@helixcommunity.org
> >> http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
> >>     
> >
> >
> >   
> 


_______________________________________________
Datatype-dev mailing list
Datatype-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
[prev in list] [next in list] [prev in thread] [next in thread]