Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libtizonia: allow registration of different egl validation hooks for different component roles #350

Closed
gpalsingh opened this issue Jul 11, 2017 · 10 comments

Comments

@gpalsingh
Copy link
Contributor

gpalsingh commented Jul 11, 2017

Hi,
I was looking for a single entrypoint function similar to omx_component_library_Setup in bellagio http://maemo.org/api_refs/5.0/beta/libomxil-bellagio/alsa_2library__entry__point_8c.html

Currently the code I have is this

OMX_ERRORTYPE OMX_ComponentInit (OMX_HANDLETYPE ap_hdl)
{   
    tiz_role_factory_t h264d_role;
    tiz_role_factory_t h264e_role;
    const tiz_role_factory_t * rf_list[] = {&h264d_role, &h264e_role};
    tiz_type_factory_t h264dprc_type;
    tiz_type_factory_t h264eprc_type;
    const tiz_type_factory_t * tf_list[] = {&h264dprc_type, &h264eprc_type};
    const tiz_eglimage_hook_t egl_validation_hook = {
        OMX_VID_DEC_AVC_OUTPUT_PORT_INDEX,
        egl_image_validation_hook,
        NULL
    };
    
    strcpy ((OMX_STRING) h264d_role.role, OMX_VID_DEC_AVC_ROLE);
    h264d_role.pf_cport = instantiate_h264_config_port;
    h264d_role.pf_port[0] = instantiate_h264_input_port;
    h264d_role.pf_port[1] = instantiate_h264_output_port;
    h264d_role.nports = 2;
    h264d_role.pf_proc = instantiate_h264_processor;
    
    strcpy ((OMX_STRING) h264e_role.role, OMX_VID_ENC_AVC_ROLE);
    h264e_role.pf_cport = instantiate_h264e_config_port;
    h264e_role.pf_port[0] = instantiate_h264e_input_port;
    h264e_role.pf_port[1] = instantiate_h264e_output_port;
    h264e_role.nports = 2;
    h264e_role.pf_proc = instantiate_h264e_processor;
    
    strcpy ((OMX_STRING) h264dprc_type.class_name, "h264dprc_class");
    h264dprc_type.pf_class_init = h264d_prc_class_init;
    strcpy ((OMX_STRING) h264dprc_type.object_name, "h264dprc");
    h264dprc_type.pf_object_init = h264d_prc_init;
    
    strcpy ((OMX_STRING) h264eprc_type.class_name, "h264eprc_class");
    h264eprc_type.pf_class_init = h264e_prc_class_init;
    strcpy ((OMX_STRING) h264eprc_type.object_name, "h264eprc");
    h264eprc_type.pf_object_init = h264e_prc_init;
    
    /* Initialize the component infrastructure */
    tiz_comp_init (ap_hdl, OMX_VID_DEC_AVC_NAME);
    
    /* Register the classes */
    tiz_comp_register_types (ap_hdl, tf_list, 2);
    
    /* Register the component roles */
    tiz_comp_register_roles (ap_hdl, rf_list, 2);
    
    /* Register egl image validation hook */
    tiz_check_omx (tiz_comp_register_eglimage_hook
                       (ap_hdl, &egl_validation_hook));
    
    return OMX_ErrorNone;
}

Then I realised it only allows me to add a single component at a time since attaching two components to a single handle doesn't make sense.
So is there a way to accomplish this without having to separating them?

@tizonia
Copy link
Collaborator

tizonia commented Jul 11, 2017

A component may have multiple roles, and the component can be instantiated multiple times. The IL core should provide a different component handle each time a component handle is requested. I have not tried this in a long time, but I think it should work. Let me know if not the case.

@gpalsingh
Copy link
Contributor Author

The components I'm trying to instantiate here are OMX.mesa.video_decoder.avc and OMX.mesa.video_encoder.avc i.e. one decoder and encoder.

I tried something similar to this which didn't quite work.

