[MPlayer-dev-eng] [PATCH]VO_VDPAU, round 5
Diego Biurrun
diego at biurrun.de
Fri Jan 23 01:09:01 CET 2009
On Thu, Jan 22, 2009 at 03:34:27AM +0100, Carl Eugen Hoyos wrote:
>
> Attached is the fifth round of the vo_vdpau patch.
>
> --- libvo/vo_vdpau.c (revision 0)
> +++ libvo/vo_vdpau.c (revision 0)
> @@ -0,0 +1,876 @@
> +/*
> + * VDPAU Renderer for MPlayer.
> + * vo_vdpau.c - VDPAU with X11 interface.
> + *
> + * Copyright (C) 2008 NVIDIA.
Pointless periods, drop the filename.
> + * Actual decoding and presentation are implemented here.
> + * All necessary frame informations are collected through
> + * "vdpau_render_state" structure after parsing all headers
Information has no plural, "the structure".
> + * etc. in libavcodec for different codecs.
> + *
> + * @{
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "config.h"
> +#include "mp_msg.h"
> +#include "help_mp.h"
> +#include "video_out.h"
> +#include "video_out_internal.h"
> +
> +#include <X11/Xlib.h>
> +#include <X11/Xutil.h>
> +#include <errno.h>
> +
> +#include "x11_common.h"
> +
> +#include "libavcodec/vdpau.h"
> +
> +#include "fastmemcpy.h"
> +#include "sub.h"
> +#include "aspect.h"
> +
> +#include "subopt-helper.h"
> +
> +#include "input/input.h"
Are all of these needed or just copy and paste?
> +#ifdef HAVE_NEW_GUI
> +#include "gui/interface.h"
> +#endif
This preprocessor directive does not exist, so this codepath appears
to be untested.
> +/* Numbers of video and output Surfaces */
number, surfaces
> +/* Numbers of palette entries */
number
> +/* VideoMixer puts videoSurface data to displayable ouputSurface. */
s/to/on/, ouTputSurface?
> +/* OutputSurfaces[2] is used in composite-picture. */
> +static VdpOutputSurfaceRenderOutputSurface *vdp_output_surface_render_output_surface;
> +static VdpOutputSurfacePutBitsIndexed *vdp_output_surface_put_bits_indexed;
Align.
> +static VdpVideoSurface *videoSurfaces;
> +static VdpOutputSurface outputSurfaces[NUM_OUTPUT_SURFACES];
> +static VdpVideoSurface videoSurface;
> +static VdpOutputSurface outputSurface;
> +
> +static VdpDecoder decoder;
> +static VdpVideoMixer videoMixer;
ditto
Also, camelcase variable names suck.
> +static struct vdpau_render_state * surface_render;
inconsistent * placement
> +static void calc_drwXY_dWH(uint32_t *drwX, uint32_t *drwY, uint32_t *d_wh, uint32_t *d_ht)
long line, more below
> +/* Initialize Get Proc Address, called from config() */
Get Proc Address?
> +/* Destroy vdpau procs, called from uninit() */
processes, procedures?
> +#ifdef HAVE_XF86VM
> + int vm = flags & VOFLAG_MODESWITCHING;
> +#endif
again, wrong preprocessor directive
> + /* Non VDPAU specific formats
> + * No HW acceleration. VdpDecoder will not be created and
> + * there will be no call for VdpDecoderRender
call to?
> +#ifdef HAVE_NEW_GUI
> + if (use_gui)
> + guiGetEvent(guiSetShVideo, 0); // let the GUI setup/resize our window
> + else
> +#endif
> + {
> +#ifdef HAVE_XF86VM
> + if (vm)
> + vo_vm_switch();
> + else
> +#endif
see above
> +#ifdef HAVE_XF86VM
> + if (vm)
> + {
inconsistent indentation
> + vdp_st = vdp_video_surface_create(vdp_device,
> + vdp_chroma_type,
> + vid_width,
> + vid_height,
> + &videoSurfaces[i]);
No need to use so many lines, more below.
> +#if TRACE_SURFACES
> + printf("VID CREATE: %u\n", videoSurfaces[i]);
> +#endif
debug junk? same below
> + max_width = FFMAX(max_width, d_width);
> + max_height = FFMAX(max_height, d_width);
align
> + vdp_st = vdp_presentation_queue_display(
> + vdp_flip_queue,
> + outputSurface, // outputSurfaces[0 / 1],
> + FFMIN(max_width, FFMAX(outRect.x1, outRectVid.x1)),
> + FFMIN(max_width, FFMAX(outRect.y1, outRectVid.y1)),
> + 0);
weird indentation, more below
> + blendState.struct_version = VDP_OUTPUT_SURFACE_RENDER_BLEND_STATE_VERSION;
> + blendState.blend_factor_source_color = VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE;
> + blendState.blend_factor_source_alpha = VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE;
> + blendState.blend_factor_destination_color = VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA;
> + blendState.blend_factor_destination_alpha = VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA;
> + blendState.blend_equation_color = VDP_OUTPUT_SURFACE_RENDER_BLEND_EQUATION_ADD;
> + blendState.blend_equation_alpha = VDP_OUTPUT_SURFACE_RENDER_BLEND_EQUATION_ADD;
align
> +static void OSD_init()
(void)
> +static void DestroyVdpauObjects()
ditto
> +static int preinit(const char *arg)
> +{
> + if (arg)
> + {
indentation
> +static int control(uint32_t request, void *data, ...)
> +{
> + switch (request)
> + {
ditto
> --- configure (revision 28347)
> +++ configure (working copy)
> @@ -4243,6 +4247,30 @@
> +int main(void) {
> + (void)vdp_device_create_x11(0, 0, 0, 0);
> + return 0; }
Do you need that cast? Also, this fits on one line.
> + _vosrc="$_vosrc vo_vdpau.c"
outdated
Diego
More information about the MPlayer-dev-eng
mailing list