> -----Original Message----- > From: libav-devel [mailto:[email protected]] On Behalf Of wm4 > Sent: Friday, January 26, 2018 5:15 PM > To: [email protected] > Subject: Re: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure > issue > > On Fri, 26 Jan 2018 05:56:46 +0000 > "Li, Zhong" <[email protected]> wrote: > > > > From: libav-devel [mailto:[email protected]] On Behalf Of > > > Ruiling Song > > > Sent: Friday, January 26, 2018 9:17 AM > > > To: [email protected] > > > Subject: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure > > > issue > > > > > > From: "Ruiling, Song" <[email protected]> > > > > > > 1.) the initial_pool_size need to be set instead of zero. > > > 2.) the PicStruct is required by MediaSDK, so give a default value. > > > > > > A simple command to reproduce the issue: > > > avconv -i INPUT -init_hw_device qsv=foo -filter_hw_device foo -vf > > > format=nv12,hwupload -c:v h264_qsv ... OUTPUT > > > > > > Signed-off-by: Ruiling Song <[email protected]> > > > --- > > > libavutil/hwcontext_qsv.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index > > > 9270b22..14f26c6 100644 > > > --- a/libavutil/hwcontext_qsv.c > > > +++ b/libavutil/hwcontext_qsv.c > > > @@ -313,6 +313,7 @@ static int qsv_init_surface(AVHWFramesContext *ctx, > > > mfxFrameSurface1 *surf) > > > surf->Info.CropH = ctx->height; > > > surf->Info.FrameRateExtN = 25; > > > surf->Info.FrameRateExtD = 1; > > > + surf->Info.PicStruct = MFX_PICSTRUCT_PROGRESSIVE; > > > > > > return 0; > > > } > > > @@ -325,8 +326,7 @@ static int qsv_init_pool(AVHWFramesContext *ctx, > > > uint32_t fourcc) > > > int i, ret = 0; > > > > > > if (ctx->initial_pool_size <= 0) { > > > - av_log(ctx, AV_LOG_ERROR, "QSV requires a fixed frame pool > > > size\n"); > > > > Should keep this log message as a warning be better? > > > > > - return AVERROR(EINVAL); > > > + ctx->initial_pool_size = 64; > > That doesn't really feel appropriate. If a fixed size pool is required, > the user should simply be forced to specify a size. Making it use a > random value doesn't make too much sense to me, and the required size > is going to depend on the caller's use cases. In addition 64 by default > sounds like a huge waste of memory.
Thanks for your comment! But I think it is better if we don't force the user to explicitly setup a value to get it work. Less parameters means less burden for end-users. If this rule is not applicable here, please correct me. I am not sure why a default workable value is not as good here. Could you share me more thought? And there are more places that set default values to initial_pool_size: Inside libavfilter/qsvvpp.c it also sets initial_pool_size to 64. Inside avtools/avcov_qsv.c, it sets initial_pool_size to 32. I am not sure do you have any comment on this? Will be 32 looks a little better? Ruiling > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
