[MPlayer-dev-eng] [PATCH] vdpau fixes and timing enhancements

[Diego Biurrun]

On Sat, Apr 18, 2009 at 04:02:30PM +0200, Dan Oscarsson wrote:
> 
> Here is a patch for my current version of fixes and enhancements to
> using vdpau.
> 
> I have not included documentation updates. I will do that if you think
> my fixes/enhancements are ok.
> 
> The patches include:

.. a lot of things ..

Please split the patch into manageable pieces.  Also, please avoid tabs
and trailing whitespace.

> --- libvo/vo_vdpau.c.org	2009-04-18 15:14:00.000000000 +0200
> +++ libvo/vo_vdpau.c	2009-04-18 15:14:50.000000000 +0200
> @@ -144,9 +149,19 @@
>  
> +typedef struct {
> +    VdpOutputSurface surface;
> +    int              width;
> +    int              height;
> +    int              dwidth;
> +    int              dheight;
> +    VdpTime          queued;
> +} output_surface_t;

_t is POSIX-reserved namespace.  I don't think you need a typedef..

Yes, we have many identifiers that violate this, but we should not add
more.

> @@ -288,26 +352,21 @@
>      if (output_surface_width < vo_dwidth || output_surface_height < vo_dheight) {
>          if (output_surface_width < vo_dwidth) {
>              output_surface_width += output_surface_width >> 1;
> -            output_surface_width = FFMAX(output_surface_width, vo_dwidth);
> +            output_surface_width = FFMIN(vo_screenwidth,FFMAX(output_surface_width, vo_dwidth));
>          }
>          if (output_surface_height < vo_dheight) {
>              output_surface_height += output_surface_height >> 1;
> -            output_surface_height = FFMAX(output_surface_height, vo_dheight);
> -        }
> -        // Creation of output_surfaces
> -        for (i = 0; i <= NUM_OUTPUT_SURFACES; i++) {
> -            if (output_surfaces[i] != VDP_INVALID_HANDLE)
> -                vdp_output_surface_destroy(output_surfaces[i]);
> -            vdp_st = vdp_output_surface_create(vdp_device, VDP_RGBA_FORMAT_B8G8R8A8,
> -                                               output_surface_width, output_surface_height,
> -                                               &output_surfaces[i]);
> -            CHECK_ST_WARNING("Error when calling vdp_output_surface_create")
> -            mp_msg(MSGT_VO, MSGL_DBG2, "OUT CREATE: %u\n", output_surfaces[i]);
> +            output_surface_height = FFMIN(vo_screenheight,FFMAX(output_surface_height, vo_dheight));
>          }
> +        // Creation of osd surface
> +        if (osd_surface != VDP_INVALID_HANDLE)
> +            vdp_output_surface_destroy(osd_surface);
> +        vdp_st = vdp_output_surface_create(vdp_device, VDP_RGBA_FORMAT_B8G8R8A8,
> +                                           output_surface_width, output_surface_height,
> +                                           &osd_surface);
> +        CHECK_ST_WARNING("Error when calling vdp_output_surface_create")
> +        mp_msg(MSGT_VO, MSGL_DBG2, "OUT CREATE OSD: %u\n", osd_surface);

cosmetics

> @@ -528,6 +597,108 @@
>  
> +static void get_vdpau_timing(void) {

K&R function declarations please.

> +    if (td1 != td2) {
> +        // time one frame more to get more accurate data
> +        vdp_st = vdp_presentation_queue_display(vdp_flip_queue, surface2, 1, 1, 0);
> +        CHECK_ST_WARNING("Error when calling vdp_presentation_queue_display")
> +        vdp_st = vdp_presentation_queue_block_until_surface_idle(vdp_flip_queue, surface3, &time2);
> +        CHECK_ST_WARNING("Error when calling vdp_presentation_queue_block_until_surface_idle")
> +        vdp_st = vdp_presentation_queue_query_surface_status(vdp_flip_queue, surface2, &out_status, &time2);
> +        CHECK_ST_WARNING("Error when calling vdp_presentation_queue_query_surface_status")
> +        if (out_status != 2) {
> +            mp_msg(MSGT_VO, MSGL_V, "[vdpau] Surface not visible as it should (accuracy test)\n");
> +            return;
> +        }
> +        td3 = time2-time3;
> +        if (td3 == td2) {
> +            td1 = td3;
> +        } else if (td3 != td1) {
> +            VdpTime tdv;
> +            VdpTime tdmax,tdmin;
> +            
> +            tdmax = FFMAX(td1,FFMAX(td2,td3));
> +            tdmin = FFMIN(td1,FFMIN(td2,td3));
> +            td1 = (time2-time1)/3;
> +            tdv = td1/10000;
> +            if (tdv != td2/10000 || tdv != td3/10000) {
> +                mp_msg(MSGT_VO, MSGL_V, "[vdpau] Timing unaccurate at 10 microsecond level - assume blit engine\n");
> +                if (tdmax-tdmin < 3000000) {  // 3 ms  (about 3 ms have been seen)
> +                    mp_msg(MSGT_VO, MSGL_V, "[vdpau] Timing unaccurate at millisecond level - acceptible for blit engine\n");
> +                } else {
> +                    mp_msg(MSGT_VO, MSGL_V, "[vdpau] Timing very unaccurate at millisecond level - assume assume no vsync alignment\n");
> +                    return;
> +                }
> +            }
> +        } else { // td1 == td3
> +        }

pointless empty else clause ..

> @@ -625,6 +796,8 @@
>  
> +    if (max_delay) get_vdpau_timing();

Please put the statement on the next line.

> @@ -1077,6 +1325,19 @@
>          deint_type = deint;
>      if (deint > 1)
>          deint_buffer_past_frames = 1;
> +    if (max_delay < 0) {
> +        if (deint) {
> +            max_delay = 1.5;
> +        } else {
> +            max_delay = 1.3;
> +        }
> +    }
> +    if (max_delay) {
> +        num_output_surfaces = 3+((int)(max_delay+0.05));
> +    } else {
> +        num_output_surfaces = 2;
> +    }

pointless {}

> --- osdep/timer.h.org	2009-04-18 15:14:05.000000000 +0200
> +++ osdep/timer.h	2009-04-18 15:14:50.000000000 +0200
> @@ -21,6 +21,8 @@
>  
> +u_int64_t get_time(void);

uint64_t?

> --- cfg-mplayer.h.org	2009-04-18 15:14:23.000000000 +0200
> +++ cfg-mplayer.h	2009-04-18 15:14:50.000000000 +0200
> @@ -71,6 +71,11 @@
> +
> +#ifdef CONFIG_VDPAU
> +extern m_option_t vdpauopts_conf[];
> +#endif

pointless #ifdef around extern declaration

> --- command.c.org	2009-04-18 15:14:29.000000000 +0200
> +++ command.c	2009-04-18 15:14:50.000000000 +0200
> @@ -198,6 +198,9 @@
>  	    return M_PROPERTY_ERROR;
>  	M_PROPERTY_CLAMP(prop, *(float *) arg);
>  	playback_speed = *(float *) arg;
> +        if (playback_speed == 1.0 && matched_playback_speed) {
> +            playback_speed = matched_playback_speed;
> +        }

pointless {}

Diego