[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...

[Diego Biurrun]

On Mon, Jan 05, 2009 at 10:39:41AM +0000, Carl Eugen Hoyos wrote:
> 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.
 
vd_ and vo_ are MPlayer things, they make no sense in an FFmpeg header.
An FFmpeg header should be understandable without knowledge of MPlayer
internals.

> > > +    /** 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.

The English verb is allocate, the past participle is allocated.

Just change it, I see no sense in sending a patch.

> > > +    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.

Plenty of things in this patch went unnoticed, probably due to the fact
that it was not high quality to begin with.  Note that I'm not saying
any of this is your fault.

Diego