[FFmpeg-devel] [PATCH] H264 DXVA2 implementation

[Laurent Aimar]

Hi,

I have attached a second version to address the issues reported.

On Thu, Jan 07, 2010, Diego Biurrun wrote:
> On Wed, Jan 06, 2010 at 09:51:36PM +0100, Laurent Aimar wrote:
> > 
> >  The attached path allows VLD H264 decoding using DXVA2 (GPU assisted
> > decoding API under VISTA and Windows 7).
> 
> Get rid of all the tabs.
 Done, sorry for that one.

> alphabetical order
 Done.

> > --- libavcodec/dxva2_h264.c	(revision 0)
> > +++ libavcodec/dxva2_h264.c	(revision 0)
> > @@ -0,0 +1,559 @@
> > +
> > +#ifdef HAVE_CONFIG_H
> > +# include "config.h"
> > +#endif
> 
> We don't use this #ifdef.
Removed.

> > +/* */
> > +struct dxva2_picture_context
> > +{
> 
> Place the { on the same line as the struct declaration.
Done.

> > +    for (i = 0; i < ctx->surface_count; i++) {
> > +        if (ctx->surface[i] == surface)
> > +            return i;
> > +    }
> 
> pointless {}
 Done, but I prefer to use {} when the code is on multiple lines (I find
it safer and easier to read).

> Align the =.
 Done.

> empty comments everywhere..
Removed. Generaly, I use them to mark separation between subpart of a function.

> Get rid of disabled code.
Done.

> Break this long line.
Done.

> 
> > +static int start_frame(AVCodecContext *avctx,
> > +                       av_unused const uint8_t *buffer, av_unused uint32_t size)
> 
> That's too long a line.  But more importantly, I think you should drop
> these unused parameters.
 I cannot, the start_function prototype comes from AVHWAccel (avcodec.h).

I have also:
 - added an assert() in get_surface_index(). The code can trigger the assert
 if and only if the avcodec user does not respect the API.
 - and removed the useless cast in get_surface().

-- 
fenrir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-dxva2-v2.patch
Type: text/x-diff
Size: 28867 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100107/429390a5/attachment.patch>