[FFmpeg-devel] [PATCH] Added QSV based VPP filter - second try

[Carl Eugen Hoyos]

Sven Dueking <sven <at> nablet.com> writes:

> Please find attached my second attempt to add Intel´s 
> VPP to FFMpeg. As requested, the patch includes a 
> documentation file

I don't know much about how the documentation works 
but why isn't this part of the filter documentation 
and how are users supposed to find it?

> +        AV_PIX_FMT_RGB32,

The Intel documentation for RGB4 reads:
"ARGB in that order, A channel is 8 MSBs"
Making this AV_PIX_FMT_ARGB imo
But I'm probably wrong again...

> +    if (inlink->format == AV_PIX_FMT_YUV420P)
> +        vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YV12;
> +    else if (inlink->format == AV_PIX_FMT_YUV422P)
> +        vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_YUY2;
> +    else if (inlink->format == AV_PIX_FMT_NV12)
> +        vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_NV12;
> +    else
> +        vpp->pVppParam->vpp.In.FourCC = MFX_FOURCC_RGB4;

Did you consider to add BGR4 and ARGB16 in the future?

> +    unsigned int bits_per_pixel = get_bpp(vpp->pVppParam->vpp.In.FourCC);

See below.

> +    width = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Width, 32);
> +    height = (unsigned int) FFALIGN(vpp->pVppParam->vpp.In.Height, 32);

Are the casts necessary or useful?

> +    vpp->in_surface = av_mallocz(sizeof(mfxFrameSurface1) * 
> vpp->num_surfaces_in);

This looks wrong to me, I believe you want to allocate 
num_surfaces_in pointers, not structs. (Sorry if I miss 
somthing.)

> +        return -1;

ENOMEM.

> +    for (i = 0; i < vpp->num_surfaces_in; i++)
> +        vpp->in_surface[i] = NULL;

This is unnecessary if you use mallocz() as you do.

> +    for (i = 0; i < vpp->num_surfaces_in; i++) {
> +        vpp->in_surface[i] = av_mallocz(sizeof(mfxFrameSurface1));
> +
> +        if (!vpp->in_surface[i])
> +            return -1;

ENOMEM but this leaks memory, you have to free 
everything else on failure.
(same below for out_surface)

> +    bits_per_pixel = 12;

Imo, remove the variable and use get_bpp() once 
and "12" on the second usage.

Carl Eugen