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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
