[MPlayer-dev-eng] [PATCH]Implement advanced deinterlacing for vdpau with SW decoders, try 2

[Reimar Döffinger]

On Sun, Mar 15, 2009 at 02:29:35PM +0100, Carl Eugen Hoyos wrote:
> Index: libvo/vo_vdpau.c
> ===================================================================
> --- libvo/vo_vdpau.c	(revision 28934)
> +++ libvo/vo_vdpau.c	(working copy)
> @@ -147,6 +147,7 @@
>  /* output_surfaces[NUM_OUTPUT_SURFACES] is misused for OSD. */
>  #define osd_surface output_surfaces[NUM_OUTPUT_SURFACES]
>  static VdpOutputSurface                   output_surfaces[NUM_OUTPUT_SURFACES + 1];
> +static VdpOutputSurface                   deint_surfaces[3];

Huh? Those are video surfaces you need, not output surfaces...

> @@ -236,9 +236,9 @@
>          CHECK_ST_WARNING("Error when calling vdp_presentation_queue_block_until_surface_idle")
>  
>          vdp_st = vdp_video_mixer_render(video_mixer, VDP_INVALID_HANDLE, 0,
> -                                        field, 0, NULL,
> +                                        field, 2, deint_surfaces,
>                                          surface_render[vid_surface_num].surface,
> -                                        0, NULL, &src_rect_vid,
> +                                        1, &deint_surfaces[2], &src_rect_vid,
>                                          output_surface,
>                                          NULL, &out_rect_vid, 0, NULL);
>          CHECK_ST_WARNING("Error when calling vdp_video_mixer_render")

Can you explain the semantics of deint_surfaces (i.e. what is stored in
it at what moment) in one short sentence? If not it's probably a bad
idea.
You should probably use (though that fixed >2 etc. is not ideal either,
an extra variable or macro or function may be better in the long term):
current_vid_surf = deint > 2 ? past_surfaces[0] : surface_render[vid_surface_num].surface;
future_vid_surf  = deint > 2 ? surface_render[vid_surface_num].surface : VDP_INVALID_HANDLE;
Note that I currently think it is better to have past_surfaces[0] be the
most recent past one instead of past_surfaces[2], but I am not
completely sure if that will prove correct.

> @@ -407,7 +407,6 @@
>          &vid_height,
>          &vdp_chroma_type
>      };
> -    if (deint == 3)
>          features[feature_count++] = VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL;
>      if (deint == 4)
>          features[feature_count++] = VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL_SPATIAL;

This may hurt people with cards with little memory, at least I think
that this might allocate some extra data. We can try it anyway and see
what happens though.

> @@ -850,7 +852,12 @@
>      }
>      top_field_first = !!(mpi->fields & MP_IMGFIELD_TOP_FIRST);
>  
> +    FFSWAP(VdpOutputSurface, deint_surfaces[2], surface_render[vid_surface_num].surface);
> +    if (deint < 3)
> +        surface_render[vid_surface_num].surface = deint_surfaces[2];
>      video_to_output_surface();
> +    deint_surfaces[0] = deint_surfaces[1];
> +    deint_surfaces[1] = surface_render[vid_surface_num].surface;

With above suggestion, this should just be
memmove(past_surfaces + 1, past_surfaces, sizeof(past_surfaces) - sizeof(VdpVideoSurface));
past_surfaces[0] = surface_render[vid_surface_num].surface;

The memmove may be a bit overkill though.