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

Mark Thompson sw at jkqxz.net
Fri Jan 26 12:04:12 CET 2018


On 26/01/18 09:15, wm4 wrote:
> 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.

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.

- Mark


More information about the libav-devel mailing list