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

[Sven Dueking]

> -----Ursprüngliche Nachricht-----
> Von: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] Im Auftrag
> von Sven Dueking
> Gesendet: Montag, 2. November 2015 10:33
> An: 'FFmpeg development discussions and patches'
> Betreff: Re: [FFmpeg-devel] [PATCH] Added QSV based VPP filter - second
> try
> 
> 
> 
> > -----Ursprüngliche Nachricht-----
> > Von: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] Im Auftrag
> > von Carl Eugen Hoyos
> > Gesendet: Donnerstag, 29. Oktober 2015 18:39
> > An: ffmpeg-devel at ffmpeg.org
> > Betreff: Re: [FFmpeg-devel] [PATCH] Added QSV based VPP filter -
> > second try
> >
> > 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?
> 
> Hi Carl,
> 
> I was unsure how to add the documentation, that´s why I asked this
> before.
> Will merge my changes into the filter documentation.
> 
> >
> > > +        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...
> 
> https://software.intel.com/sites/default/files/mediasdk-man.pdf
> 
> RGB4
> Thirty-two-bit RGB color format. Also known as RGB32
> 
> MFX_FOURCC_RGB4 : RGB4 (RGB32) color planes
> 
> This matches the define in the latest SDK
> 
>     MFX_FOURCC_RGB4         = MFX_MAKEFOURCC('R','G','B','4'),    /*
> RGB32 */
> 
> As far as I know older versions of mfxstructures.h have such comment as
> you mentioned.
> 
> Anyway, according to the sample
> 
>    case MFX_FOURCC_RGB4:
>         ptr->G = ptr->B + 1;
>         ptr->R = ptr->B + 2;
>         ptr->A = ptr->B + 3;
> 
> it´s BGRA (and the output is fine using BRGA and "bad" using ARGB).
> 
> >
> > > +    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?
> 
> No, VPP doesn´t support ARGB16.
> 
> >
> > > +    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?
> 
> Nope ;)
> 
> >
> > > +    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.)
> 
> You are correct, thanks !
> 
> >
> > > +        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.
> >
> 
> Also ok.
> 
> Many thanks for your feedback !

Hi all,

Attached an updated version of the VPP filter.

Thanks,
Sven

> 
> > Carl Eugen
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-added-QSV-VPP-filter.patch
Type: application/octet-stream
Size: 33903 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151105/d920246a/attachment.obj>