[FFmpeg-devel] [PATCH]NVIDIA VDPAU patch for h264

[Michael Niedermayer]

On Sun, Nov 30, 2008 at 10:03:39PM +0100, Carl Eugen Hoyos wrote:
> Hi!
>
> Attached is the h.264 part of the VDPAU patch.
> Tested on GeForce9400.
>

> ENABLE* is bad for mplayer-based options: It is also never used for xvmc.

i dont understand what you mean
if mplayers configure doesnt set ENABLE_*, mplayers configure should be fixed

[...]
> @@ -101,6 +104,16 @@
>      {0,2,0,2,7,10,7,10}
>  };
>  
> +static const enum PixelFormat pixfmt_vdpau_h264_baseline_420[] = {
> +                                           PIX_FMT_VDPAU_H264_BASELINE,
> +                                           PIX_FMT_NONE};
> +static const enum PixelFormat pixfmt_vdpau_h264_main_420[] = {
> +                                           PIX_FMT_VDPAU_H264_MAIN,
> +                                           PIX_FMT_NONE};
> +static const enum PixelFormat pixfmt_vdpau_h264_high_420[] = {
> +                                           PIX_FMT_VDPAU_H264_HIGH,
> +                                           PIX_FMT_NONE};
> +

i still do not understand why these are split per profile, and iam not going
to accept this without fully understanding it AND it actually being usefull.
There is no fallback to the SW decoder if one profile isnt supported, thus i
dont see what would be gained by this.
The profile should be available through AVCodecContext for a user app to
decide what to do ...

[...]
> +static int VDPAU_h264_set_reference_frames(H264Context *h)
> +{
> +    MpegEncContext * s = &h->s;
> +    vdpau_render_state_t * render, * render_ref;
> +    VdpReferenceFrameH264 * rf, * rf2;
> +    Picture * pic;
> +    int i, list;
> +
> +    render = (vdpau_render_state_t*)s->current_picture_ptr->data[2];

> +    assert(render != NULL);
> +    assert(render->magic == MP_VDPAU_RENDER_MAGIC);
> +    if ((render == NULL) || (render->magic != MP_VDPAU_RENDER_MAGIC))
> +        return -1; // make sure that this is render packet

assert(!A)
if(A)

also this code is duplicated all over the place ...


[...]
> +/**
> + * \brief The videoSurface is used for render.
> + */
> +#define MP_VDPAU_STATE_USED_FOR_RENDER 1
> +
> +/**
> + * \brief The videoSurface is needed for reference/prediction,
> + * codec manipulates this.
> + */
> +#define MP_VDPAU_STATE_USED_FOR_REFERENCE 2
> +

> +#define MP_VDPAU_RENDER_MAGIC 0x1DC8E14B

missing doxy


> +
> +typedef struct {
> +    int  magic;
> +

> +    VdpVideoSurface surface; //used as rendered surface, never changed.

syntax not doxy compatible


> +
> +    int state; // Holds MP_VDPAU_STATE_* values

same


> +
> +    union _VdpPictureInfo {
> +        VdpPictureInfoMPEG1Or2 mpeg;
> +        VdpPictureInfoH264     h264;
> +        VdpPictureInfoVC1       vc1;
> +    } info;

not just h264, as the patch desc says


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081130/5f5179b4/attachment.pgp>