[libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure issue

Song, Ruiling ruiling.song at intel.com
Tue Jan 30 04:35:15 CET 2018



> -----Original Message-----
> From: libav-devel [mailto:libav-devel-bounces at libav.org] On Behalf Of wm4
> Sent: Friday, January 26, 2018 5:15 PM
> To: libav-devel at libav.org
> Subject: Re: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure issue
> 
> On Fri, 26 Jan 2018 05:56:46 +0000
> "Li, Zhong" <zhong.li at intel.com> wrote:
> 
> > > From: libav-devel [mailto:libav-devel-bounces at libav.org] On Behalf Of
> > > Ruiling Song
> > > Sent: Friday, January 26, 2018 9:17 AM
> > > To: libav-devel at libav.org
> > > Subject: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure issue
> > >
> > > From: "Ruiling, Song" <ruiling.song at intel.com>
> > >
> > > 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 <ruiling.song at intel.com>
> > > ---
> > >  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
> libav-devel at libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel


More information about the libav-devel mailing list