[Mesa-dev] [PATCH 5/8] st/va: handle default post process regions

[Emil Velikov]

On 15 December 2015 at 10:30, Christian König <deathsimple at vodafone.de> wrote:
> On 15.12.2015 11:11, Emil Velikov wrote:
>>
>> On 11 December 2015 at 12:33, Christian König <deathsimple at vodafone.de>
>> wrote:
>>>
>>> From: Christian König <christian.koenig at amd.com>
>>>
>>> Avoid referencing NULL pointers.
>>>
>> Lacking any prior knowledge of the sequential patches, I'm afraid this
>> commit message doesn't make any sense. How about "Will be used in the
>> follow up patches" or anything alike ?
>
>
> It's not used in the follow up patch, it just prevents MPV from crashing.
>
> See passing in regions is optional. GStreamer seems to always pass them in,
> but MPV just uses NULL here.
>
> So without it it will just crash when we try to dereference the pointers.
>
Coffee hasn't fully kicked in when I was looking at this. Apologies
for the noise - there's nothing wrong with the commit message.

>
>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>   src/gallium/state_trackers/va/postproc.c | 36
>>> +++++++++++++++++++++++++-------
>>>   1 file changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/va/postproc.c
>>> b/src/gallium/state_trackers/va/postproc.c
>>> index 2d17694..105f251 100644
>>> --- a/src/gallium/state_trackers/va/postproc.c
>>> +++ b/src/gallium/state_trackers/va/postproc.c
>>> @@ -29,9 +29,26 @@
>>>
>>>   #include "va_private.h"
>>>
>>> +static const VARectangle *
>>> +vlVaRegionDefault(const VARectangle *region, struct pipe_video_buffer
>>> *buf,
>>> +                 VARectangle *def)
>>> +{
>>> +   if (region)
>>> +      return region;
>>> +
>>> +   def->x = 0;
>>> +   def->y = 0;
>>> +   def->width = buf->width;
>>> +   def->height = buf->height;
>>> +
>>> +   return def;
>>> +}
>>> +
>>>   VAStatus
>>>   vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
>>> vlVaContext *context, vlVaBuffer *buf)
>>>   {
>>> +   VARectangle def_src_region, def_dst_region;
>>> +   const VARectangle *src_region, *dst_region;
>>>      struct u_rect src_rect;
>>>      struct u_rect dst_rect;
>>>      vlVaSurface *src_surface;
>>> @@ -64,15 +81,18 @@
>>> vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
>>> *contex
>>>
>>>      psurf = surfaces[0];
>>>
>>> -   src_rect.x0 = pipeline_param->surface_region->x;
>>> -   src_rect.y0 = pipeline_param->surface_region->y;
>>> -   src_rect.x1 = pipeline_param->surface_region->x +
>>> pipeline_param->surface_region->width;
>>> -   src_rect.y1 = pipeline_param->surface_region->y +
>>> pipeline_param->surface_region->height;
>>> +   src_region = vlVaRegionDefault(pipeline_param->surface_region,
>>> src_surface->buffer, &def_src_region);
>>> +   dst_region = vlVaRegionDefault(pipeline_param->output_region,
>>> context->target, &def_dst_region);
>>> +
>>
>> Mind moving this a couple of lines down - alongside the users of dst_rect
>> ?
>
>
> I unfortunately already commit that one as well because of Juliens rb.
>
> Sorry going to wait a few more days the next time,

Don't worry about it, I guess I should have looked at these a bit earlier :-)

-Emil