diff mbox series

[FFmpeg-devel,v7,1/8] libavcodec/qsv: enabling d3d11va support, added mfxhdlpair

Message ID 20201103184557.18444-1-artem.galin@intel.com
State New
Headers show
Series [FFmpeg-devel,v7,1/8] libavcodec/qsv: enabling d3d11va support, added mfxhdlpair | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

galinart Nov. 3, 2020, 6:45 p.m. UTC
Adding DX11 relevant device type checks and adjusting callbacks with
proper MediaSDK pair type support.

Extending structure for proper MediaSDK pair type support.

Signed-off-by: Artem Galin <artem.galin@intel.com>
---
 libavcodec/qsv.c          | 53 ++++++++++++++++++++++++++-------------
 libavcodec/qsv_internal.h |  2 +-
 2 files changed, 37 insertions(+), 18 deletions(-)

Comments

Max Dmitrichenko Nov. 14, 2020, 4:49 p.m. UTC | #1
On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com> wrote:

> Adding DX11 relevant device type checks and adjusting callbacks with
> proper MediaSDK pair type support.
>
> Extending structure for proper MediaSDK pair type support.
>
> Signed-off-by: Artem Galin <artem.galin@intel.com>
> .....


Patchset obviously closes the gap of DirectX 11 support
and already checked with several apps that use FFMPEG.

Any particular review comments should be yet expected?

Changes seem to be straight forward
and incorporate prev. comments.

thank in advance

regards
Max
James Almer July 13, 2021, 2:53 p.m. UTC | #2
On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com> wrote:
> 
>> Adding DX11 relevant device type checks and adjusting callbacks with
>> proper MediaSDK pair type support.
>>
>> Extending structure for proper MediaSDK pair type support.
>>
>> Signed-off-by: Artem Galin <artem.galin@intel.com>
>> .....
> 
> 
> Patchset obviously closes the gap of DirectX 11 support
> and already checked with several apps that use FFMPEG.
> 
> Any particular review comments should be yet expected?
> 
> Changes seem to be straight forward
> and incorporate prev. comments.
> 
> thank in advance
> 
> regards
> Max

There are some issues pointed out by Soft Works related to switching the 
default to DX11 breaking existing command lines with DX9, plus an 
OpenCL<->QSV interop regression this would introduce that according to 
him should be easy to fix.

If those are addressed, the set should be good.
Xiang, Haihao July 15, 2021, 2:34 a.m. UTC | #3
On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com> wrote:
> > 
> > > Adding DX11 relevant device type checks and adjusting callbacks with
> > > proper MediaSDK pair type support.
> > > 
> > > Extending structure for proper MediaSDK pair type support.
> > > 
> > > Signed-off-by: Artem Galin <artem.galin@intel.com>
> > > .....
> > 
> > 
> > Patchset obviously closes the gap of DirectX 11 support
> > and already checked with several apps that use FFMPEG.
> > 
> > Any particular review comments should be yet expected?
> > 
> > Changes seem to be straight forward
> > and incorporate prev. comments.
> > 
> > thank in advance
> > 
> > regards
> > Max
> 
> There are some issues pointed out by Soft Works related to switching the 
> default to DX11 breaking existing command lines with DX9, plus an 
> OpenCL<->QSV interop regression this would introduce that according to 
> him should be easy to fix.
> 
> If those are addressed, the set should be good.

If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281778.html
(qsvdec: add support for HW_DEVICE_CTX method) first, we should be able to use the common device stuff to initialize d3d11va device for QSV and needn't use child_device_type to specify child device.

-init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va 

Thanks
Haihao

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Soft Works July 15, 2021, 3:10 a.m. UTC | #4
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Thursday, 15 July 2021 04:35
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> d3d11va support, added mfxhdlpair
> 
> On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com>
> wrote:
> > >
> > > > Adding DX11 relevant device type checks and adjusting callbacks
> > > > with proper MediaSDK pair type support.
> > > >
> > > > Extending structure for proper MediaSDK pair type support.
> > > >
> > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > >
> > >
> > > Patchset obviously closes the gap of DirectX 11 support and already
> > > checked with several apps that use FFMPEG.
> > >
> > > Any particular review comments should be yet expected?
> > >
> > > Changes seem to be straight forward
> > > and incorporate prev. comments.
> > >
> > > thank in advance
> > >
> > > regards
> > > Max
> >
> > There are some issues pointed out by Soft Works related to switching
> > the default to DX11 breaking existing command lines with DX9, plus an
> > OpenCL<->QSV interop regression this would introduce that according to
> > him should be easy to fix.
> >
> > If those are addressed, the set should be good.
> 
> If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> June/281778.html
> (qsvdec: add support for HW_DEVICE_CTX method) first, we should be able
> to use the common device stuff to initialize d3d11va device for QSV and
> needn't use child_device_type to specify child device.

There's no doubt that the new method will be better, but the point is not to break existing command lines. From my experience, ffmpeg usually avoids to break established command line patterns, which is a good thing imo.

Anything more official than my perception? Please feel free to chime in... ;-)

softworkz

I
James Almer July 15, 2021, 3:20 a.m. UTC | #5
On 7/15/2021 12:10 AM, Soft Works wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Xiang, Haihao
>> Sent: Thursday, 15 July 2021 04:35
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
>> d3d11va support, added mfxhdlpair
>>
>> On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
>>> On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
>>>> On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com>
>> wrote:
>>>>
>>>>> Adding DX11 relevant device type checks and adjusting callbacks
>>>>> with proper MediaSDK pair type support.
>>>>>
>>>>> Extending structure for proper MediaSDK pair type support.
>>>>>
>>>>> Signed-off-by: Artem Galin <artem.galin@intel.com> .....
>>>>
>>>>
>>>> Patchset obviously closes the gap of DirectX 11 support and already
>>>> checked with several apps that use FFMPEG.
>>>>
>>>> Any particular review comments should be yet expected?
>>>>
>>>> Changes seem to be straight forward
>>>> and incorporate prev. comments.
>>>>
>>>> thank in advance
>>>>
>>>> regards
>>>> Max
>>>
>>> There are some issues pointed out by Soft Works related to switching
>>> the default to DX11 breaking existing command lines with DX9, plus an
>>> OpenCL<->QSV interop regression this would introduce that according to
>>> him should be easy to fix.
>>>
>>> If those are addressed, the set should be good.
>>
>> If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
>> June/281778.html
>> (qsvdec: add support for HW_DEVICE_CTX method) first, we should be able
>> to use the common device stuff to initialize d3d11va device for QSV and
>> needn't use child_device_type to specify child device.
> 
> There's no doubt that the new method will be better, but the point is not to break existing command lines. From my experience, ffmpeg usually avoids to break established command line patterns, which is a good thing imo.
> 
> Anything more official than my perception? Please feel free to chime in... ;-)

