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

[Reimar Döffinger]

On Sun, Mar 15, 2009 at 10:40:26AM +0000, Carl Eugen Hoyos wrote:
> Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:
> > >  /* number of video and output surfaces */
> > > -#define NUM_OUTPUT_SURFACES                2
> > > +#define NUM_OUTPUT_SURFACES                5
> > 
> > Why?
> 
> Because three additional surfaces are needed for advanced deinterlacing?

Why are they needed? Where is that documented in the code? What happens
when you don't provide those additional surfaces?

> > > @@ -220,6 +221,11 @@
> > >      for (i = 0; i <= !!(deint > 1); i++) {
> > >          int field = VDP_VIDEO_MIXER_PICTURE_STRUCTURE_FRAME;
> > >          VdpOutputSurface output_surface;
> > > +        VdpVideoSurface const past_pict[2] =
> > > {surface_render[(vid_surface_num+2)&3].surface,
> > > +        surface_render[(vid_surface_num+1)&3].surface};
> > > +        int current_pict = (deint > 2) ? (vid_surface_num + 3) & 3 :
> > > vid_surface_num;
> > 
> > This is confusing, -1, -2, -3 would be clearer (making vid_surface_num
> > unsigned might be a good idea though)?
> 
> That turned out to be more difficult than expected.

Why? I guess the making unsigned part?

> Should I use INT_MAX instead of -1?

Then you can just use -1 and not make vid_surface_num unsigned, the only
reason to make it unsigned is to avoid relying on two's complement
representation, but since the NVidia driver is binary only anyway it
actually does not matter.

> > But thinking of hoe to extend this to accelerated video, I think it
> > would be better to create a global
> > VdpVideoSurface past_surfaces[3];
> > and queue them up in there in draw_image.
> 
> You mean three pointers to VdpVideoSurface? Or copy the surfaces?
> I don't see how this works with the way draw_image() calls get_surface().

video_to_output_surface();
past_surfaces[2] = past_surfaces[1];
past_surfaces[1] = past_surfaces[0];
past_surfaces[0] = surface_render[vid_surface_num].surface;
or something like that.

> > Particularly since it might be better to display the first frames with
> > bob deinterlacing than just discarding them as I think you do now...
> 
> All frames are shown with the patch as I sent it.

I misread your VDP_INVALID_HANDLE check.
So during the start of the video you set "past frames" in
vdp_video_mixer_render to 2, but both of those are invalid
(or alternatively possibly from the last video with -fixed-vo)?