[vlc-devel] [PATCH] omxil: Add a prefix to the iomx function symbols

[Rafaël Carré]

Le 2012-01-24 02:48, Martin Storsjö a écrit :
> On Tue, 24 Jan 2012, Rafaël Carré wrote:
> 
>> Le 2012-01-24 02:27, Martin Storsjö a écrit :
>>> On some devices (apparently froyo and earlier), some OMX core
>>> (with sw-only codecs) already is loaded into the process, and
>>> dlsym(RTLD_DEFAULT) can just as well return functions from that
>>> one instead of the ones from the iomx wrapper that we've loaded.
>>>
>>> This makes sure we really get the functions we want.
>>> ---
>>>  modules/codec/omxil/iomx.cpp |   16 ++++++++++------
>>>  modules/codec/omxil/omxil.c  |    2 +-
>>>  2 files changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/modules/codec/omxil/iomx.cpp b/modules/codec/omxil/iomx.cpp
>>> index ccc4abb..1316c0b 100644
>>> --- a/modules/codec/omxil/iomx.cpp
>>> +++ b/modules/codec/omxil/iomx.cpp
>>> @@ -29,6 +29,8 @@
>>>  #include <binder/MemoryDealer.h>
>>>  #include <OMX_Component.h>
>>>
>>> +#define PREFIX(x) I ## x
>>
>> shouldn't we use "vlc" ?

perhaps vlc_ even

> We could use whatever, I felt that function names like "IOMX_Init" and so 
> on would be neat, but "vlcOMX_Init" would work just as well.

Ok, just wanting to avoid a rename when there start to be a "IOMX_Init"
function ^^

> 
> The original idea was that when we build the iomx wrapper separately, it 
> actually becomes a fully functional drop-in-replacement for any other OMX 
> core, so any other app that does dlopen + dlsym for loading an OMX core 
> could use this implementation, too.
> 
> I kept this as a separate macro, to ease modification if someone wants to 
> build it with normal OMX entry point names instead.
> 
> The reason for not using normal dlopen (but preloading it from the java 
> side instead) is twofold: dlopen doesn't look in the app lib directory by 
> default (so we'd have to add extra code there to find out what the 
> directory is) and that we don't (easily) know which version (gingerbread 
> or ics) to load, both of which are taken care of automatically on the java 
> side.
> 
>>> +
>>>  using namespace android;
>>>
>>>  class IOMXContext {
>>> @@ -270,7 +272,8 @@ static OMX_ERRORTYPE iomx_set_config(OMX_HANDLETYPE component, OMX_INDEXTYPE ind
>>>      return get_error(ctx->iomx->setConfig(node->node, index, param, sizeof(OMX_BOOL)));
>>>  }
>>>
>>> -OMX_ERRORTYPE OMX_GetHandle(OMX_HANDLETYPE *handle_ptr, OMX_STRING component_name, OMX_PTR app_data, OMX_CALLBACKTYPE *callbacks)
>>> +extern "C" {
>>
>> not related? is that needed though?
> 
> It's related and needed - this wrapper is built as C++, and we need to 
> declare these functions to have C linkage. It wasn't needed when they were 
> named with the official entry point names, since the omx headers declare 
> those functions with C linkage already.

ok, lgtm