[FFmpeg-cvslog] r16431 - in trunk: configure libavcodec/Makefile libavcodec/allcodecs.c libavcodec/avcodec.h libavcodec/h264.c libavcodec/h264_parser.c libavcodec/imgconvert.c libavcodec/mpegvideo.c libavcodec/vdp...

[Carl Eugen Hoyos]

Diego Biurrun <diego <at> biurrun.de> writes:

> 
> On Mon, Jan 05, 2009 at 12:55:28AM +0100, cehoyos wrote:
> > 
> > Log:
> > Add VDPAU hardware accelerated decoding for H264 which will be used by
> > MPlayer.
> > 
> > --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> > +++ trunk/libavcodec/vdpau_render.h	Mon Jan  5 00:55:27 2009	(r16431)
> > @@ -0,0 +1,86 @@
> > +
> > +/**
> > + * \brief This structure is used as a CALL-BACK between the ffmpeg
> > + * decoder (vd_) and presentation (vo_) module.
> 
> This comment makes no sense at all and refers to MPlayer internals.

I think it makes sense, but can easily be changed.

> 
> > +    union _VdpPictureInfo {
> 
> illegal identifier

I will send a patch.

> > +    /** Describe size/location of the compressed video data */
> > +    int bitstreamBuffersAlloced;
> 
> allocated

While I personally think you may be wrong here, I can of course send a patch to
change this too.

> 
> > +    int bitstreamBuffersUsed;
> > +    VdpBitstreamBuffer *bitstreamBuffers;
> 
> All these camel-cased variable names are plenty ugly.

They can be changed, but allow me to note that this is the first time you
mention it.

> This code was committed prematurely.  I know that Michael had given his
> OK already, but there were still outstanding comments.  Also, this code
> is based on a low-quality crufty part of FFmpeg (xvmc) and inherits
> its problems, i.e. only usable through internal API.
> 
> I'm starting to think it might be better to revert this and subject it
> to closer scrutiny before we make it part of the external API.

It was your suggestion to make it part of the external API, but I will try to
address all issues.

Carl Eugen