[FFmpeg-devel] [PATCH] H264 DXVA2 implementation

[Diego Biurrun]

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.

> --- configure	(revision 21036)
> +++ configure	(working copy)
> @@ -873,6 +874,7 @@
>      swscale_alpha
>      vaapi
>      vdpau
> +    dxva2
>      version3
>      x11grab
>      zlib

alphabetical order

> @@ -1117,6 +1119,8 @@
>  h264_vaapi_hwaccel_select="vaapi"
>  h264_vdpau_decoder_deps="vdpau_vdpau_h vdpau_vdpau_x11_h"
>  h264_vdpau_decoder_select="vdpau h264_decoder"
> +h264_dxva2_hwaccel_deps="dxva2api_h"
> +h264_dxva2_hwaccel_select="dxva2 h264_decoder"
>  imc_decoder_select="fft mdct"
>  jpegls_decoder_select="golomb"
>  jpegls_encoder_select="golomb"

ditto

> @@ -2332,6 +2336,7 @@
>  check_header vdpau/vdpau.h
>  check_header vdpau/vdpau_x11.h
>  check_header X11/extensions/XvMClib.h
> +check_header dxva2api.h

ditto

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

> +/* */
> +struct dxva2_picture_context
> +{

empty comment?

Place the { on the same line as the struct declaration.

> +    for (i = 0; i < ctx->surface_count; i++) {
> +        if (ctx->surface[i] == surface)
> +            return i;
> +    }

pointless {}

> +        dsc->CompressedBufferType = type;
> +        dsc->DataSize = size;
> +        dsc->NumMBsInBuffer = mb_count;

Align the =.

> +    /* */

empty comments everywhere..

> +            pp->RefFrameList[i].Index7Bits = get_surface_index(ctx, r);
> +            pp->RefFrameList[i].AssociatedFlag = r->long_ref != 0;

Align the =.

> +#if 0
> +            if (r->field_poc[0] == INT_MAX || r->field_poc[1] == INT_MAX)
> +                pp->NonExistingFrameFlags = 1 << i; /* FIXME to check */
> +#endif

Get rid of disabled code.

> +        } else {
> +            pp->RefFrameList[i].bPicEntry = 0xff;
> +            pp->FieldOrderCntList[i][0] = 0;
> +            pp->FieldOrderCntList[i][1] = 0;
> +            pp->FrameNumList[i] = 0;

Align.

> +static void fill_scaling_lists(const H264Context *h, DXVA_Qmatrix_H264 *qm)
> +{
> +    unsigned i, j;
> +    memset(qm, 0, sizeof(*qm));
> +    for (i = 0; i < 6; i++) {
> +        for (j = 0; j < 16; j++)
> +            qm->bScalingLists4x4[i][j] = h->pps.scaling_matrix4[i][zigzag_scan[j]];
> +    }
> +    for (i = 0; i < 2; i++) {
> +        for (j = 0; j < 64; j++)
> +            qm->bScalingLists8x8[i][j] = h->pps.scaling_matrix8[i][ff_zigzag_direct[j]];
> +    }

more pointless {}

> +    memset(slice, 0, sizeof(*slice));
> +    slice->BSNALunitDataLocation = position;
> +    slice->SliceBytesInBuffer = size;
> +    slice->wBadSliceChopping = 0;

Align.

There are more alignment possibilities below..

> +static void fill_slice_long(AVCodecContext *avctx,
> +                            DXVA_Slice_H264_Long *slice, unsigned position, unsigned size)

Break this long line.

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

> +static int decode_slice(AVCodecContext *avctx, const uint8_t *buffer, uint32_t size)

another long line

> --- libavcodec/allcodecs.c	(revision 21036)
> +++ libavcodec/allcodecs.c	(working copy)
> @@ -60,6 +60,7 @@
>      REGISTER_HWACCEL (MPEG4_VAAPI, mpeg4_vaapi);
>      REGISTER_HWACCEL (VC1_VAAPI, vc1_vaapi);
>      REGISTER_HWACCEL (WMV3_VAAPI, wmv3_vaapi);
> +    REGISTER_HWACCEL (H264_DXVA2, h264_dxva2);

alphabetical order

> --- libavcodec/dxva2.h	(revision 0)
> +++ libavcodec/dxva2.h	(revision 0)
> @@ -0,0 +1,68 @@
> +/*
> + * DXVA2 HW acceleration.

Drop the period.

> --- libavcodec/Makefile	(revision 21036)
> +++ libavcodec/Makefile	(working copy)
> @@ -3,7 +3,7 @@
>  NAME = avcodec
>  FFLIBS = avutil
>  
> -HEADERS = avcodec.h opt.h vdpau.h xvmc.h
> +HEADERS = avcodec.h opt.h vdpau.h xvmc.h dxva2.h

alphabetical order

Diego