That patch appears to attempt to maintain support for existing command 
lines for a deprecation period, something we've done before with other 
hwaccels like cuvid, so if it's really better, it's fine and acceptable.

> 
> softworkz
> 
> I
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Soft Works July 15, 2021, 3:49 a.m. UTC | #6
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> James Almer
> Sent: Thursday, 15 July 2021 05:21
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> d3d11va support, added mfxhdlpair
> 
> On 7/15/2021 12:10 AM, Soft Works wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Xiang, Haihao
> >> Sent: Thursday, 15 July 2021 04:35
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> >> d3d11va support, added mfxhdlpair
> >>
> >> On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> >>> On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> >>>> On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com>
> >> wrote:
> >>>>
> >>>>> Adding DX11 relevant device type checks and adjusting callbacks
> >>>>> with proper MediaSDK pair type support.
> >>>>>
> >>>>> Extending structure for proper MediaSDK pair type support.
> >>>>>
> >>>>> Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> >>>>
> >>>>
> >>>> Patchset obviously closes the gap of DirectX 11 support and already
> >>>> checked with several apps that use FFMPEG.
> >>>>
> >>>> Any particular review comments should be yet expected?
> >>>>
> >>>> Changes seem to be straight forward and incorporate prev. comments.
> >>>>
> >>>> thank in advance
> >>>>
> >>>> regards
> >>>> Max
> >>>
> >>> There are some issues pointed out by Soft Works related to switching
> >>> the default to DX11 breaking existing command lines with DX9, plus
> >>> an OpenCL<->QSV interop regression this would introduce that
> >>> according to him should be easy to fix.
> >>>
> >>> If those are addressed, the set should be good.
> >>
> >> If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> >> June/281778.html
> >> (qsvdec: add support for HW_DEVICE_CTX method) first, we should be
> >> able to use the common device stuff to initialize d3d11va device for
> >> QSV and needn't use child_device_type to specify child device.
> >
> > There's no doubt that the new method will be better, but the point is not
> to break existing command lines. From my experience, ffmpeg usually avoids
> to break established command line patterns, which is a good thing imo.
> >
> > Anything more official than my perception? Please feel free to chime
> > in... ;-)
> 
> That patch appears to attempt to maintain support for existing command
> lines for a deprecation period, something we've done before with other
> hwaccels like cuvid, so if it's really better, it's fine and acceptable.
^

Hi James,

"That patch" that preserves compatibility doesn't exist yet. We need to make sure that command lines using the '--child_device' parameter will continue to work, while Haihao's patch will be the "future way":

> -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va

The new behaviour that is introduced by this patch is that an initialized hw device can now be applied to qsv decoders (currently only to filter graphs and qsv encoders).

Additionally, the default should remain to be D3D9 (if none of the above is specified) as laid out earlier - again for maintaining compatibility but also for better robustness and performance.

PS: Regarding the OpenCL regression you mentioned, I had already submitted the patch to Intel and they have added it to their staging-repo for ffmpeg patches: https://github.com/intel-media-ci/cartwheel-ffmpeg/commit/7456b1ac0d385acc6d2851470c1b04ab3780adf8

Kind regards,
softworkz
Xiang, Haihao July 15, 2021, 5:43 a.m. UTC | #7
On Thu, 2021-07-15 at 03:49 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > James Almer
> > Sent: Thursday, 15 July 2021 05:21
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > d3d11va support, added mfxhdlpair
> > 
> > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Xiang, Haihao
> > > > Sent: Thursday, 15 July 2021 04:35
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > > d3d11va support, added mfxhdlpair
> > > > 
> > > > On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > > > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > > > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com>
> > > > 
> > > > wrote:
> > > > > > 
> > > > > > > Adding DX11 relevant device type checks and adjusting callbacks
> > > > > > > with proper MediaSDK pair type support.
> > > > > > > 
> > > > > > > Extending structure for proper MediaSDK pair type support.
> > > > > > > 
> > > > > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > > > > 
> > > > > > 
> > > > > > Patchset obviously closes the gap of DirectX 11 support and already
> > > > > > checked with several apps that use FFMPEG.
> > > > > > 
> > > > > > Any particular review comments should be yet expected?
> > > > > > 
> > > > > > Changes seem to be straight forward and incorporate prev. comments.
> > > > > > 
> > > > > > thank in advance
> > > > > > 
> > > > > > regards
> > > > > > Max
> > > > > 
> > > > > There are some issues pointed out by Soft Works related to switching
> > > > > the default to DX11 breaking existing command lines with DX9, plus
> > > > > an OpenCL<->QSV interop regression this would introduce that
> > > > > according to him should be easy to fix.
> > > > > 
> > > > > If those are addressed, the set should be good.
> > > > 
> > > > If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > June/281778.html
> > > > (qsvdec: add support for HW_DEVICE_CTX method) first, we should be
> > > > able to use the common device stuff to initialize d3d11va device for
> > > > QSV and needn't use child_device_type to specify child device.
> > > 
> > > There's no doubt that the new method will be better, but the point is not
> > 
> > to break existing command lines. From my experience, ffmpeg usually avoids
> > to break established command line patterns, which is a good thing imo.
> > > 
> > > Anything more official than my perception? Please feel free to chime
> > > in... ;-)
> > 
> > That patch appears to attempt to maintain support for existing command
> > lines for a deprecation period, something we've done before with other
> > hwaccels like cuvid, so if it's really better, it's fine and acceptable.
> 
> ^
> 
> Hi James,
> 
> "That patch" that preserves compatibility doesn't exist yet. We need to make
> sure that command lines using the '--child_device' parameter will continue to
> work, while Haihao's patch will be the "future way":
> 
> > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va
> 
> The new behaviour that is introduced by this patch is that an initialized hw
> device can now be applied to qsv decoders (currently only to filter graphs and
> qsv encoders).

'-init_hw_device qsv=qsv:hw,child_device=xxx' still works,

e.g. /dev/dri/renderD129 is used for both qsv decoder and encoder on Linux when
running the command below on a HW with multiple GPUs.

$> ffmpeg -y -init_hw_device qsv=qsv:hw,child_device=/dev/dri/renderD129
-hwaccel qsv -c:v hevc_qsv -i input.265 -c:v hevc_qsv -f rawvideo out.h265

