[Mesa-dev] [PATCH 9/9] st/va: add motion adaptive deinterlacing

[Christian König]

Am 18.12.2015 um 16:04 schrieb Emil Velikov:
> On 16 December 2015 at 20:14, Christian König <deathsimple at vodafone.de> wrote:
>
>
>> @@ -174,6 +175,51 @@ static VAStatus vlVaPostProcBlit(vlVaDriver *drv, vlVaContext *context,
>>      return VA_STATUS_SUCCESS;
>>   }
>>
>> +static struct pipe_video_buffer *
>> +vlVaApplyDeint(vlVaDriver *drv, vlVaContext *context,
>> +               VAProcPipelineParameterBuffer *param,
>> +               struct pipe_video_buffer *current,
>> +               unsigned field)
>> +{
>> +   vlVaSurface *prevprev, *prev, *next;
>> +
>> +   if (param->num_forward_references < 1 ||
>> +       param->num_backward_references < 2)
>> +      return current;
>> +
>> +   prevprev = handle_table_get(drv->htab, param->backward_references[1]);
>> +   prev = handle_table_get(drv->htab, param->backward_references[0]);
>> +   next = handle_table_get(drv->htab, param->forward_references[0]);
>> +
>> +   if (!prevprev || !prev || !next)
>> +      return current;
>> +
>> +   if (context->deint && (context->deint->video_width != current->width ||
>> +       context->deint->video_height != current->height)) {
>> +      vl_deint_filter_cleanup(context->deint);
>> +      FREE(context->deint);
>> +      context->deint = NULL;
>> +   }
>> +
>> +   if (!context->deint) {
>> +      context->deint = MALLOC(sizeof(struct vl_deint_filter));
>> +      if (!vl_deint_filter_init(context->deint, drv->pipe, current->width,
>> +                                current->height, false, false)) {
> Tear down the old one, if we succeed ?

Nope, if we don't succeed to create the new one we don't want to keep 
the old one either.

E.g. we don't want a deinterlace for the incorrect resolution around.

>
>> +         FREE(context->deint);
>> +         context->deint = NULL;
>> +         return current;
>> +      }
>> +   }
>> +
>> +   if (!vl_deint_filter_check_buffers(context->deint, prevprev->buffer,
>> +                                      prev->buffer, current, next->buffer))
> On failure, teardown the newly created deint ?

NAK, failure here is normal. For motion adaptive deinterlacing we fail 
for the first few frames/fields until we have enough to actually do the 
calculation.

>
>> +      return current;
>> +
>> +   vl_deint_filter_render(context->deint, prevprev->buffer, prev->buffer,
>> +                          current, next->buffer, field);
>> +   return context->deint->video_buffer;
>> +}
>> +
>>   VAStatus
>>   vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *context, vlVaBuffer *buf)
>>   {
>> @@ -222,6 +271,14 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *contex
>>               deinterlace = VL_COMPOSITOR_WEAVE;
>>               break;
>>
>> +         case VAProcDeinterlacingMotionAdaptive:
>> +            if (deint->flags & VA_DEINTERLACING_BOTTOM_FIELD)
>> +               src = vlVaApplyDeint(drv, context, param, src, 1);
>> +            else
>> +               src = vlVaApplyDeint(drv, context, param, src, 0);
> nit: one can also use
>
>              src = vlVaApplyDeint(drv, context, param, src,
>                                   deint->flags & VA_DEINTERLACING_BOTTOM_FIELD);

Fair enough, I fixed that one.

Any further comments or can I push this patch?

Regards,
Christian.

>
> -Emil