> -----Ursprüngliche Nachricht----- > Von: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] Im Auftrag > von Carl Eugen Hoyos > Gesendet: Donnerstag, 29. Oktober 2015 18:39 > An: ffmpeg-devel@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 ! > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