> 
> Additionally, the default should remain to be D3D9 (if none of the above is
> specified) as laid out earlier - again for maintaining compatibility but also
> for better robustness and performance.

We may refine Artem's patch to make sure the default is D3D9 on Microsoft
windows when both dxva2 and d3d11va are enabled if applying my patch first. 

Thanks
Haihao


> 
> PS: Regarding the OpenCL regression you mentioned, I had already submitted the
> patch to Intel and they have added it to their staging-repo for ffmpeg
> patches: 
> https://github.com/intel-media-ci/cartwheel-ffmpeg/commit/7456b1ac0d385acc6d2851470c1b04ab3780adf8
> 
> Kind regards,
> softworkz
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Soft Works July 15, 2021, 6:43 a.m. UTC | #8
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Thursday, 15 July 2021 07:43
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> d3d11va support, added mfxhdlpair
> 
> On Thu, 2021-07-15 at 03:49 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > James Almer
> > > Sent: Thursday, 15 July 2021 05:21
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > d3d11va support, added mfxhdlpair
> > >
> > > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> > > > > Of Xiang, Haihao
> > > > > Sent: Thursday, 15 July 2021 04:35
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > enabling d3d11va support, added mfxhdlpair
> > > > >
> > > > > On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > > > > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > > > > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin
> > > > > > > <artem.galin@gmail.com>
> > > > >
> > > > > wrote:
> > > > > > >
> > > > > > > > Adding DX11 relevant device type checks and adjusting
> > > > > > > > callbacks with proper MediaSDK pair type support.
> > > > > > > >
> > > > > > > > Extending structure for proper MediaSDK pair type support.
> > > > > > > >
> > > > > > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > > > > >
> > > > > > >
> > > > > > > Patchset obviously closes the gap of DirectX 11 support and
> > > > > > > already checked with several apps that use FFMPEG.
> > > > > > >
> > > > > > > Any particular review comments should be yet expected?
> > > > > > >
> > > > > > > Changes seem to be straight forward and incorporate prev.
> comments.
> > > > > > >
> > > > > > > thank in advance
> > > > > > >
> > > > > > > regards
> > > > > > > Max
> > > > > >
> > > > > > There are some issues pointed out by Soft Works related to
> > > > > > switching the default to DX11 breaking existing command lines
> > > > > > with DX9, plus an OpenCL<->QSV interop regression this would
> > > > > > introduce that according to him should be easy to fix.
> > > > > >
> > > > > > If those are addressed, the set should be good.
> > > > >
> > > > > If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > June/281778.html
> > > > > (qsvdec: add support for HW_DEVICE_CTX method) first, we should
> > > > > be able to use the common device stuff to initialize d3d11va
> > > > > device for QSV and needn't use child_device_type to specify child
> device.
> > > >
> > > > There's no doubt that the new method will be better, but the point
> > > > is not
> > >
> > > to break existing command lines. From my experience, ffmpeg usually
> > > avoids to break established command line patterns, which is a good thing
> imo.
> > > >
> > > > Anything more official than my perception? Please feel free to
> > > > chime in... ;-)
> > >
> > > That patch appears to attempt to maintain support for existing
> > > command lines for a deprecation period, something we've done before
> > > with other hwaccels like cuvid, so if it's really better, it's fine and
> acceptable.
> >
> > ^
> >
> > Hi James,
> >
> > "That patch" that preserves compatibility doesn't exist yet. We need
> > to make sure that command lines using the '--child_device' parameter
> > will continue to work, while Haihao's patch will be the "future way":
> >
> > > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va
> >
> > The new behaviour that is introduced by this patch is that an
> > initialized hw device can now be applied to qsv decoders (currently
> > only to filter graphs and qsv encoders).
> 
> '-init_hw_device qsv=qsv:hw,child_device=xxx' still works,

Sure - that needs to work anyway. What I meant is the -qsv_device parameter, which is a global-scope param and takes a device path on Linux or the D3D adapter ID on Windows.

ffmpeg -hwaccel qsv -qsv_device /dev/dri/renderD128 -c:v h264_qsv -i input.mp4 -c:v h264_qsv output.mp4

For my implementation I had added an additional global parameter (--qsv_use_dx11). When it is set, the -qsv_device parameter indicates a DXGI adapter id rather than a D3D adapter. But let's forget about that. The init_hw_device approach is much better, and -qsv_device should just keep working like before.

I'm currently rebasing all our patches to the latest ffmpeg; probably tomorrow I'll check out your patch.

> > Additionally, the default should remain to be D3D9 (if none of the
> > above is
> > specified) as laid out earlier - again for maintaining compatibility
> > but also for better robustness and performance.
> 
> We may refine Artem's patch to make sure the default is D3D9 on Microsoft
> windows 

Sounds good.

> when both dxva2 and d3d11va are enabled if applying my patch
> first.

Not sure what you mean by that. 

