[MPlayer-dev-eng] [PATCH]VO_VDPAU, round 2
Carl Eugen Hoyos
cehoyos at ag.or.at
Sat Jan 10 01:41:22 CET 2009
Reimar Döffinger <Reimar.Doeffinger <at> stud.uni-karlsruhe.de> writes:
> > +VdpDeviceDestroy * vdp_device_destroy;
> > +VdpVideoSurfaceCreate * vdp_video_surface_create;
> > +VdpVideoSurfaceDestroy * vdp_video_surface_destroy;
>
> If you don't mind, I think it would be better to align these and place the *
> consistently, preferably without a space to the right.
> More importantly though, should/need these really all be global (no
> "static")?
They are "static aligned" now;-)
> And would it maybe be nicer to put them all into a struct, making it
> easier to allow multiple vo instances "somewhen"?
Not done for the moment. Are they not function pointers that are identical for
each instance?
> > +static uint32_t max_width = 0, max_height = 0; // zero means: not set
>
> I have some idea, but I am not completely sure what these are there for.
Then you know already more than I do;-)
[...]
> > + if (vo_config_count)
> > + {
> > + // FIXME: We should really check for matching parameters here,
> > + // and uninit/re-config if they have changed?
> > + return 0;
> > + }
>
> This comment sounds like it will break horribly with -fixed-vo and
> playing e.g. a VC-1 file after a H.264 file...
It doesn't break yet: No -vc vdpau yet
I don't know how to fix it correctly: What about failing?
> > +#ifdef HAVE_NEW_GUI
[a lot of code]
> > + if ((flags & VOFLAG_FULLSCREEN) && WinID <= 0)
> > + vo_fs = 1;
>
> Duplicated fro vo_xv, and the last one should probably not have the
> "WinID <= 0" check, I just have not yet thought of a nice way to clean
> this up.
I did not touch this yet: It looks a bit different in vdpau, xv and xvmc, so I
need more advice.
[...]
> > + // indexData creation, component order - I, A, I, A, .....
> > + for (i = 0; i < h; i++) {
> > + for (j = 0; j < w; j++) {
> > + indexData[i*2*w + j*2] = src[i*stride+j]; // index for
palette-table
> > + a = srca[i*stride+j]; // alpha_data
> > + indexData[i*2*w + j*2 + 1] = (a == 0) ? 255 : a;
>
> This looks like the classical misunderstood formula.
> vo_direct3d should have things right.
> It should be just
> indexData[i*2*w + j*2 + 1] = -srca[i*stride+j];
>
> > + blendState.blend_factor_source_color =
VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA;
> > + blendState.blend_factor_source_alpha =
VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA;
>
> And these factors should be something like
> VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE.
>
> > + blendState.blend_factor_destination_color =
VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_SRC_ALPHA;
> > + blendState.blend_factor_destination_alpha =
VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_SRC_ALPHA;
>
> And these VDP_OUTPUT_SURFACE_RENDER_BLEND_FACTOR_ONE_MINUS_SRC_ALPHA
Changed, I hope it's truly correct now.
[...]
> > +static uint32_t start_slice(mp_image_t * mpi)
> > +{
> > + mpi->flags &= ~MP_IMGFLAG_DRAW_CALLBACK;
> > + return VO_TRUE;
> > +}
>
> This looks suspicious...
Not understood...
> > + }
> > + case VOCTRL_ONTOP:
> > + vo_x11_ontop();
> > + return VO_TRUE;
> > + case VOCTRL_UPDATE_SCREENINFO:
> > + update_xinerama_info();
> > + return VO_TRUE;
> > + }
>
> The code looks like panscan is implemented, but
> VOCTRL_GET_PANSCAN/VOCTRL_SET_PANSCAN is missing here...
I did not look at this yet.
New patch will follow soon, thank you for reviewing, Carl Eugen
More information about the MPlayer-dev-eng
mailing list