On Fri, 26 Jan 2018 11:04:12 +0000
Mark Thompson <s...@jkqxz.net> wrote:

> On 26/01/18 09:15, wm4 wrote:
> > On Fri, 26 Jan 2018 05:56:46 +0000
> > "Li, Zhong" <zhong...@intel.com> wrote:
> >   
> >>> From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of
> >>> Ruiling Song
> >>> Sent: Friday, January 26, 2018 9:17 AM
> >>> To: libav-devel@libav.org
> >>> Subject: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure 
> >>> issue
> >>>
> >>> From: "Ruiling, Song" <ruiling.s...@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.s...@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.  
> 
> Agree.
> 
> Maybe it would be a good idea to resurrect the set at 
> <https://lists.libav.org/pipermail/libav-devel/2017-September/084776.html>, 
> in particular there is 
> <https://lists.libav.org/pipermail/libav-devel/2017-September/084778.html> 
> for exactly this case.  I don't know if we want to modify how this works to 
> be more like what was used in libavcodec, though.
> 
> Thoughts?  I can send the set again if that would be helpful.

That sounds generally like a good idea, although I'm not sure how
exactly the details should work.

In theory, it would be nice if libavfilter would have an automagic way
to figure out the optimal pool sizes (and it'd have to involve decoder
and encoder somehow), but it doesn't look like we'll get such a thing
any time soon. So manual configuration it is, which should be still
better than putting random sizes into the code to evade the problem.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to