Kind regards,
softworkz
Xiang, Haihao July 15, 2021, 7:55 a.m. UTC | #9
On Thu, 2021-07-15 at 06:43 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Thursday, 15 July 2021 07:43
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > d3d11va support, added mfxhdlpair
> > 
> > On Thu, 2021-07-15 at 03:49 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > James Almer
> > > > Sent: Thursday, 15 July 2021 05:21
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > > d3d11va support, added mfxhdlpair
> > > > 
> > > > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > 
> > Behalf
> > > > > > Of Xiang, Haihao
> > > > > > Sent: Thursday, 15 July 2021 04:35
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > > enabling d3d11va support, added mfxhdlpair
> > > > > > 
> > > > > > On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > > > > > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > > > > > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin
> > > > > > > > <artem.galin@gmail.com>
> > > > > > 
> > > > > > wrote:
> > > > > > > > 
> > > > > > > > > Adding DX11 relevant device type checks and adjusting
> > > > > > > > > callbacks with proper MediaSDK pair type support.
> > > > > > > > > 
> > > > > > > > > Extending structure for proper MediaSDK pair type support.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Patchset obviously closes the gap of DirectX 11 support and
> > > > > > > > already checked with several apps that use FFMPEG.
> > > > > > > > 
> > > > > > > > Any particular review comments should be yet expected?
> > > > > > > > 
> > > > > > > > Changes seem to be straight forward and incorporate prev.
> > 
> > comments.
> > > > > > > > 
> > > > > > > > thank in advance
> > > > > > > > 
> > > > > > > > regards
> > > > > > > > Max
> > > > > > > 
> > > > > > > There are some issues pointed out by Soft Works related to
> > > > > > > switching the default to DX11 breaking existing command lines
> > > > > > > with DX9, plus an OpenCL<->QSV interop regression this would
> > > > > > > introduce that according to him should be easy to fix.
> > > > > > > 
> > > > > > > If those are addressed, the set should be good.
> > > > > > 
> > > > > > If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > > June/281778.html
> > > > > > (qsvdec: add support for HW_DEVICE_CTX method) first, we should
> > > > > > be able to use the common device stuff to initialize d3d11va
> > > > > > device for QSV and needn't use child_device_type to specify child
> > 
> > device.
> > > > > 
> > > > > There's no doubt that the new method will be better, but the point
> > > > > is not
> > > > 
> > > > to break existing command lines. From my experience, ffmpeg usually
> > > > avoids to break established command line patterns, which is a good thing
> > 
> > imo.
> > > > > 
> > > > > Anything more official than my perception? Please feel free to
> > > > > chime in... ;-)
> > > > 
> > > > That patch appears to attempt to maintain support for existing
> > > > command lines for a deprecation period, something we've done before
> > > > with other hwaccels like cuvid, so if it's really better, it's fine and
> > 
> > acceptable.
> > > 
> > > ^
> > > 
> > > Hi James,
> > > 
> > > "That patch" that preserves compatibility doesn't exist yet. We need
> > > to make sure that command lines using the '--child_device' parameter
> > > will continue to work, while Haihao's patch will be the "future way":
> > > 
> > > > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va
> > > 
> > > The new behaviour that is introduced by this patch is that an
> > > initialized hw device can now be applied to qsv decoders (currently
> > > only to filter graphs and qsv encoders).
> > 
> > '-init_hw_device qsv=qsv:hw,child_device=xxx' still works,
> 
> Sure - that needs to work anyway. What I meant is the -qsv_device parameter,
> which is a global-scope param and takes a device path on Linux or the D3D
> adapter ID on Windows.
> 
> ffmpeg -hwaccel qsv -qsv_device /dev/dri/renderD128 -c:v h264_qsv -i input.mp4
> -c:v h264_qsv output.mp4

Yes, -qsv_device still works, I added opt_qsv_device() to handle qsv_device
option. 

> 
> For my implementation I had added an additional global parameter (
> --qsv_use_dx11). When it is set, the -qsv_device parameter indicates a DXGI
> adapter id rather than a D3D adapter. But let's forget about that. The
> init_hw_device approach is much better, and -qsv_device should just keep
> working like before.
> 
> I'm currently rebasing all our patches to the latest ffmpeg; probably tomorrow
> I'll check out your patch.

Thanks for checking out the patch. 

> 
> > > Additionally, the default should remain to be D3D9 (if none of the
> > > above is
> > > specified) as laid out earlier - again for maintaining compatibility
> > > but also for better robustness and performance.
> > 
> > We may refine Artem's patch to make sure the default is D3D9 on Microsoft
> > windows 
> 
> Sounds good.
> 
> > when both dxva2 and d3d11va are enabled if applying my patch
> > first.
> 
> Not sure what you mean by that. 

What I meant is we needn't use parameter child_device_type (which was added in
Artem's patch) to specify the child device type after applying my patch, but we
should make sure the default child device type is DXVA2 when both DXVA2 and
D3D11VA are available however user doesn't specify the type. E.g. '-
init_hw_device qsv=qsv:hw' should work with DXVA2, not D3D11VA if both DXVA2 and
D3D11VA are availabe. 

Thanks
Haihao


> 
> Kind regards,
> softworkz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Soft Works July 15, 2021, 9:32 a.m. UTC | #10
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Thursday, 15 July 2021 09:56
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> d3d11va support, added mfxhdlpair
> 
> On Thu, 2021-07-15 at 06:43 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Xiang, Haihao
> > > Sent: Thursday, 15 July 2021 07:43
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > d3d11va support, added mfxhdlpair
> > >
> > > On Thu, 2021-07-15 at 03:49 +0000, Soft Works wrote:
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> > > > > Of James Almer
> > > > > Sent: Thursday, 15 July 2021 05:21
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > enabling d3d11va support, added mfxhdlpair
> > > > >
> > > > > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > >
> > > Behalf
> > > > > > > Of Xiang, Haihao
> > > > > > > Sent: Thursday, 15 July 2021 04:35
> > > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > > > enabling d3d11va support, added mfxhdlpair
> > > > > > >
> > > > > > > On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > > > > > > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > > > > > > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin
> > > > > > > > > <artem.galin@gmail.com>
> > > > > > >
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Adding DX11 relevant device type checks and adjusting
> > > > > > > > > > callbacks with proper MediaSDK pair type support.
> > > > > > > > > >
> > > > > > > > > > Extending structure for proper MediaSDK pair type support.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Patchset obviously closes the gap of DirectX 11 support
> > > > > > > > > and already checked with several apps that use FFMPEG.
> > > > > > > > >
> > > > > > > > > Any particular review comments should be yet expected?
> > > > > > > > >
> > > > > > > > > Changes seem to be straight forward and incorporate prev.
> > >
> > > comments.
> > > > > > > > >
> > > > > > > > > thank in advance
> > > > > > > > >
> > > > > > > > > regards
> > > > > > > > > Max
> > > > > > > >
> > > > > > > > There are some issues pointed out by Soft Works related to
> > > > > > > > switching the default to DX11 breaking existing command
> > > > > > > > lines with DX9, plus an OpenCL<->QSV interop regression
> > > > > > > > this would introduce that according to him should be easy to fix.
> > > > > > > >
> > > > > > > > If those are addressed, the set should be good.
> > > > > > >
> > > > > > > If we may apply
> > > > > > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > > > June/281778.html
> > > > > > > (qsvdec: add support for HW_DEVICE_CTX method) first, we
> > > > > > > should be able to use the common device stuff to initialize
> > > > > > > d3d11va device for QSV and needn't use child_device_type to
> > > > > > > specify child
> > >
> > > device.
> > > > > >
> > > > > > There's no doubt that the new method will be better, but the
> > > > > > point is not
> > > > >
> > > > > to break existing command lines. From my experience, ffmpeg
> > > > > usually avoids to break established command line patterns, which
> > > > > is a good thing
> > >
> > > imo.
> > > > > >
> > > > > > Anything more official than my perception? Please feel free to
> > > > > > chime in... ;-)
> > > > >
> > > > > That patch appears to attempt to maintain support for existing
> > > > > command lines for a deprecation period, something we've done
> > > > > before with other hwaccels like cuvid, so if it's really better,
> > > > > it's fine and
> > >
> > > acceptable.
> > > >
> > > > ^
> > > >
> > > > Hi James,
> > > >
> > > > "That patch" that preserves compatibility doesn't exist yet. We
> > > > need to make sure that command lines using the '--child_device'
> > > > parameter will continue to work, while Haihao's patch will be the
> "future way":
> > > >
> > > > > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device
> qsv@d3d11va
> > > >
> > > > The new behaviour that is introduced by this patch is that an
> > > > initialized hw device can now be applied to qsv decoders
> > > > (currently only to filter graphs and qsv encoders).
> > >
> > > '-init_hw_device qsv=qsv:hw,child_device=xxx' still works,
> >
> > Sure - that needs to work anyway. What I meant is the -qsv_device
> > parameter, which is a global-scope param and takes a device path on
> > Linux or the D3D adapter ID on Windows.
> >
> > ffmpeg -hwaccel qsv -qsv_device /dev/dri/renderD128 -c:v h264_qsv -i
> > input.mp4 -c:v h264_qsv output.mp4
> 
> Yes, -qsv_device still works, I added opt_qsv_device() to handle qsv_device
> option.
> 
> >
> > For my implementation I had added an additional global parameter (
> > --qsv_use_dx11). When it is set, the -qsv_device parameter indicates a
> > DXGI adapter id rather than a D3D adapter. But let's forget about
> > that. The init_hw_device approach is much better, and -qsv_device
> > should just keep working like before.
> >
> > I'm currently rebasing all our patches to the latest ffmpeg; probably
> > tomorrow I'll check out your patch.
> 
> Thanks for checking out the patch.
> 
> >
> > > > Additionally, the default should remain to be D3D9 (if none of the
> > > > above is
> > > > specified) as laid out earlier - again for maintaining
> > > > compatibility but also for better robustness and performance.
> > >
> > > We may refine Artem's patch to make sure the default is D3D9 on
> > > Microsoft windows
> >
> > Sounds good.
> >
> > > when both dxva2 and d3d11va are enabled if applying my patch first.
> >
> > Not sure what you mean by that.
> 
> What I meant is we needn't use parameter child_device_type (which was
> added in Artem's patch) to specify the child device type after applying my
> patch, but we should make sure the default child device type is DXVA2 when
> both DXVA2 and D3D11VA are available however user doesn't specify the
> type. E.g. '- init_hw_device qsv=qsv:hw' should work with DXVA2, not
> D3D11VA if both DXVA2 and D3D11VA are availabe.

