[FFmpeg-devel] [PATCH] H264 DXVA2 implementation

[Reimar Döffinger]

On Sat, Jan 09, 2010 at 05:52:29PM +0100, Laurent Aimar wrote:
> +                if (h->luma_weight_flag[list]) {
> +                    slice->Weights[list][i][0][0] = h->luma_weight[list][i];
> +                    slice->Weights[list][i][0][1] = h->luma_offset[list][i];
> +                } else {
> +                    slice->Weights[list][i][0][0] = 1 << h->luma_log2_weight_denom;
> +                    slice->Weights[list][i][0][1] = 0;
> +                }
> +                for (plane = 1; plane < 3; plane++) {
> +                    int w, o;
> +                    if (h->chroma_weight_flag[list]) {
> +                        w = h->chroma_weight[list][i][plane-1];
> +                        o = h->chroma_offset[list][i][plane-1];
> +                    } else {
> +                        w = 1 << h->chroma_log2_weight_denom;
> +                        o = 0;
> +                    }
> +                    slice->Weights[list][i][plane][0] = w;
> +                    slice->Weights[list][i][plane][1] = o;

That looks like code duplicate in slightly different way.
If there's a nice way to avoid the duplication that's great, otherwise it
should at least be done the same way.

> +            if (i == ctx_pic->slice_count - 1)
> +                padding = 128 - ((&current[start_code_size + size] - dxva_data) & 127);
[...]
> +            slice->BSNALunitDataLocation = current - dxva_data;
> +            slice->SliceBytesInBuffer    = start_code_size + size + padding;
[...]
> +            if (start_code_size + size > end - current) {
> +                av_log(avctx, AV_LOG_DEBUG, "Failed to build bitstream");
> +                break;
> +            }

I think checking the values first and then using them (even if there are no
dereferences yet) would both be more correct and more logical.