[FFmpeg-devel] [PATCH] avutils/hwcontext_qsv: set the source device in qsv_device_create

[Mark Thompson]

On 11/03/2021 08:53, Xu, Guangxin wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Guangxin Xu
>> Sent: Friday, March 5, 2021 9:46 AM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH] avutils/hwcontext_qsv: set the source
>> device in qsv_device_create
>>
>> On Tue, Feb 23, 2021 at 9:34 AM Guangxin Xu <oddstone at gmail.com> wrote:
>>
>>>
>>>
>>> On Mon, Feb 22, 2021 at 5:17 PM Soft Works <softworkz at hotmail.com>
>> wrote:
>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf
>> Of
>>>>> Xu Guangxin
>>>>> Sent: Monday, February 22, 2021 9:45 AM
>>>>> To: ffmpeg-devel at ffmpeg.org
>>>>> Cc: Xu Guangxin <guangxin.xu at intel.com>
>>>>> Subject: [FFmpeg-devel] [PATCH] avutils/hwcontext_qsv: set the
>>>>> source device in qsv_device_create
>>>>>
>>>>> opencl_device_derive only handles AV_HWDEVICE_TYPE_VAAPI.
>>>>> We need a source device for qsv.
>>>>>
>>>>> this will fix following pipeline:
>>>>> ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128
>>>>> -init_hw_device opencl=ocl at intel -hwaccel qsv -c:v h264_qsv
>>>>> -hwaccel_output_format qsv
>>>> -i
>>>>> $input -filter_hw_device ocl -vf
>>>>>
>> 'hwmap=derive_device=opencl,format=opencl,unsharp_opencl,hwmap=der
>>>>> ive_device=qsv:reverse=1:extra_hw_frames=32'  -c:v hevc_qsv  -y
>>>> test.h265
>>>>> ---
>>>>>   libavutil/hwcontext_qsv.c | 8 +++++++-
>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
>>>>> index 35a944f8f8..af3ee32cac 100644
>>>>> --- a/libavutil/hwcontext_qsv.c
>>>>> +++ b/libavutil/hwcontext_qsv.c
>>>>> @@ -1269,7 +1269,13 @@ static int
>>>>> qsv_device_create(AVHWDeviceContext
>>>>> *ctx, const char *device,
>>>>>
>>>>>       impl = choose_implementation(device);
>>>>>
>>>>> -    return qsv_device_derive_from_child(ctx, impl, child_device, 0);
>>>>> +    ret = qsv_device_derive_from_child(ctx, impl, child_device, 0);
>>>>> +    if (ret == 0) {
>>>>> +        ctx->internal->source_device =
>>>> av_buffer_ref(priv->child_device_ctx);
>>>>> +        if (!ctx->internal->source_device)
>>>>> +            ret = AVERROR(ENOMEM);
>>>>> +    }
>>>>> +    return ret;
>>>>>   }
>>>>
>>>> That's funny, I made almost the same change only two days ago:
>>>>
>>>>      impl = choose_implementation(device);
>>>>      ret = qsv_device_derive_from_child(ctx, impl, child_device, 0);
>>>>      if (ret >= 0)
>>>>          ctx->internal->source_device =
>>>> av_buffer_ref(priv->child_device_ctx);
>>>>
>>>>      return ret;
>>>>
>>>>
>>>>  From my POV, this change is correct and required.
>>>>
>>> Glad to hear this. Thanks for the endorsement  :)
>>>
>> Hi softworkz,
>> Could you help merge this?
>> thanks
>>
> Hmm, this patch will failed for "make fate-hwdevice V=2"
> It because Qsv's internal->source_device is vaapi.
> When we derivation qsv to vaapi, and derivation back again, it will create a new qsv device.
> 
> Hi Mark,
> Can we remove this test for qsv and vaapi? Or could you suggest a better way for me?
> https://github.com/FFmpeg/FFmpeg/blob/069d2b4a50a6eb2f925f36884e6b9bd9a1e54670/libavutil/tests/hwdevice.c#L75
> 
> thanks
> 
> ...
> Successfully tested derivation vaapi -> qsv.
> Test passed for vaapi with device :0.
> Device type qsv successfully created.
> Derivation qsv to vaapi succeeded, but derivation back again did not return the original device.
> Test failed for qsv with default options.
> Attempted to test 2 device types: 1 passed, 1 failed, 0 skipped.
> make: *** [tests/Makefile:256: fate-hwdevice] Error 1

You are hacking it to lie to the hwcontext implementation around how the device was derived, so the test fails because it finds a different derivation structure to what it created?  That seems correct?

Your command-line was:

$ ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -init_hw_device opencl=ocl at intel -hwaccel qsv -c:v h264_qsv -hwaccel_output_format qsv -i $input -filter_hw_device ocl -vf 'hwmap=derive_device=opencl,format=opencl,unsharp_opencl,hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32'  -c:v hevc_qsv  -y test.h265

So that's trying to make five device references in turn:

1. Make a VAAPI device explicitly.
2. Derive an OpenCL device from the VAAPI device 1.
3. Make a libmfx device implicitly (because a libmfx decoder was requested).
4. Derive a new OpenCL device from the libmfx device 3 (you told the filter graph about device 2, but then didn't use it anywhere).
5. Derive a libmfx device from the OpenCL device 4, which should return libmfx device 3.

What's going wrong?  Step 4 fails to create an OpenCL device because devices 1/2 and device 3 aren't actually connected together at all.

How to solve that?  Either we can fix the connection, or we could just use the device we already have in step 4 rather than trying to create a new one.

To fix the connection, derive device 3 from device 1.  This doesn't work in the ffmpeg utility because the hacky support in ffmpeg_qsv.c doesn't support the normal device stuff.  It would work in your own program.

Using the existing device (by using device 2 rather than deriving a new on at step 4) looks like it should work:

$ ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -init_hw_device opencl=ocl at intel -hwaccel qsv -c:v h264_qsv -hwaccel_output_format qsv -i $input -filter_hw_device ocl -vf 'hwmap,format=opencl,unsharp_opencl,hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32'  -c:v hevc_qsv  -y test.h265

Unfortunately it doesn't, and for the same reason that the first approach didn't - devices 1 and 3 aren't connected, so the OpenCL mapping is being given frames in the wrong device context and therefore fails (in fact the Intel ICD crashes with an abort, which is about the best you can hope for with this sort of undefined behaviour).


So, how can we make this work?  Well, the weird libmfx hacks are causing the problem, so let's just duck that by dumping the pointless wrapper decoder and using the normal decoder directly:

$ ffmpeg -init_hw_device vaapi=intel:/dev/dri/renderD128 -hwaccel vaapi -hwaccel_output_format vaapi -i $input -vf 'hwmap=derive_device=opencl,format=opencl,unsharp_opencl,hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32'  -c:v hevc_qsv  -y test.h265

If you want to make it work with the libmfx wrapper decoder, then I think the most sensible way is to implement AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX in that decoder so it works like the others and the ffmpeg_qsv.c hacks can be removed entirely.

- Mark