Ah, thanks for the clarification, but that doesn't work out. The parameter - child_device_type is essential:

1. The device numbers that you specify via qsv_device or child_device are totally different between D3D9 and D3D11/DXGI. In the latter case, you specify the index of a graphics adapter/hardware device, no matter how many displays you _can_ connect to it and no matter how many displays _are_ connected.
In case of D3D9, you specify the index in a collection of connected displays  which belong to the context of the active user session.
=> without specifying a device type, an index number is meaningless

2. It's important that the behavior is deterministic: when you specify D3D9, you need to be able to rely on getting D3D9 (or fail otherwise), same for D3D11. 

3. When you want to combine a DXVA2/D3D11VA decoder with a qsv encoder or filter context, you need to make a choice between these two, which also means that you need to be able to control D3D9/11 at the QSV side.

4. OpenCL interop support is different between D3D9 and D3D11 and you might need to adjust your command line accordingly. That's only possible when you can be sure up-front about which kind of context you'll get.

5. Drivers for Gen 3, 4 and 5 are no longer getting updates for QSV. There are still driver updates being published, but the QSV/MSDK part remains unchanged. These versions have issues that Artem's patch doesn't address

Summing up: a selection mechanism between D3D9 and D3D11 is  inevitable, but it must not be automatic.
Of course - it would be possible to create a DXVA2 or D3D11VA context first and derive from that one, but that's technically not the same and too complicated for the average user.

Kind Regards,
softworkz
Max Dmitrichenko July 15, 2021, 3:15 p.m. UTC | #11
On Thu, Jul 15, 2021 at 5:49 AM Soft Works <softworkz@hotmail.com> wrote:

>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > James Almer
> > Sent: Thursday, 15 July 2021 05:21
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > d3d11va support, added mfxhdlpair
> >
> > On 7/15/2021 12:10 AM, Soft Works wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > >> Xiang, Haihao
> > >> Sent: Thursday, 15 July 2021 04:35
> > >> To: ffmpeg-devel@ffmpeg.org
> > >> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > >> d3d11va support, added mfxhdlpair
> > >>
> > >> On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > >>> On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > >>>> On Tue, Nov 3, 2020 at 7:47 PM Artem Galin <artem.galin@gmail.com>
> > >> wrote:
> > >>>>
> > >>>>> Adding DX11 relevant device type checks and adjusting callbacks
> > >>>>> with proper MediaSDK pair type support.
> > >>>>>
> > >>>>> Extending structure for proper MediaSDK pair type support.
> > >>>>>
> > >>>>> Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > >>>>
> > >>>>
> > >>>> Patchset obviously closes the gap of DirectX 11 support and already
> > >>>> checked with several apps that use FFMPEG.
> > >>>>
> > >>>> Any particular review comments should be yet expected?
> > >>>>
> > >>>> Changes seem to be straight forward and incorporate prev. comments.
> > >>>>
> > >>>> thank in advance
> > >>>>
> > >>>> regards
> > >>>> Max
> > >>>
> > >>> There are some issues pointed out by Soft Works related to switching
> > >>> the default to DX11 breaking existing command lines with DX9, plus
> > >>> an OpenCL<->QSV interop regression this would introduce that
> > >>> according to him should be easy to fix.
> > >>>
> > >>> If those are addressed, the set should be good.
> > >>
> > >> If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > >> June/281778.html
> > >> (qsvdec: add support for HW_DEVICE_CTX method) first, we should be
> > >> able to use the common device stuff to initialize d3d11va device for
> > >> QSV and needn't use child_device_type to specify child device.
> > >
> > > There's no doubt that the new method will be better, but the point is
> not
> > to break existing command lines. From my experience, ffmpeg usually
> avoids
> > to break established command line patterns, which is a good thing imo.
> > >
> > > Anything more official than my perception? Please feel free to chime
> > > in... ;-)
> >
> > That patch appears to attempt to maintain support for existing command
> > lines for a deprecation period, something we've done before with other
> > hwaccels like cuvid, so if it's really better, it's fine and acceptable.
> ^
>
> Hi James,
>
> "That patch" that preserves compatibility doesn't exist yet. We need to
> make sure that command lines using the '--child_device' parameter will
> continue to work, while Haihao's patch will be the "future way":
>
> > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va
>
> The new behaviour that is introduced by this patch is that an initialized
> hw device can now be applied to qsv decoders (currently only to filter
> graphs and qsv encoders).
>
> Additionally, the default should remain to be D3D9 (if none of the above
> is specified) as laid out earlier - again for maintaining compatibility but
> also for better robustness and performance.
>
>
Upon introduction of DX11/D3D11VA support, it makes sense to make it
default as well,
this would help for upcoming implementations and general usages in the long
run,
including new usages, as headless platforms support,
which is an obvious D3D9 gap, as an example.