strcpy ((OMX_STRING) h264d_role.role, "video.video_decoder.avc");
    h264d_role.pf_cport = instantiate_h264_config_port;
    h264d_role.pf_port[0] = instantiate_h264_input_port;
    h264d_role.pf_port[1] = instantiate_h264_output_port;
    h264d_role.nports = 2;
    h264d_role.pf_proc = instantiate_h264_processor;
    
    strcpy ((OMX_STRING) h264e_role.role, "video.video_encoder.avc");
    h264e_role.pf_cport = instantiate_h264e_config_port;
    h264e_role.pf_port[0] = instantiate_h264e_input_port;
    h264e_role.pf_port[1] = instantiate_h264e_output_port;
    h264e_role.nports = 2;
    h264e_role.pf_proc = instantiate_h264e_processor;
    
    strcpy ((OMX_STRING) h264dprc_type.class_name, "h264dprc_class");
    h264dprc_type.pf_class_init = h264d_prc_class_init;
    strcpy ((OMX_STRING) h264dprc_type.object_name, "h264dprc");
    h264dprc_type.pf_object_init = h264d_prc_init;
    
    strcpy ((OMX_STRING) h264eprc_type.class_name, "h264eprc_class");
    h264eprc_type.pf_class_init = h264e_prc_class_init;
    strcpy ((OMX_STRING) h264eprc_type.object_name, "h264eprc");
    h264eprc_type.pf_object_init = h264e_prc_init;
    
    /* Initialize the component infrastructure */
    tiz_comp_init (ap_hdl, "OMX.mesa.video");

Also I would like to have control over what hooks to register for the component. For example, register egl_validation_hook only for the decoder and not for the encoder, all in the same function.

@CapOM
Copy link

CapOM commented Jul 13, 2017

Hi Juan, I think the question behind is does Tizonia support to have multiple component name in one shared library ?

For example with bellagio, mycomps.so can register compA and compB, where the il client will retrieve them by calling:
OMX_GetHandle(handle_to_shared_lib_mycomps.so, "compA")
OMX_GetHandle(handle_to_shared_lib_mycomps.so, "compB")

With Tizonia, it looks like we can only register only "compA" in one shared lib. Which has been registered by calling tiz_comp_init(hdl, "compA");. Unless we can call tiz_comp_init multiple times in one OMX_ComponentInit ?

I wonder if this is covered by the specs (1.1.2, 1.2.0) or if the choice is left to the implementation/target.

That said, we can workaround this by putting a more generic name for the component: "OMX.mesa.video" and then using the roles. Though it looks like there is a bug, see below:

When running tizonia --comp-list it does not list all roles, only the first one registered. Same with gst-omx, I can only access the first role in the array (rf_list[]).

Thx a lot
Julien

@juanrubio
Copy link
Member

Hi guys,

Tizonia does not support loading multiple component "names" from a single shared object. This use case is effectively supported by the spec in both 1.1.2 and 1.2 with the "roles" functionality. I.e. The component "name" is just a unique "id",

“OMX.<vendor_name>.<vendor_specified_convention>”.

which does not necessarily need to display the component's functionality (although it typically does), and the "roles" are the various "forms" or "functions" that the component (i.e. the shared object or dll) actually support:

For example, with tizonia, you can use the --comp-list and the --roles-of-comp flags to display "shared objects" and "roles"

➜  ~  tizonia --comp-list            
tizonia 0.8.0. Copyright (C) 2017 Juan A. Rubio
This software is part of the Tizonia project <http://tizonia.org>

