[FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for HW_DEVICE_CTX method

Tue Aug 3 08:21:56 EEST 2021

On Mon, 2021-08-02 at 10:27 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Xiang, Haihao
> > Sent: Monday, 2 August 2021 10:25
> > To: ffmpeg-devel at ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > HW_DEVICE_CTX method
> > 
> > On Mon, 2021-08-02 at 07:11 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > > Xiang, Haihao
> > > > Sent: Monday, 2 August 2021 07:53
> > > > To: ffmpeg-devel at ffmpeg.org
> > > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > HW_DEVICE_CTX method
> > > > 
> > > > On Fri, 2021-07-30 at 08:18 +0000, Soft Works wrote:
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On
> > 
> > Behalf
> > > > > > Of Haihao Xiang
> > > > > > Sent: Thursday, 29 July 2021 09:04
> > > > > > To: ffmpeg-devel at ffmpeg.org
> > > > > > Cc: Haihao Xiang <haihao.xiang at intel.com>
> > > > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsvdec: add support for
> > > > > > HW_DEVICE_CTX method
> > > > > > 
> > > > > > This allows user set hw_device_ctx instead of hw_frames_ctx for
> > > > > > QSV decoders, hence we may remove the ad-hoc libmfx setup code
> > > > > > from FFmpeg.
> > > > > > 
> > > > > > "-hwaccel_output_format format" is applied to QSV decoders after
> > > > > > removing the ad-hoc libmfx code. To keep compatibility with old
> > > > > > commandlines, the default format is set to AV_PIX_FMT_QSV. User
> > > > > > should set "- hwaccel_output_format qsv" explicitly if
> > > > > > AV_PIX_FMT_QSV
> > > > 
> > > > is expected.
> > > > > > 
> > > > > > User may use the normal way to set device for QSV decoders now.
> > > > > > "-qsv_device device" is deprecated and will be removed from
> > 
> > FFmpeg.
> > > > > > 
> > > > > > For example:
> > > > > > 
> > > > > > $> ffmpeg -init_hw_device vaapi=va:/dev/dri/card0
> > > > > > -init_hw_device qsv=hw at va -hwaccel qsv -c:v h264_qsv -i
> > > > > > input.h264 -f null -
> > > > > 
> > > > > Hi Haihao,
> > > > > 
> > > > > Why so complicated with double device initialization?
> > > > > The regular way is this:
> > > > > 
> > > > > ffmpeg -init_hw_device
> > 
> > "qsv=dev:hw_any,child_device=/dev/dri/card0"
> > > > > -hwaccel qsv -c:v h264_qsv -i input.h264 -f null -
> > > > > 
> > > > > Or for Windows:
> > > > > 
> > > > > ffmpeg -init_hw_device "qsv=dev:hw_any,child_device=1" -hwaccel
> > > > > qsv -c:v h264_qsv -i input.h264 -f null -
> > > > 
> > > > After applying this patch, the above commands may work too, actually
> > > > a child device is created internally in this way, then a QSV device
> > > > is derived from the child device implicitly. However deriving a
> > > > device explicitly creates a connection between source device and
> > > > derived device. We may derive a new device of the source type from
> > > > the derived device.
> > > 
> > > "may work too" is not sufficient. The existing and documented method
> > > must continue to work.
> > 
> > Could you please provide an example which is broken by this patch? I don't
> > seeregression in my testing.
> 
> What I wanted to point out is that "may work too" is not a good attitude 
> towards functionality used by millions of users (most of which without 
> even knowing).
> 

Okay, I've gotten your point. I agree with you we should make FFmpeg/qsv more
user-friendly. My first step is to make QSV work with HW_DEVICE_CTX method
without breaking existing commands in this patch. Actually I have already taken 
the commands you mentioned below into account and submitted another patch (
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html)

> Besides compatibility and command line complexity, there's also a technical
> problem with your approach.
> 

> The following command line does not work:
> 
> ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd at dx -hwaccel qsv -c:v
> h264_qsv -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> ERROR: A hardware device reference is required to upload frames to

I'm aware of this issue. Without this patch, the above command and other similar
commands don't work too, so this issue is not caused by this patch. 

e.g, below is an example which doesn't use qsv decoder at all, it also doesn't
work

$> ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD128 -init_hw_device
qsv=hw at va -f lavfi -i yuvtestsrc  -vf
format=nv12,hwupload=extra_hw_frames=64 -c:v h264_qsv -y out.h264

The root cause is there is no default selection when there are multiple hw
devices, I submitted 
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html to make it worksa few weeks ago. 

> 
> It can be fixed by setting filter_hw_device:
> 
> ffmpeg -init_hw_device dxva2=dx:1 -init_hw_device qsv=qd at dx -hwaccel qsv -c:v
> h264_qsv -i INPUT -filter_hw_device qd -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv
> 
> This is not required when using the regular QSV initialization:
> 
> ffmpeg -init_hw_device qsv=qd1:hw2,child_device=1 -hwaccel qsv -c:v h264_qsv
> -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> 
> Needing to add filter_hw_device seems to be easy - but only when you know that
> you need to do that. And that's the kind of point where many users would give
> up.
> 
> 
> But now let's look at the classic initialization using qsv_device:
> (btw: I don't think that this should be deprecated at all; it's a more
> convenient way - easier to remember and not that much to type)

Currently there are a few different options which can be used to create HW
devices for QSV, many user are confused with these options. e.g. We must use
qsv_device to specify the device for QSV decoder if we want to use a non-default 
device, however we have to use init_hw_device to initialize hw device for QSV
encoder, so I prefer to reduce options and use the same way for device
initialization and selection in the future. But I'm fine to keep qsv_device
option if you think it is a better choice. 

> 
> ffmpeg -qsv_device 1 -hwaccel qsv -c:v h264_qsv -i INPUT -filter_complex
> "testsrc2=s=256x256,hwupload=extra_hw_frames=8[subs];[0:0][subs]overlay_qsv"
> -c:v h264_qsv output.mp4
> 
> This doesn't work with your patch either, and it's not possible to set the
> filter device that way.
> Though, it will work when you change your internal initialization to use the
> regular single-shot initialization.
> 
> It's not exactly a regression because it didn't work before, but that's how it
> should work (finally) thanks to this patch.

I'm aware of this issue too, and 
http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html can fix it as
well.

Yes, the above command may work if using single-shot initialization, but the
following command does not work. However it works if using double device
initialization (http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281879.html
is not needed for this case).

$> ffmpeg -qsv_device /dev/dri/renderD128 -hwaccel qsv -c:v h264_qsv -i
input.h264 -vf hwmap=derive_device=opencl,unsharp_opencl -f null -

So I don't think single-shot initialization is better than double device
initialization, however I am fine to use single-shot initialization if the above
command is not expected to work.

Thanks
Haihao