[MPlayer-dev-eng] [PATCH] vdpau fixes and timing enhancements
Dan Oscarsson
Dan.Oscarsson at tietoenator.com
Mon Apr 20 08:30:16 CEST 2009
On 2009-04-18 at 16:34 +0200 Diego Biurrun wrote:
> > The patches include:
>
> ... a lot of things ..
>
> Please split the patch into manageable pieces. Also, please avoid tabs
> and trailing whitespace.
I did that before but got nowhere. It is a lot of work to split it into
parts, though the mplayer.c patches are fairly easy to split out. That I
did before.
There should not be any tabs or tailing whitespace unless my Linux tools
add them when I copy between windows.
> > +} 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.
I just followed the standard I saw in many (yes many) places in mplayer.
You need a typedef if you want to avoid having to write stuct
everywhere.
>
> > @@ -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
What is bad here?
>
> > @@ -528,6 +597,108 @@
> >
> > +static void get_vdpau_timing(void) {
>
> K&R function declarations please.
You want the { on next line or what?
> > + if (max_delay) {
> > + num_output_surfaces = 3+((int)(max_delay+0.05));
> > + } else {
> > + num_output_surfaces = 2;
> > + }
>
> pointless {}
Yes but much clearer for the eye. Also much easier to add new lines in
each conditional part.
>
> > --- 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?
What is best name? Is uint64_t more portable?
More information about the MPlayer-dev-eng
mailing list