Component at index [0] -> [OMX.Aratelia.audio_decoder.aac]
Component at index [1] -> [OMX.Aratelia.audio_source.spotify.pcm]
Component at index [2] -> [OMX.Aratelia.container_demuxer.ogg]
Component at index [3] -> [OMX.Aratelia.audio_decoder.opus]
Component at index [4] -> [OMX.Aratelia.video_decoder.vp8]
Component at index [5] -> [OMX.Aratelia.audio_decoder.mpeg]
Component at index [6] -> [OMX.Aratelia.audio_source.http]
Component at index [7] -> [OMX.Aratelia.audio_decoder.opusfile.opus]
Component at index [8] -> [OMX.Aratelia.file_writer.binary]
Component at index [9] -> [OMX.Aratelia.audio_metadata_eraser.mp3]
Component at index [10] -> [OMX.Aratelia.ilcore.test_component]
Component at index [11] -> [OMX.Aratelia.audio_renderer.pulseaudio.pcm]
Component at index [12] -> [OMX.Aratelia.audio_renderer.alsa.pcm]
Component at index [13] -> [OMX.Aratelia.audio_decoder.pcm]
Component at index [14] -> [OMX.Aratelia.audio_encoder.mp3]
Component at index [15] -> [OMX.Aratelia.audio_renderer.http]
Component at index [16] -> [OMX.Aratelia.audio_decoder.flac]
Component at index [17] -> [OMX.Aratelia.iv_renderer.yuv.overlay]
Component at index [18] -> [OMX.Aratelia.audio_decoder.vorbis]
Component at index [19] -> [OMX.Aratelia.audio_decoder.mp3]
Component at index [20] -> [OMX.Aratelia.container_demuxer.webm]
Component at index [21] -> [OMX.Aratelia.container_muxer.ogg]
Component at index [22] -> [OMX.Aratelia.file_reader.binary]
Component at index [23] -> [OMX.Aratelia.tizonia.test_component]

➜  ~  tizonia --roles-of-comp OMX.Aratelia.audio_source.http
tizonia 0.8.0. Copyright (C) 2017 Juan A. Rubio
This software is part of the Tizonia project <http://tizonia.org>

Component [OMX.Aratelia.audio_source.http] : role #0 -> [audio_source.http]
Component [OMX.Aratelia.audio_source.http] : role #1 -> [audio_source.http.gmusic]
Component [OMX.Aratelia.audio_source.http] : role #2 -> [audio_source.http.scloud]
Component [OMX.Aratelia.audio_source.http] : role #3 -> [audio_source.http.dirble]
Component [OMX.Aratelia.audio_source.http] : role #4 -> [audio_source.http.youtube]
Component [OMX.Aratelia.audio_source.http] : role #5 -> [audio_source.http.deezer]

So the only standard way of instantiating components is with the OMX_GetHandle macro, which does not allow you to "refer" to any specific plugin/shared object. You can only pass the component's name:

OMX_API OMX_ERRORTYPE OMX_APIENTRY OMX_GetHandle (
OMX_OUT OMX_HANDLETYPE *pHandle,
OMX_IN OMX_STRING cComponentName,
OMX_IN OMX_PTR pAppData,
OMX_IN OMX_CALLBACKTYPE * pCallBacks
)

Bellagio's IL Core implementation is split in two parts. One part is the cross-platform bits and pieces that would not depend (in theory) on any OS-specific apis, and another part, the "loaders", that would allow the Bellagio IL Core to be ported to multiple platforms. These "loaders" are platform specific and contain the parts that would call the platform mechanisms responsible of initializing hardware functions, component discovery and loading.

So I believe that the APIs you are referring to in the initial question belong to the "loader" api, which is specific of Bellagio and hence non-standard.

So the option that IL provides to this problem really is to give a more generic name to the component (as you already noticed), e.g. "OMX.mesa.video.avc" with two roles "video_decoder.avc" and "video_encoder.avc".

Regarding registering an egl hook per role, this is an interesting use case that I did not envisioned. So it looks like you are wanting to register a hook per each of those two roles, which would possibly require two of these structs (i.e. possibly with different port indexes and different validation hook function)

  const tiz_eglimage_hook_t egl_validation_hook = {
    ARATELIA_VP8_DECODER_OUTPUT_PORT_INDEX,
    egl_image_validation_hook,
    NULL
  };

If the hook's port index is the same in both cases, then a possible workaround would be to use the same hook function and query the port for its domain to understand whether the active role is one or the other. This solution is ugly; Note that it is not possible to query the currently active "role" since roles can only be set with SetParameter(OMX_IndexParamStandardComponentRole) but GetParameter(OMX_IndexParamStandardComponentRole) would fail (the spec does not support this query).

