[Mesa-dev] [PATCH 4/4] st/omx/dec: make decoder video buffer progressive

[Leo Liu]

Hi Emil,

Have a look again, I think the logic is nothing wrong.

reason In lines


On 07/07/2016 11:39 AM, Emil Velikov wrote:
> On 6 July 2016 at 19:03, Leo Liu <leo.liu at amd.com> wrote:
>> The idea of encode tunneling is to use video buffer directly for encoder,
>> but currently the encoder doesn’t support interlaced surface, the OMX
>> decoder set progressive surface before on that purpose.
>>
>> Since now we are polling the driver for interlacing information for
>> decoder, we got the interlaced as preferred as other APIs(VDPAU, VA-API),
>> thus breaking the transcode with tunneling.
>>
>> The solution is when with tunnel detected, re-allocate progressive target
>> buffers, and then converting the interlaced decoder results to there.
>>
>> This has been tested with transcode results bit to bit matching as before
>> with surface from progressive to progressive.
>>
>> Signed-off-by: Leo Liu <leo.liu at amd.com>
>> ---
>>   src/gallium/state_trackers/omx/vid_dec.c | 65 +++++++++++++++++++++++++++++++-
>>   src/gallium/state_trackers/omx/vid_dec.h |  6 ++-
>>   2 files changed, 68 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/omx/vid_dec.c b/src/gallium/state_trackers/omx/vid_dec.c
>> index a989c10..7842966 100644
>> --- a/src/gallium/state_trackers/omx/vid_dec.c
>> +++ b/src/gallium/state_trackers/omx/vid_dec.c
>> @@ -167,6 +167,19 @@ static OMX_ERRORTYPE vid_dec_Constructor(OMX_COMPONENTTYPE *comp, OMX_STRING nam
>>      if (!priv->pipe)
>>         return OMX_ErrorInsufficientResources;
>>
>> +   if (!vl_compositor_init(&priv->compositor, priv->pipe)) {
>> +      priv->pipe->destroy(priv->pipe);
>> +      priv->pipe = NULL;
>> +      return OMX_ErrorInsufficientResources;
>> +   }
>> +
>> +   if (!vl_compositor_init_state(&priv->cstate, priv->pipe)) {
>> +      vl_compositor_cleanup(&priv->compositor);
>> +      priv->pipe->destroy(priv->pipe);
>> +      priv->pipe = NULL;
>> +      return OMX_ErrorInsufficientResources;
>> +   }
>> +()
> IIRC as vid_dec_Constructor() fails, the caller (bellagio?) explicitly
> calls the destructor vid_dec_Destructor(). Thus the above teardown
> should not be needed.


We take reference of the structure of priv->compositor, and priv->cstate 
for init.
the init return true or false, and there is no clear flag from 
compositor and cstate to reflect on the success of the init function.
so we use priv->pipe as a clear flag in order to clean them up at the 
destructor.


>>      priv->sPortTypesParam[OMX_PortDomainVideo].nStartPortNumber = 0;
>>      priv->sPortTypesParam[OMX_PortDomainVideo].nPorts = 2;
>>      priv->ports = CALLOC(2, sizeof(omx_base_PortType *));
>> @@ -218,8 +231,11 @@ static OMX_ERRORTYPE vid_dec_Destructor(OMX_COMPONENTTYPE *comp)
>>         priv->ports=NULL;
>>      }
>>
>> -   if (priv->pipe)
>> +   if (priv->pipe) {
>> +      vl_compositor_cleanup_state(&priv->cstate);
>> +      vl_compositor_cleanup(&priv->compositor);
> Neither vl_compositor_cleanup_state() nor vl_compositor_cleanup() is
> happy if upon deref. the value (pointer again) is NULL.
>
> omx/vid_enc.c could use similar cleanups ?
so, if priv->pipe is true,
it won't be NULL, the cstate and compositor is for sure there.


Are I right?

Thanks,
Leo

>
> Thanks,
> Emil