[Mesa-dev] [PATCHv2 1/3] omx: query driver encode capatibility earlier

[Emil Velikov]

On 16/02/14 10:20, Christian König wrote:
> Am 15.02.2014 20:35, schrieb Emil Velikov:
>> Query of the driver support AVC_HIGH encode before reporting
>> the total number of supported components.
>>
>> v2: drop if (name_specific_length) check
> 
> I considered this as well, but as far as I understand it that's not
> necessary correct.
> 
Hi Christian

I'm not sure I see your concern.

If you consider this an individual project one will not implement nor
advertise an encoder (second component) if they know it is not going to
work (would that be missing hardware capability or other reason). Same
thing here - there is no point in saying "we have two components" only
to die later after realising that is not the case.

I suspect that this may even help with the following bug
https://bugs.freedesktop.org/show_bug.cgi?id=74944

> Have you checked the spec on how exactly we should do it?
> 
I have skimmed through the openmax IL 1.1.2 spec and it does not seem to
cover this topic. Or perhaps I should be looking at the
libomxil-bellagio documentation ?

Updated patches with improved commit message/comments should clear any
confusion caused.

http://patchwork.freedesktop.org/patch/20396/
http://patchwork.freedesktop.org/patch/20397/
http://patchwork.freedesktop.org/patch/20398/

-Emil
> Christian.
> 
>>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>   src/gallium/state_trackers/omx/entrypoint.c | 17 +++++++++++++++--
>>   src/gallium/state_trackers/omx/vid_enc.c    | 25
>> +++++++++----------------
>>   2 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/omx/entrypoint.c
>> b/src/gallium/state_trackers/omx/entrypoint.c
>> index 52b2495..fa654ba 100644
>> --- a/src/gallium/state_trackers/omx/entrypoint.c
>> +++ b/src/gallium/state_trackers/omx/entrypoint.c
>> @@ -50,10 +50,23 @@ static unsigned omx_usecount = 0;
>>     int omx_component_library_Setup(stLoaderComponentType **stComponents)
>>   {
>> +   struct vl_screen *vscreen = omx_get_screen();
>> +   struct pipe_screen *screen = vscreen ? vscreen->pscreen : NULL;
>>      OMX_ERRORTYPE r;
>> +   int num_components = 1;
>> +
>> +   /*
>> +    * Increment the number of components if the driver supports
>> +    * PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH encoding.
>> +    */
>> +   if (screen->get_video_param(screen,
>> PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH,
>> +                               PIPE_VIDEO_ENTRYPOINT_ENCODE,
>> +                               PIPE_VIDEO_CAP_SUPPORTED) {
>> +      num_components++;
>> +   }
>>        if (stComponents == NULL)
>> -      return 2;
>> +      return num_components;
>>        /* component 0 - video decoder */
>>      r = vid_dec_LoaderComponent(stComponents[0]);
>> @@ -65,7 +78,7 @@ int
>> omx_component_library_Setup(stLoaderComponentType **stComponents)
>>      if (r != OMX_ErrorNone)
>>         return r;
>>   -   return 2;
>> +   return num_components;
>>   }
>>     struct vl_screen *omx_get_screen(void)
>> diff --git a/src/gallium/state_trackers/omx/vid_enc.c
>> b/src/gallium/state_trackers/omx/vid_enc.c
>> index 6e65274..564f5b6 100644
>> --- a/src/gallium/state_trackers/omx/vid_enc.c
>> +++ b/src/gallium/state_trackers/omx/vid_enc.c
>> @@ -106,12 +106,7 @@ OMX_ERRORTYPE
>> vid_enc_LoaderComponent(stLoaderComponentType *comp)
>>      comp->componentVersion.s.nRevision = 0;
>>      comp->componentVersion.s.nStep = 1;
>>      comp->constructor = vid_enc_Constructor;
>> -
>> -   if (screen->get_video_param(screen,
>> PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH,
>> -                   PIPE_VIDEO_ENTRYPOINT_ENCODE,
>> PIPE_VIDEO_CAP_SUPPORTED))
>> -      comp->name_specific_length = 1;
>> -   else
>> -      comp->name_specific_length = 0;
>> +   comp->name_specific_length = 1;
>>        omx_put_screen();
>>   @@ -129,19 +124,17 @@ OMX_ERRORTYPE
>> vid_enc_LoaderComponent(stLoaderComponentType *comp)
>>      if (!comp->role_specific)
>>         goto error_arrays;
>>   -   if (comp->name_specific_length) {
>> -      comp->name_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
>> -      if (!comp->name_specific[0])
>> -         goto error_specific;
>> +   comp->name_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
>> +   if (!comp->name_specific[0])
>> +      goto error_specific;
>>   -      vid_enc_name_avc(comp->name_specific[0]);
>> +   vid_enc_name_avc(comp->name_specific[0]);
>>   -      comp->role_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
>> -      if (!comp->role_specific[0])
>> -         goto error_specific;
>> +   comp->role_specific[0] = CALLOC(1, OMX_MAX_STRINGNAME_SIZE);
>> +   if (!comp->role_specific[0])
>> +      goto error_specific;
>>   -      strcpy(comp->role_specific[0], OMX_VID_ENC_AVC_ROLE);
>> -   }
>> +   strcpy(comp->role_specific[0], OMX_VID_ENC_AVC_ROLE);
>>        return OMX_ErrorNone;
>>   
>