So it looks like the best solution going forward is to introduce an additional egl hook registration API that would allow the registration of "per role" hooks. Would this be the best option for your use cases?

@gpalsingh
Copy link
Contributor Author

Thanks for the explanation. "per role" egl hook registration API should do the work.

@CapOM
Copy link

CapOM commented Jul 14, 2017

Thx Juan for the detailed answer!

Regarding the non eglimage part, our bug was that we were using:

tiz_comp_init ("OMX.mesa.video.all")
instantiate_h264d_config_port("OMX.mesa.video_decoder.avc")
instantiate_h264e_config_port("OMX.mesa.video_encoder.avc")

Then the listed component name with: tizonia --comp-list was "OMX.mesa.video_decoder.avc" instead of "OMX.mesa.video.all". So the first call to factory_new I guess.

Using the same name for all I could fixed the pb: (well I just looked at how you do for other components that have multiple roles)

tiz_comp_init ("OMX.mesa.video.all")
instantiate_h264d_config_port("OMX.mesa.video.all"")
instantiate_h264e_config_port(""OMX.mesa.video.all")

Now "tizonia --roles-of-comp OMX.mesa.video.all" prints:
Component [OMX.mesa.video.all] : role #0 -> [video_encoder.avc]
Component [OMX.mesa.video.all] : role #1 -> [video_decoder.avc]

So all good ! :)

Also I noticed a crash in tizscheduler.c::set_thread_name:

/* Let's skip the 'OMX.Company.' part */
p_cname = strstr (strstr (ap_sched->cname, ".") + 1, ".") + 1;
p_next_dot = strstr (p_cname, ".");

If I set : tiz_comp_init ("OMX.mesa.video") . so omitting the .all. Maybe like you said in the spec there is at least a third '.' but I guess it should not crash and should returns an error from tiz_comp_init.

Thx!

@juanrubio
Copy link
Member

tiz_comp_init ("OMX.mesa.video.all")
instantiate_h264d_config_port("OMX.mesa.video_decoder.avc")
instantiate_h264e_config_port("OMX.mesa.video_encoder.avc")

Julien, Gurkirpal, I'm not sure I got this point, could you point me to the code repo where you are hosting the avc component?

Regarding the component name, I believe the spec assumes two dots only, so that code in tizscheduler.c needs some fixing.

I'm creating another issue for that, see #353

@CapOM
Copy link

CapOM commented Jul 16, 2017

I meant like https://github.com/tizonia/tizonia-openmax-il/blob/master/plugins/file_reader/src/fr.c#L139

static OMX_PTR
instantiate_config_port (OMX_HANDLETYPE ap_hdl)
{
/* Instantiate the config port /
return factory_new (tiz_get_type (ap_hdl, "tizuricfgport"),
NULL, /
this port does not take options */
ARATELIA_FILE_READER_COMPONENT_NAME, file_reader_version);
}

We were putting the role name when calling factory_new, instead of the component name. But it looks like instantiate_h264d_config_port and instantiate_h264e_config_port are just the same so we should refactor that.

Thx for openning #353

@juanrubio
Copy link
Member

I've pushed a new API (921af2d):

/**
 * Registration of 'per-role' EGL image validation hooks.
 *
 * @ingroup tizscheduler
 *
 * @param ap_hdl The OpenMAX IL handle.
 * @param ap_role A role name (a string of up to OMX_MAX_STRINGNAME_SIZE).
 * @param ap_hook EGL image validation hook info.
 * @return OMX_ErrorNone on success, other OMX_ERRORTYPE on error.
 */
OMX_ERRORTYPE
tiz_comp_register_role_eglimage_hook (const OMX_HANDLETYPE ap_hdl,
                                      const OMX_U8 *ap_role,
                                      const tiz_eglimage_hook_t * ap_hook);

I still want to do some more testing and code review, so the implementation might not be final yet. But you guys can try and give it a go to see if this is what we need.

@juanrubio juanrubio changed the title Registering multiple components in same function libtizonia: allow registration of different egl validation hooks for different component roles Jul 17, 2017
@gpalsingh
Copy link
Contributor Author

Works with the H.264 dec. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants