[Mesa-dev] [PATCH] st/vdpau: change the order in which filters are applied

[Christian König]

Am 11.08.2016 um 17:12 schrieb Nayan Deshmukh:
> Apply the median and matrix filter before the compostioning
> we apply the deinterlacing first to avoid the extra overhead
> in processing the past and the future surfaces in deinterlacing.

It is good practice to put a v1,v2 etc... after the commit message 
noting what changed since the last version of the patch.

>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh at gmail.com>
> ---
>   src/gallium/state_trackers/vdpau/mixer.c | 25 +++++++++++++++----------
>   1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c
> index cb0ef03..c280bef 100644
> --- a/src/gallium/state_trackers/vdpau/mixer.c
> +++ b/src/gallium/state_trackers/vdpau/mixer.c
> @@ -240,8 +240,8 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer,
>      struct u_rect rect, clip, *prect, dirty_area;
>      unsigned i, layer = 0;
>      struct pipe_video_buffer *video_buffer;
> -   struct pipe_sampler_view *sampler_view;
> -   struct pipe_surface *surface;
> +   struct pipe_sampler_view *sampler_view, **sampler_views;
> +   struct pipe_surface *surface, **surfaces;
>   
>      vlVdpVideoMixer *vmixer;
>      vlVdpSurface *surf;
> @@ -325,6 +325,19 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer,
>         }
>      }
>   
> +   surfaces = video_buffer->get_surfaces(video_buffer);
> +   sampler_views = video_buffer->get_sampler_view_components(video_buffer);

You should use get_sampler_view_planes() here, otherwise you only get 3 
sampler views instead of the 6 for each plane.

> +
> +   for(i = 0; i < VL_NUM_COMPONENTS ; ++i) {

Here as well you need to handle each plane, not only each component. 
E.g. use VL_MAX_SURFACES instead of VL_NUM_COMPONENTS here.

> +      if (vmixer->noise_reduction.filter)
> +         vl_median_filter_render(vmixer->noise_reduction.filter,
> +                                 sampler_views[i], surfaces[i]);
> +
> +      if (vmixer->sharpness.filter)
> +         vl_matrix_filter_render(vmixer->sharpness.filter,
> +                                 sampler_views[i], surfaces[i]);

Some sampler views and surfaces can be NULL, so this will crash 
depending on the video buffer format in use. You need to skip the NULL ones.

Regards,
Christian.

> +   }
> +
>      prect = RectToPipe(video_source_rect, &rect);
>      if (!prect) {
>         rect.x0 = 0;
> @@ -394,14 +407,6 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer,
>      else {
>         vl_compositor_render(&vmixer->cstate, compositor, surface, &dirty_area, true);
>   
> -      if (vmixer->noise_reduction.filter)
> -         vl_median_filter_render(vmixer->noise_reduction.filter,
> -                                 sampler_view, surface);
> -
> -      if (vmixer->sharpness.filter)
> -         vl_matrix_filter_render(vmixer->sharpness.filter,
> -                                 sampler_view, surface);
> -
>         if (vmixer->bicubic.filter)
>            vl_bicubic_filter_render(vmixer->bicubic.filter,
>                                    sampler_view, dst->surface,