[FFmpeg-devel] [FFmpeg-cvslog] lavu/qsv: make a copy as libmfx alignment requirement for uploading

[Mark Thompson]

On 23/10/18 08:49, Li, Zhong wrote:
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Mark Thompson
>> Sent: Sunday, October 14, 2018 12:36 AM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] lavu/qsv: make a copy as
>> libmfx alignment requirement for uploading
>>
>> On 11/10/18 06:38, Zhong Li wrote:
>>> ffmpeg | branch: master | Zhong Li <zhong.li at intel.com> | Mon Sep 17
>>> 19:16:44 2018 +0800| [681aa7d14f97fd98181ca6d61e11be48fe65692d] |
>>> committer: Zhong Li
>>>
>>> lavu/qsv: make a copy as libmfx alignment requirement for uploading
>>>
>>> Libmfx requires 16 bytes aligned input/output for uploading.
>>> Currently only output is 16 byte aligned and assigning same
>>> width/height to input with smaller buffer size actually, thus definitely will
>> cause segment fault.
>>>
>>> Can reproduce with any 1080p nv12 rawvideo input:
>>> ffmpeg -init_hw_device qsv=qsv:hw -hwaccel qsv -filter_hw_device qsv
>>> -f rawvideo -pix_fmt nv12 -s:v 1920x1080 -i 1080p_nv12.yuv -vf
>>> 'format=nv12,hwupload=extra_hw_frames=16,hwdownload,format=nv12'
>> -an
>>> -y out_nv12.yuv
>>>
>>> It can fix #7418
>>>
>>> Signed-off-by: Zhong Li <zhong.li at intel.com>
>>>
>>>>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=681aa7d14f9
>>>> 7fd98181ca6d61e11be48fe65692d
>>> ---
>>>
>>>  libavutil/hwcontext_qsv.c | 31 +++++++++++++++++++++++++++++--
>>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
>>> index 33e121b416..814ce215ce 100644
>>> --- a/libavutil/hwcontext_qsv.c
>>> +++ b/libavutil/hwcontext_qsv.c
>>> @@ -862,6 +862,10 @@ static int
>> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>>>      mfxSyncPoint sync = NULL;
>>>      mfxStatus err;
>>>      int ret = 0;
>>> +    /* make a copy if the input is not padded as libmfx requires */
>>> +    AVFrame tmp_frame, *src_frame;
>>> +    int realigned = 0;
>>> +
>>>
>>>      while (!s->session_upload_init && !s->session_upload && !ret) {
>>> #if HAVE_PTHREADS @@ -887,16 +891,36 @@ static int
>>> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>>>      if (ret < 0)
>>>          return ret;
>>>
>>> +
>>> +    if (src->height & 16 || src->linesize[0] & 16) {
>>
>> Doesn't this still need to check that the layout is compatible with other the
>> limitations that libmfx has before trying to do the upload?  In particular,
>> that the pitches for the different planes are actually all the same - I certainly
>> wouldn't expect them to be for YUV420P.
> 
> Do you mean our previous discussion on https://lists.libav.org/pipermail/libav-devel/2018-April/086070.html ?
> If yes, I will try to reproduce it with hwupload pipeline instead of transcoding pipeline, and then give an update patch.

Yes.  You might need to write a small test program to generation an arbitrary allocation pattern, since libavutil only has a fixed layout.

>>
>> (IIRC there was a requirement 0 < frame->data[N] - frame->data[0] < 2^24
>> as well (or something like that, I might be wrong), which also need not be
>> true.)
> 
> I don't know such a requirement, could it be found from MSDK guide? 

That came from discussion of hardware capabilities around VAAPI, I think.

Won't something like this be the reason for wanting a virtually contiguous buffer?  If there isn't a restriction along these lines then there is no need to make the buffer virtually contiguous.  (After all, the hardware can't know about C undefined behaviour in pointer comparisons...)

- Mark