Compatibility is still preserved and available with D3D9 path, there is no
functional change.
If D3D9 is critical for further usage - it has to be requested explicitly.

regards
Max
Soft Works July 15, 2021, 3:52 p.m. UTC | #12
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Max Dmitrichenko
> Sent: Thursday, 15 July 2021 17:15
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> d3d11va support, added mfxhdlpair
> 
> On Thu, Jul 15, 2021 at 5:49 AM Soft Works <softworkz@hotmail.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > James Almer
> > > Sent: Thursday, 15 July 2021 05:21
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > d3d11va support, added mfxhdlpair
> > >
> > > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> > > >> Xiang, Haihao
> > > >> Sent: Thursday, 15 July 2021 04:35
> > > >> To: ffmpeg-devel@ffmpeg.org
> > > >> Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > >> enabling d3d11va support, added mfxhdlpair
> > > >>
> > > >> On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > >>> On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > >>>> On Tue, Nov 3, 2020 at 7:47 PM Artem Galin
> > > >>>> <artem.galin@gmail.com>
> > > >> wrote:
> > > >>>>
> > > >>>>> Adding DX11 relevant device type checks and adjusting
> > > >>>>> callbacks with proper MediaSDK pair type support.
> > > >>>>>
> > > >>>>> Extending structure for proper MediaSDK pair type support.
> > > >>>>>
> > > >>>>> Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > >>>>
> > > >>>>
> > > >>>> Patchset obviously closes the gap of DirectX 11 support and
> > > >>>> already checked with several apps that use FFMPEG.
> > > >>>>
> > > >>>> Any particular review comments should be yet expected?
> > > >>>>
> > > >>>> Changes seem to be straight forward and incorporate prev.
> comments.
> > > >>>>
> > > >>>> thank in advance
> > > >>>>
> > > >>>> regards
> > > >>>> Max
> > > >>>
> > > >>> There are some issues pointed out by Soft Works related to
> > > >>> switching the default to DX11 breaking existing command lines
> > > >>> with DX9, plus an OpenCL<->QSV interop regression this would
> > > >>> introduce that according to him should be easy to fix.
> > > >>>
> > > >>> If those are addressed, the set should be good.
> > > >>
> > > >> If we may apply http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > >> June/281778.html
> > > >> (qsvdec: add support for HW_DEVICE_CTX method) first, we should
> > > >> be able to use the common device stuff to initialize d3d11va
> > > >> device for QSV and needn't use child_device_type to specify child
> device.
> > > >
> > > > There's no doubt that the new method will be better, but the point
> > > > is
> > not
> > > to break existing command lines. From my experience, ffmpeg usually
> > avoids
> > > to break established command line patterns, which is a good thing imo.
> > > >
> > > > Anything more official than my perception? Please feel free to
> > > > chime in... ;-)
> > >
> > > That patch appears to attempt to maintain support for existing
> > > command lines for a deprecation period, something we've done before
> > > with other hwaccels like cuvid, so if it's really better, it's fine and
> acceptable.
> > ^
> >
> > Hi James,
> >
> > "That patch" that preserves compatibility doesn't exist yet. We need
> > to make sure that command lines using the '--child_device' parameter
> > will continue to work, while Haihao's patch will be the "future way":
> >
> > > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device qsv@d3d11va
> >
> > The new behaviour that is introduced by this patch is that an
> > initialized hw device can now be applied to qsv decoders (currently
> > only to filter graphs and qsv encoders).
> >
> > Additionally, the default should remain to be D3D9 (if none of the
> > above is specified) as laid out earlier - again for maintaining
> > compatibility but also for better robustness and performance.
> >
> >
> Upon introduction of DX11/D3D11VA support, it makes sense to make it
> default as well, this would help for upcoming implementations and general
> usages in the long run, including new usages, as headless platforms support,
> which is an obvious D3D9 gap, as an example.
> 
> Compatibility is still preserved and available with D3D9 path, there is no
> functional change.
> If D3D9 is critical for further usage - it has to be requested explicitly.

Which is a breaking change. It would not only affect a huge amount of applications and users - it would also invalidate thousands of command parameter examples that are spread all over the web.

I'd love to hear some other voices on this (even when opinions differ). 
I don't want to be the only one opposing it each time..

softworkz
Xiang, Haihao July 16, 2021, 1:25 a.m. UTC | #13
On Thu, 2021-07-15 at 09:32 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Thursday, 15 July 2021 09:56
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > d3d11va support, added mfxhdlpair
> > 
> > On Thu, 2021-07-15 at 06:43 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Xiang, Haihao
> > > > Sent: Thursday, 15 July 2021 07:43
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv: enabling
> > > > d3d11va support, added mfxhdlpair
> > > > 
> > > > On Thu, 2021-07-15 at 03:49 +0000, Soft Works wrote:
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > 
> > Behalf
> > > > > > Of James Almer
> > > > > > Sent: Thursday, 15 July 2021 05:21
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > > enabling d3d11va support, added mfxhdlpair
> > > > > > 
> > > > > > On 7/15/2021 12:10 AM, Soft Works wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > > -----Original Message-----
> > > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > > > 
> > > > Behalf
> > > > > > > > Of Xiang, Haihao
> > > > > > > > Sent: Thursday, 15 July 2021 04:35
> > > > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 1/8] libavcodec/qsv:
> > > > > > > > enabling d3d11va support, added mfxhdlpair
> > > > > > > > 
> > > > > > > > On Tue, 2021-07-13 at 11:53 -0300, James Almer wrote:
> > > > > > > > > On 11/14/2020 1:49 PM, Max Dmitrichenko wrote:
> > > > > > > > > > On Tue, Nov 3, 2020 at 7:47 PM Artem Galin
> > > > > > > > > > <artem.galin@gmail.com>
> > > > > > > > 
> > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > > Adding DX11 relevant device type checks and adjusting
> > > > > > > > > > > callbacks with proper MediaSDK pair type support.
> > > > > > > > > > > 
> > > > > > > > > > > Extending structure for proper MediaSDK pair type support.
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Artem Galin <artem.galin@intel.com> .....
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Patchset obviously closes the gap of DirectX 11 support
> > > > > > > > > > and already checked with several apps that use FFMPEG.
> > > > > > > > > > 
> > > > > > > > > > Any particular review comments should be yet expected?
> > > > > > > > > > 
> > > > > > > > > > Changes seem to be straight forward and incorporate prev.
> > > > 
> > > > comments.
> > > > > > > > > > 
> > > > > > > > > > thank in advance
> > > > > > > > > > 
> > > > > > > > > > regards
> > > > > > > > > > Max
> > > > > > > > > 
> > > > > > > > > There are some issues pointed out by Soft Works related to
> > > > > > > > > switching the default to DX11 breaking existing command
> > > > > > > > > lines with DX9, plus an OpenCL<->QSV interop regression
> > > > > > > > > this would introduce that according to him should be easy to
> > > > > > > > > fix.
> > > > > > > > > 
> > > > > > > > > If those are addressed, the set should be good.
> > > > > > > > 
> > > > > > > > If we may apply
> > > > > > > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-
> > > > > > > > June/281778.html
> > > > > > > > (qsvdec: add support for HW_DEVICE_CTX method) first, we
> > > > > > > > should be able to use the common device stuff to initialize
> > > > > > > > d3d11va device for QSV and needn't use child_device_type to
> > > > > > > > specify child
> > > > 
> > > > device.
> > > > > > > 
> > > > > > > There's no doubt that the new method will be better, but the
> > > > > > > point is not
> > > > > > 
> > > > > > to break existing command lines. From my experience, ffmpeg
> > > > > > usually avoids to break established command line patterns, which
> > > > > > is a good thing
> > > > 
> > > > imo.
> > > > > > > 
> > > > > > > Anything more official than my perception? Please feel free to
> > > > > > > chime in... ;-)
> > > > > > 
> > > > > > That patch appears to attempt to maintain support for existing
> > > > > > command lines for a deprecation period, something we've done
> > > > > > before with other hwaccels like cuvid, so if it's really better,
> > > > > > it's fine and
> > > > 
> > > > acceptable.
> > > > > 
> > > > > ^
> > > > > 
> > > > > Hi James,
> > > > > 
> > > > > "That patch" that preserves compatibility doesn't exist yet. We
> > > > > need to make sure that command lines using the '--child_device'
> > > > > parameter will continue to work, while Haihao's patch will be the
> > 
> > "future way":
> > > > > 
> > > > > > -init_hw_device d3d11va=d3d11va:xxx -init_hw_device
> > 
> > qsv@d3d11va
> > > > > 
> > > > > The new behaviour that is introduced by this patch is that an
> > > > > initialized hw device can now be applied to qsv decoders
> > > > > (currently only to filter graphs and qsv encoders).
> > > > 
> > > > '-init_hw_device qsv=qsv:hw,child_device=xxx' still works,
> > > 
> > > Sure - that needs to work anyway. What I meant is the -qsv_device
> > > parameter, which is a global-scope param and takes a device path on
> > > Linux or the D3D adapter ID on Windows.
> > > 
> > > ffmpeg -hwaccel qsv -qsv_device /dev/dri/renderD128 -c:v h264_qsv -i
> > > input.mp4 -c:v h264_qsv output.mp4
> > 
> > Yes, -qsv_device still works, I added opt_qsv_device() to handle qsv_device
> > option.
> > 
> > > 
> > > For my implementation I had added an additional global parameter (
> > > --qsv_use_dx11). When it is set, the -qsv_device parameter indicates a
> > > DXGI adapter id rather than a D3D adapter. But let's forget about
> > > that. The init_hw_device approach is much better, and -qsv_device
> > > should just keep working like before.
> > > 
> > > I'm currently rebasing all our patches to the latest ffmpeg; probably
> > > tomorrow I'll check out your patch.
> > 
> > Thanks for checking out the patch.
> > 
> > > 
> > > > > Additionally, the default should remain to be D3D9 (if none of the
> > > > > above is
> > > > > specified) as laid out earlier - again for maintaining
> > > > > compatibility but also for better robustness and performance.
> > > > 
> > > > We may refine Artem's patch to make sure the default is D3D9 on
> > > > Microsoft windows
> > > 
> > > Sounds good.
> > > 
> > > > when both dxva2 and d3d11va are enabled if applying my patch first.
> > > 
> > > Not sure what you mean by that.
> > 
> > What I meant is we needn't use parameter child_device_type (which was
> > added in Artem's patch) to specify the child device type after applying my
> > patch, but we should make sure the default child device type is DXVA2 when
> > both DXVA2 and D3D11VA are available however user doesn't specify the
> > type. E.g. '- init_hw_device qsv=qsv:hw' should work with DXVA2, not
> > D3D11VA if both DXVA2 and D3D11VA are availabe.
> 
> Ah, thanks for the clarification, but that doesn't work out. The parameter -
> child_device_type is essential:
> 
> 1. The device numbers that you specify via qsv_device or child_device are
> totally different between D3D9 and D3D11/DXGI. In the latter case, you specify
> the index of a graphics adapter/hardware device, no matter how many displays
> you _can_ connect to it and no matter how many displays _are_ connected.
> In case of D3D9, you specify the index in a collection of connected
> displays  which belong to the context of the active user session.
> => without specifying a device type, an index number is meaningless
> 
> 2. It's important that the behavior is deterministic: when you specify D3D9,
> you need to be able to rely on getting D3D9 (or fail otherwise), same for
> D3D11. 
> 
> 3. When you want to combine a DXVA2/D3D11VA decoder with a qsv encoder or
> filter context, you need to make a choice between these two, which also means
> that you need to be able to control D3D9/11 at the QSV side.
> 
> 4. OpenCL interop support is different between D3D9 and D3D11 and you might
> need to adjust your command line accordingly. That's only possible when you
> can be sure up-front about which kind of context you'll get.
> 
> 5. Drivers for Gen 3, 4 and 5 are no longer getting updates for QSV. There are
> still driver updates being published, but the QSV/MSDK part remains unchanged.
> These versions have issues that Artem's patch doesn't address
> 
> Summing up: a selection mechanism between D3D9 and D3D11 is  inevitable, but
> it must not be automatic.
> Of course - it would be possible to create a DXVA2 or D3D11VA context first
> and derive from that one, but that's technically not the same and too
> complicated for the average user.

Thanks for explanation, I'm fine to keep child_device_type (my original thought
was to ask user to derive a QSV device from D3D11VA for D3D11VA support). 

BR
Haihao
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index 6e3154e1a3..d9d7c2cb43 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -36,6 +36,8 @@ 
 #include "avcodec.h"
 #include "qsv_internal.h"
 
+#define MFX_IMPL_VIA_MASK(impl) (0x0f00 & (impl))
+
 #if QSV_VERSION_ATLEAST(1, 12)
 #include "mfx/mfxvp8.h"
 #endif
@@ -243,7 +245,9 @@  int ff_qsv_find_surface_idx(QSVFramesContext *ctx, QSVFrame *frame)
     int i;
     for (i = 0; i < ctx->nb_mids; i++) {
         QSVMid *mid = &ctx->mids[i];
-        if (mid->handle == frame->surface.Data.MemId)
+        mfxHDLPair *pair = (mfxHDLPair*)frame->surface.Data.MemId;
+        if ((mid->handle_pair->first == pair->first) &&
+            (mid->handle_pair->second == pair->second))
             return i;
     }
     return AVERROR_BUG;
@@ -383,7 +387,11 @@  static int ff_qsv_set_display_handle(AVCodecContext *avctx, QSVSession *qs)
 int ff_qsv_init_internal_session(AVCodecContext *avctx, QSVSession *qs,
                                  const char *load_plugins, int gpu_copy)
 {
+#if CONFIG_D3D11VA
+    mfxIMPL          impl = MFX_IMPL_AUTO_ANY | MFX_IMPL_VIA_D3D11;
+#else
     mfxIMPL          impl = MFX_IMPL_AUTO_ANY;
+#endif
     mfxVersion        ver = { { QSV_VERSION_MINOR, QSV_VERSION_MAJOR } };
     mfxInitParam init_par = { MFX_IMPL_AUTO_ANY };
 
@@ -472,7 +480,7 @@  static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
 
     for (i = 0; i < nb_surfaces; i++) {
         QSVMid *mid = &mids[i];
-        mid->handle        = frames_hwctx->surfaces[i].Data.MemId;
+        mid->handle_pair   = (mfxHDLPair*)frames_hwctx->surfaces[i].Data.MemId;
         mid->hw_frames_ref = hw_frames_ref1;
     }
 
@@ -649,7 +657,7 @@  static mfxStatus qsv_frame_lock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr)
         goto fail;
 
     qsv_mid->surf.Info = hw_frames_hwctx->surfaces[0].Info;
-    qsv_mid->surf.Data.MemId = qsv_mid->handle;
+    qsv_mid->surf.Data.MemId = qsv_mid->handle_pair;
 
     /* map the data to the system memory */
     ret = av_hwframe_map(qsv_mid->locked_frame, qsv_mid->hw_frame,
@@ -682,7 +690,13 @@  static mfxStatus qsv_frame_unlock(mfxHDL pthis, mfxMemId mid, mfxFrameData *ptr)
 static mfxStatus qsv_frame_get_hdl(mfxHDL pthis, mfxMemId mid, mfxHDL *hdl)
 {
     QSVMid *qsv_mid = (QSVMid*)mid;
-    *hdl = qsv_mid->handle;
+    mfxHDLPair *pair_dst = (mfxHDLPair*)hdl;
+    mfxHDLPair *pair_src = (mfxHDLPair*)qsv_mid->handle_pair;
+
+    pair_dst->first = pair_src->first;
+
+    if (pair_src->second != (mfxMemId)MFX_INFINITE)
+        pair_dst->second = pair_src->second;
     return MFX_ERR_NONE;
 }
 
@@ -690,24 +704,19 @@  int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
                                AVBufferRef *device_ref, const char *load_plugins,
                                int gpu_copy)
 {
-    static const mfxHandleType handle_types[] = {
-        MFX_HANDLE_VA_DISPLAY,
-        MFX_HANDLE_D3D9_DEVICE_MANAGER,
-        MFX_HANDLE_D3D11_DEVICE,
-    };
     AVHWDeviceContext    *device_ctx = (AVHWDeviceContext*)device_ref->data;
     AVQSVDeviceContext *device_hwctx = device_ctx->hwctx;
     mfxSession        parent_session = device_hwctx->session;
     mfxInitParam            init_par = { MFX_IMPL_AUTO_ANY };
     mfxHDL                    handle = NULL;
+    int          hw_handle_supported = 0;
 
     mfxSession    session;
     mfxVersion    ver;
     mfxIMPL       impl;
     mfxHandleType handle_type;
     mfxStatus err;
-
-    int i, ret;
+    int ret;
 
     err = MFXQueryIMPL(parent_session, &impl);
     if (err == MFX_ERR_NONE)
@@ -716,13 +725,23 @@  int ff_qsv_init_session_device(AVCodecContext *avctx, mfxSession *psession,
         return ff_qsv_print_error(avctx, err,
                                   "Error querying the session attributes");
 
-    for (i = 0; i < FF_ARRAY_ELEMS(handle_types); i++) {
-        err = MFXVideoCORE_GetHandle(parent_session, handle_types[i], &handle);
-        if (err == MFX_ERR_NONE) {
-            handle_type = handle_types[i];
-            break;
+    if (MFX_IMPL_VIA_VAAPI == MFX_IMPL_VIA_MASK(impl)) {
+        handle_type = MFX_HANDLE_VA_DISPLAY;
+        hw_handle_supported = 1;
+    } else if (MFX_IMPL_VIA_D3D11 == MFX_IMPL_VIA_MASK(impl)) {
+        handle_type = MFX_HANDLE_D3D11_DEVICE;
+        hw_handle_supported = 1;
+    } else if (MFX_IMPL_VIA_D3D9 == MFX_IMPL_VIA_MASK(impl)) {
+        handle_type = MFX_HANDLE_D3D9_DEVICE_MANAGER;
+        hw_handle_supported = 1;
+    }
+
+    if (hw_handle_supported) {
+        err = MFXVideoCORE_GetHandle(parent_session, handle_type, &handle);
+        if (err != MFX_ERR_NONE) {
+            return ff_qsv_print_error(avctx, err,
+                                  "Error getting handle session");
         }
-        handle = NULL;
     }
     if (!handle) {
         av_log(avctx, AV_LOG_VERBOSE, "No supported hw handle could be retrieved "
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index 6b2fbbe252..10b08f0f9a 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -62,7 +62,7 @@ 
 
 typedef struct QSVMid {
     AVBufferRef *hw_frames_ref;
-    mfxHDL handle;
+    mfxHDLPair *handle_pair;
 
     AVFrame *locked_frame;
     AVFrame *hw_frame;