diff mbox series

[FFmpeg-devel] lavc/qsvdec: allow qsv decoders to use initialized device

Message ID 1599014660-10719-1-git-send-email-dmitry.v.rogozhkin@intel.com
State New
Headers show
Series [FFmpeg-devel] lavc/qsvdec: allow qsv decoders to use initialized device
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Rogozhkin, Dmitry V Sept. 2, 2020, 2:44 a.m. UTC
qsv decoders did not allow to use devices explicitly initialized on
the command line and actually were using default device. This starts
to cause confusion with intel discrete GPUs since in this case decoder
might run on default integrated GPU device (/dev/dri/renderD128) and
encoder on the device specified on the command line (/dev/dri/renderD129).

Example:
ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device qsv=hw@va \
  -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264

Signed-off-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 libavcodec/qsvdec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Soft Works Sept. 2, 2020, 3:32 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Dmitry Rogozhkin
> Sent: Wednesday, September 2, 2020 4:44 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> initialized device
> 
> qsv decoders did not allow to use devices explicitly initialized on the
> command line and actually were using default device. This starts to cause
> confusion with intel discrete GPUs since in this case decoder might run on
> default integrated GPU device (/dev/dri/renderD128) and encoder on the
> device specified on the command line (/dev/dri/renderD129).
> 
> Example:
> ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> qsv=hw@va \
>   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> 

Hi Dmitry,

How about this:

-init_hw_device qsv=vad:hw_any,child_device=/dev/dri/renderD128

Unfortunately I do not have one of the new Intel discrete GPUs
so I wasn't able to test, but the above parameter is working
at least in a way that it fails when the specified child_device 
is wrong (e.g. Nvidia GPU)).

Kind regards,
softworkz
Soft Works Sept. 2, 2020, 4:13 a.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Wednesday, September 2, 2020 5:33 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> initialized device
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Dmitry Rogozhkin
> > Sent: Wednesday, September 2, 2020 4:44 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> > initialized device
> >
> > qsv decoders did not allow to use devices explicitly initialized on
> > the command line and actually were using default device. This starts
> > to cause confusion with intel discrete GPUs since in this case decoder
> > might run on default integrated GPU device (/dev/dri/renderD128) and
> > encoder on the device specified on the command line
> (/dev/dri/renderD129).
> >
> > Example:
> > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> > qsv=hw@va \
> >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> >
> 
> Hi Dmitry,
> 
> How about this:
> 
> -init_hw_device qsv=vad:hw_any,child_device=/dev/dri/renderD128
> 

I apologize, I picked the wrong thing. The qsv_device parameter is what allows setting the device for a QSV decoder:

fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv -hwaccel:v:0 qsv -i INPUT ....

Kind regards,
softworkz
Soft Works Sept. 2, 2020, 4:32 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Wednesday, September 2, 2020 6:13 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> initialized device
> 
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Dmitry Rogozhkin
> > > Sent: Wednesday, September 2, 2020 4:44 AM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to
> > > use initialized device
> > >
> > > qsv decoders did not allow to use devices explicitly initialized on
> > > the command line and actually were using default device. This starts
> > > to cause confusion with intel discrete GPUs since in this case
> > > decoder might run on default integrated GPU device
> > > (/dev/dri/renderD128) and encoder on the device specified on the
> > > command line
> > (/dev/dri/renderD129).
> > >
> > > Example:
> > > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> > > qsv=hw@va \
> > >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264

> I apologize, I picked the wrong thing. The qsv_device parameter is what
> allows setting the device for a QSV decoder:
> 
> fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv -hwaccel:v:0 qsv -i
> INPUT ....
> 
> Kind regards,
> softworkz

Here's the commit where the parameter had been added: https://github.com/FFmpeg/FFmpeg/commit/1a79b8f8d2b5d26c60c237d6e585873238e46914
Rogozhkin, Dmitry V Sept. 2, 2020, 6:45 a.m. UTC | #4
On Wed, 2020-09-02 at 04:32 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Soft Works
> > Sent: Wednesday, September 2, 2020 6:13 AM
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders
> > to use
> > initialized device
> > 
> > > 
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> > > > Of
> > > > Dmitry Rogozhkin
> > > > Sent: Wednesday, September 2, 2020 4:44 AM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > > > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders
> > > > to
> > > > use initialized device
> > > > 
> > > > qsv decoders did not allow to use devices explicitly
> > > > initialized on
> > > > the command line and actually were using default device. This
> > > > starts
> > > > to cause confusion with intel discrete GPUs since in this case
> > > > decoder might run on default integrated GPU device
> > > > (/dev/dri/renderD128) and encoder on the device specified on
> > > > the
> > > > command line
> > > 
> > > (/dev/dri/renderD129).
> > > > 
> > > > Example:
> > > > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129
> > > > -init_hw_device
> > > > qsv=hw@va \
> > > >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> > I apologize, I picked the wrong thing. The qsv_device parameter is
> > what
> > allows setting the device for a QSV decoder:
> > 
> > fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv -hwaccel:v:0
> > qsv -i
> > INPUT ....
> > 
> > Kind regards,
> > softworkz
> 
> Here's the commit where the parameter had been added: 
> https://github.com/FFmpeg/FFmpeg/commit/1a79b8f8d2b5d26c60c237d6e585873238e46914

I am aware of this option.

> -qsv_device /dev/dri/renderD129
By itself this don’t work. Both decoder and encoder will run on
/dev/dri/renderD128 instead.

> -hwaccel qsv -qsv_device /dev/dri/renderD129
Adding -hwaccel helps. This works. However, to me this is non-
intuitive: why qsv_device should be used instead of hwaccel_device
while ffmpeg help gives a different hint:
    -hwaccel hwaccel name  use HW accelerated decoding
    -hwaccel_device devicename  select a device for HW acceleration
From this perspective Haihao’s patch which is currently on the mailing
list makes sense to me  it just simplifies things.

Unfortunately ffmpeg device selection options are quite confusing. One
of the problems with -qsv_device is that the following pure encoding
won’t work:

ffmpeg -hwaccel qsv -qsv_device /dev/dri/renderD129 -f rawvideo
-pix_fmt yuv420p -s:v 1920x1080 -r 30 -i a.yuv -c:v h264_qsv out.h264

Execution will happen on /dev/dri/renderD128 instead. To make it work
on 129 you will need to run something like the following:

ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device 
qsv=hw@va -f rawvideo -pix_fmt yuv420p -s:v 1920x1080 -r 30 -i a.yuv
-c:v h264_qsv out.h264

This will work. But returning back to transcoding case: there are 2
significantly different command lines which should be used for
transcoding and encoding to make things run on /dev/dri/renderD129.
This is inconvenient to handle… And additionally there is also
-filter_hw_device which also contributes to the complication. Also
there are special marks in documentation for the qsv “Unlike most other
values, this option does not enable accelerated decoding (that is used
automatically whenever a qsv decoder is selected), but accelerated
transcoding, without copying the frames into the system memory. For it
to work, both the decoder and the encoder must support QSV acceleration
and no filters must be used.”

One missing thing seems to be documentation on the scope of
-init_hw_device option applicability. This seems to be a global option,
but in the example from the commit message encoder actually takes
device from it, but decoder just ignores it and goes with default
device. Why? This does not seem to be right.

Can someone, please, shine the light on how all these device selection
options were supposed to work? /Eventually my patch tries to fix
behavior in accordance with my understanding of the supposed behavior./

> 
> _______________________________________________
> 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 Sept. 2, 2020, 8:41 a.m. UTC | #5
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Rogozhkin, Dmitry V
> Sent: Wednesday, September 2, 2020 8:45 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> initialized device
> 
> On Wed, 2020-09-02 at 04:32 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Soft Works
> > > Sent: Wednesday, September 2, 2020 6:13 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders
> > > to use initialized device
> > >
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> > > > > Of Dmitry Rogozhkin
> > > > > Sent: Wednesday, September 2, 2020 4:44 AM
> > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > > > > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders
> > > > > to use initialized device
> > > > >
> > > > > qsv decoders did not allow to use devices explicitly initialized
> > > > > on the command line and actually were using default device. This
> > > > > starts to cause confusion with intel discrete GPUs since in this
> > > > > case decoder might run on default integrated GPU device
> > > > > (/dev/dri/renderD128) and encoder on the device specified on the
> > > > > command line
> > > >
> > > > (/dev/dri/renderD129).
> > > > >
> > > > > Example:
> > > > > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129
> > > > > -init_hw_device qsv=hw@va \
> > > > >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> > > I apologize, I picked the wrong thing. The qsv_device parameter is
> > > what allows setting the device for a QSV decoder:
> > >
> > > fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv -hwaccel:v:0
> > > qsv -i INPUT ....
> > >
> > > Kind regards,
> > > softworkz
> >
> > Here's the commit where the parameter had been added:
> >
> https://github.com/FFmpeg/FFmpeg/commit/1a79b8f8d2b5d26c60c237d6e5
> 8587
> > 3238e46914
> 
> I am aware of this option.
> 
> > -qsv_device /dev/dri/renderD129
> By itself this don’t work. Both decoder and encoder will run on
> /dev/dri/renderD128 instead.
> 
> > -hwaccel qsv -qsv_device /dev/dri/renderD129
> Adding -hwaccel helps. This works. However, to me this is non-
> intuitive: why qsv_device should be used instead of hwaccel_device while
> ffmpeg help gives a different hint:
>     -hwaccel hwaccel name  use HW accelerated decoding
>     -hwaccel_device devicename  select a device for HW acceleration From this
> perspective Haihao’s patch which is currently on the mailing list makes sense
> to me  it just simplifies things.

In case of QSV, the meaning of hwaccel_device is already defined:

device selects a value in ‘MFX_IMPL_*’. Allowed values are:
auto sw  hw  auto_any hw_any hw2 hw3 hw4

From my understanding, that's the reason why the qsv_device parameter
was introduced.

> 
> Unfortunately ffmpeg device selection options are quite confusing. 

That's true without doubt. Sadly, there's not much consistency among
all hw accelerations in general. 

> This will work. But returning back to transcoding case: there are 2 significantly
> different command lines which should be used for transcoding and encoding
> to make things run on /dev/dri/renderD129.
> This is inconvenient to handle… And additionally there is also -
> filter_hw_device which also contributes to the complication. Also there are
> special marks in documentation for the qsv “Unlike most other values, this
> option does not enable accelerated decoding (that is used automatically
> whenever a qsv decoder is selected), but accelerated transcoding, without
> copying the frames into the system memory. For it to work, both the
> decoder and the encoder must support QSV acceleration and no filters must
> be used.”
> One missing thing seems to be documentation on the scope of -
> init_hw_device option applicability. This seems to be a global option, but in
> the example from the commit message encoder actually takes device from it,
> but decoder just ignores it and goes with default device. Why? This does not
> seem to be right.
> 
> Can someone, please, shine the light on how all these device selection
> options were supposed to work?

From my understanding, this has evolved somewhat like this:

Initially, it was only possible to use a D3D or VAAPI context that was  internally
created by the MSDK. And the MSDK's mechanism for this are the MSDK's enum
values (auto sw  hw  auto_any hw_any hw2 hw3 hw4).

For that reason, those values were made the possible options for device selection
In case of QSV hwaccel. Probably, the initial implementation of QSV hwaccel didn't
do much more than connecting the device context between encoder and decoder
(probably joining the encoder session to the decoder session).

I think that the documentation text you quoted was added at around that time:

> Unlike most other values, this
> option does not enable accelerated decoding (that is used automatically
> whenever a qsv decoder is selected), but accelerated transcoding, without
> copying the frames into the system memory

Later, there were requirements that couldn't be implemented by using the D3D
Or VAAPI context that MSDK creates internally. At least these two:

- Interoperability with DXVA decoders on Windows
  (requires the D3D context to be created externally)
- VPP processing (IIRC it requires or required custom surface allocation and that
  Isn't possible with a hw context created by MSDK internally)

To manually create a D3D context on Windows, you need to know the right adapter
number of the GPU. But in earlier times of the MSDK dispatcher code, there 
was no direct correlation between the hw, hw2, hw3 and hw4 values and the 
D3D9 adapter numbers, because the dispatcher was counting Intel devices only
(was changed later, but it's still not exactly reliable).

Similar for VAAPI: it wasn't reliably possible to deduct the DRI path from the 
MFX_IMPL enum value.

As such, there had to be a way for specifying a D3D9 adapter id on Windows or 
DRI node on Linux. The hwaccel_device param already had Its semantics and
It wasn't possible to union the possible values, because - guess what - sometimes
It was and even still is required to specify hw, hw2, etc. in addition to the D3D adapter 
or DRI node (I'd need to look up the cases)).

All that lead to the introduction of the 'qsv_device' parameter as a separate parameter
and it should explain why it had to be a separate parameter (and still has to be).

That's all ugly and awful and I wouldn't want to defend it.

But an approach to make this any better should be well thought out and should not 
break anything.

Kind regards,
softworkz

PS: Corrections welcome!
Rogozhkin, Dmitry V Sept. 2, 2020, 2:21 p.m. UTC | #6
On Wed, 2020-09-02 at 08:41 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Rogozhkin, Dmitry V
> > Sent: Wednesday, September 2, 2020 8:45 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders
> > to use
> > initialized device
> > 
> > On Wed, 2020-09-02 at 04:32 +0000, Soft Works wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> > > > Of
> > > > Soft Works
> > > > Sent: Wednesday, September 2, 2020 6:13 AM
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv
> > > > decoders
> > > > to use initialized device
> > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > 
> > Behalf
> > > > > > Of Dmitry Rogozhkin
> > > > > > Sent: Wednesday, September 2, 2020 4:44 AM
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv
> > > > > > decoders
> > > > > > to use initialized device
> > > > > > 
> > > > > > qsv decoders did not allow to use devices explicitly
> > > > > > initialized
> > > > > > on the command line and actually were using default device.
> > > > > > This
> > > > > > starts to cause confusion with intel discrete GPUs since in
> > > > > > this
> > > > > > case decoder might run on default integrated GPU device
> > > > > > (/dev/dri/renderD128) and encoder on the device specified
> > > > > > on the
> > > > > > command line
> > > > > 
> > > > > (/dev/dri/renderD129).
> > > > > > 
> > > > > > Example:
> > > > > > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129
> > > > > > -init_hw_device qsv=hw@va \
> > > > > >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> > > > 
> > > > I apologize, I picked the wrong thing. The qsv_device parameter
> > > > is
> > > > what allows setting the device for a QSV decoder:
> > > > 
> > > > fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv
> > > > -hwaccel:v:0
> > > > qsv -i INPUT ....
> > > > 
> > > > Kind regards,
> > > > softworkz
> > > 
> > > Here's the commit where the parameter had been added:
> > > 
> > 
> > https://github.com/FFmpeg/FFmpeg/commit/1a79b8f8d2b5d26c60c237d6e5
> > 8587
> > > 3238e46914
> > 
> > I am aware of this option.
> > 
> > > -qsv_device /dev/dri/renderD129
> > 
> > By itself this don’t work. Both decoder and encoder will run on
> > /dev/dri/renderD128 instead.
> > 
> > > -hwaccel qsv -qsv_device /dev/dri/renderD129
> > 
> > Adding -hwaccel helps. This works. However, to me this is non-
> > intuitive: why qsv_device should be used instead of hwaccel_device
> > while
> > ffmpeg help gives a different hint:
> >     -hwaccel hwaccel name  use HW accelerated decoding
> >     -hwaccel_device devicename  select a device for HW acceleration
> > From this
> > perspective Haihao’s patch which is currently on the mailing list
> > makes sense
> > to me  it just simplifies things.
> 
> In case of QSV, the meaning of hwaccel_device is already defined:
> 

Adding new values does not break functionality. I believe that
definition for hwaccel_device on Linux for qsv can be extended w/ drm
device specifiers for the convenience of overyone. As I noted in other
thread, msdk on Linux does not distinguish hw,hw2,hw3 - these are all
treated the same and library requires external device setting via
SetHandle.

> device selects a value in ‘MFX_IMPL_*’. Allowed values are:
> auto sw  hw  auto_any hw_any hw2 hw3 hw4
> 
> From my understanding, that's the reason why the qsv_device parameter
> was introduced.
> 
> > 
> > Unfortunately ffmpeg device selection options are quite confusing. 
> 
> That's true without doubt. Sadly, there's not much consistency among
> all hw accelerations in general. 
> 
> > This will work. But returning back to transcoding case: there are 2
> > significantly
> > different command lines which should be used for transcoding and
> > encoding
> > to make things run on /dev/dri/renderD129.
> > This is inconvenient to handle… And additionally there is also -
> > filter_hw_device which also contributes to the complication. Also
> > there are
> > special marks in documentation for the qsv “Unlike most other
> > values, this
> > option does not enable accelerated decoding (that is used
> > automatically
> > whenever a qsv decoder is selected), but accelerated transcoding,
> > without
> > copying the frames into the system memory. For it to work, both the
> > decoder and the encoder must support QSV acceleration and no
> > filters must
> > be used.”
> > One missing thing seems to be documentation on the scope of -
> > init_hw_device option applicability. This seems to be a global
> > option, but in
> > the example from the commit message encoder actually takes device
> > from it,
> > but decoder just ignores it and goes with default device. Why? This
> > does not
> > seem to be right.
> > 
> > Can someone, please, shine the light on how all these device
> > selection
> > options were supposed to work?
> 
> From my understanding, this has evolved somewhat like this:
> 
> Initially, it was only possible to use a D3D or VAAPI context that
> was  internally
> created by the MSDK. And the MSDK's mechanism for this are the MSDK's
> enum
> values (auto sw  hw  auto_any hw_any hw2 hw3 hw4).
Not fully right. That's correct for Windows. For Linux:
- msdk library never created VAAPI context internally, this always have
to be specified externally by application
- Basically because of that msdk on Linux never distinguished between
hw,hw2,hw3,etc. - it only was concerned about sw vs. hw.

> 
> For that reason, those values were made the possible options for
> device selection
> In case of QSV hwaccel. Probably, the initial implementation of QSV
> hwaccel didn't
> do much more than connecting the device context between encoder and
> decoder
> (probably joining the encoder session to the decoder session).
> 
> I think that the documentation text you quoted was added at around
> that time:
> 
> > Unlike most other values, this
> > option does not enable accelerated decoding (that is used
> > automatically
> > whenever a qsv decoder is selected), but accelerated transcoding,
> > without
> > copying the frames into the system memory
> 
> Later, there were requirements that couldn't be implemented by using
> the D3D
> Or VAAPI context that MSDK creates internally. At least these two:
> 
> - Interoperability with DXVA decoders on Windows
>   (requires the D3D context to be created externally)
> - VPP processing (IIRC it requires or required custom surface
> allocation and that
>   Isn't possible with a hw context created by MSDK internally)
> 
> To manually create a D3D context on Windows, you need to know the
> right adapter
> number of the GPU. But in earlier times of the MSDK dispatcher code,
> there 
> was no direct correlation between the hw, hw2, hw3 and hw4 values and
> the 
> D3D9 adapter numbers, because the dispatcher was counting Intel
> devices only
> (was changed later, but it's still not exactly reliable).
> 
> Similar for VAAPI: it wasn't reliably possible to deduct the DRI path
> from the 
> MFX_IMPL enum value.
> 
> As such, there had to be a way for specifying a D3D9 adapter id on
> Windows or 
> DRI node on Linux. The hwaccel_device param already had Its semantics
> and
> It wasn't possible to union the possible values, because - guess what
> - sometimes
> It was and even still is required to specify hw, hw2, etc. in
> addition to the D3D adapter 
> or DRI node (I'd need to look up the cases)).

I believe this still can be handled via hw_accel to everyone's
convenience. Basically, what we can do is: extend allowed set of values
w/ drm device specification on Linux, i.e.:
   device selects a value in ‘MFX_IMPL_*’. Allowed values are:
auto sw  hw  auto_any hw_any hw2 hw3 hw4 >>>or drm device path. If drm
device path is specified, 'hw' implementation is being used with the
specified drm device.<<<

This is extension to the current specification and I don't think that
it break anything.

> 
> All that lead to the introduction of the 'qsv_device' parameter as a
> separate parameter
> and it should explain why it had to be a separate parameter (and
> still has to be).

I still don't see the full picture. What I am looking for at the first
place is how maintainers and architects envision hwaccel to work in
general. Basically, there are few ways to specify the device (-hwaccel, 
-init_hw_device, -filter_hw_device), questions are:
1. What's scope of applicability of each option w/ explanation why each
of the option is actually needed (as one of the examples: why -
filter_hw_device is needed and why -init_hw_device can't be used
instead?)
2. Since there are few methods how component can get a device: what's
the priority order? (for example, if device can be deducted from
incoming frames, is there a way to override it w/ some command line
options?)

> 
> That's all ugly and awful and I wouldn't want to defend it.
> 
> But an approach to make this any better should be well thought out
> and should not 
> break anything.
> 
> Kind regards,
> softworkz
> 
> PS: Corrections welcome!
> _______________________________________________
> 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".
Rogozhkin, Dmitry V Sept. 2, 2020, 2:36 p.m. UTC | #7
On Wed, 2020-09-02 at 14:21 +0000, Rogozhkin, Dmitry V wrote:
> On Wed, 2020-09-02 at 08:41 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Rogozhkin, Dmitry V
> > > Sent: Wednesday, September 2, 2020 8:45 AM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv
> > > decoders
> > > to use
> > > initialized device
> > > 
> > > On Wed, 2020-09-02 at 04:32 +0000, Soft Works wrote:
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > > > > Behalf
> > > > > Of
> > > > > Soft Works
> > > > > Sent: Wednesday, September 2, 2020 6:13 AM
> > > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > > devel@ffmpeg.org>
> > > > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv
> > > > > decoders
> > > > > to use initialized device
> > > > > 
> > > > > > 
> > > > > > > -----Original Message-----
> > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> > > 
> > > Behalf
> > > > > > > Of Dmitry Rogozhkin
> > > > > > > Sent: Wednesday, September 2, 2020 4:44 AM
> > > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> > > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv
> > > > > > > decoders
> > > > > > > to use initialized device
> > > > > > > 
> > > > > > > qsv decoders did not allow to use devices explicitly
> > > > > > > initialized
> > > > > > > on the command line and actually were using default
> > > > > > > device.
> > > > > > > This
> > > > > > > starts to cause confusion with intel discrete GPUs since
> > > > > > > in
> > > > > > > this
> > > > > > > case decoder might run on default integrated GPU device
> > > > > > > (/dev/dri/renderD128) and encoder on the device specified
> > > > > > > on the
> > > > > > > command line
> > > > > > 
> > > > > > (/dev/dri/renderD129).
> > > > > > > 
> > > > > > > Example:
> > > > > > > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129
> > > > > > > -init_hw_device qsv=hw@va \
> > > > > > >   -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y
> > > > > > > output.h264
> > > > > 
> > > > > I apologize, I picked the wrong thing. The qsv_device
> > > > > parameter
> > > > > is
> > > > > what allows setting the device for a QSV decoder:
> > > > > 
> > > > > fmpeg -qsv_device /dev/dri/renderD128 -c:v:0 h264_qsv
> > > > > -hwaccel:v:0
> > > > > qsv -i INPUT ....
> > > > > 
> > > > > Kind regards,
> > > > > softworkz
> > > > 
> > > > Here's the commit where the parameter had been added:
> > > > 
> > > 
> > > https://github.com/FFmpeg/FFmpeg/commit/1a79b8f8d2b5d26c60c237d6e5
> > > 8587
> > > > 3238e46914
> > > 
> > > I am aware of this option.
> > > 
> > > > -qsv_device /dev/dri/renderD129
> > > 
> > > By itself this don’t work. Both decoder and encoder will run on
> > > /dev/dri/renderD128 instead.
> > > 
> > > > -hwaccel qsv -qsv_device /dev/dri/renderD129
> > > 
> > > Adding -hwaccel helps. This works. However, to me this is non-
> > > intuitive: why qsv_device should be used instead of
> > > hwaccel_device
> > > while
> > > ffmpeg help gives a different hint:
> > >     -hwaccel hwaccel name  use HW accelerated decoding
> > >     -hwaccel_device devicename  select a device for HW
> > > acceleration
> > > From this
> > > perspective Haihao’s patch which is currently on the mailing list
> > > makes sense
> > > to me  it just simplifies things.
> > 
> > In case of QSV, the meaning of hwaccel_device is already defined:
> > 
> 
> Adding new values does not break functionality. I believe that
> definition for hwaccel_device on Linux for qsv can be extended w/ drm
> device specifiers for the convenience of overyone. As I noted in
> other
> thread, msdk on Linux does not distinguish hw,hw2,hw3 - these are all
> treated the same and library requires external device setting via
> SetHandle.

Small suggestion: let's move discussion around -qsv_device and
-hwaccel_device options entirely to the "ffmpeg_qsv: use
-hwaccel_device to specify a device for VAAPI backend" thread and a
return focus on the original patch which is not about -qsv_device, but
about this command line:

ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device 
qsv=hw@va -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264

For what I see it uses valid device specifications and still in
original ffmpeg implementation it results in:
- decoder is being run on /dev/dri/renderD128
- encoder is being run on /dev/dri/renderD129

In general, per my taste, I would try to use the following device
specification working with QSV on Linux across all command lines:

-init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device qsv=hw@va
-filter_hw_device hw

My primary goal is to make it workable for all pipelines. Hence current
patch.

> 
> > device selects a value in ‘MFX_IMPL_*’. Allowed values are:
> > auto sw  hw  auto_any hw_any hw2 hw3 hw4
> > 
> > From my understanding, that's the reason why the qsv_device
> > parameter
> > was introduced.
> > 
> > > 
> > > Unfortunately ffmpeg device selection options are quite
> > > confusing. 
> > 
> > That's true without doubt. Sadly, there's not much consistency
> > among
> > all hw accelerations in general. 
> > 
> > > This will work. But returning back to transcoding case: there are
> > > 2
> > > significantly
> > > different command lines which should be used for transcoding and
> > > encoding
> > > to make things run on /dev/dri/renderD129.
> > > This is inconvenient to handle… And additionally there is also -
> > > filter_hw_device which also contributes to the complication. Also
> > > there are
> > > special marks in documentation for the qsv “Unlike most other
> > > values, this
> > > option does not enable accelerated decoding (that is used
> > > automatically
> > > whenever a qsv decoder is selected), but accelerated transcoding,
> > > without
> > > copying the frames into the system memory. For it to work, both
> > > the
> > > decoder and the encoder must support QSV acceleration and no
> > > filters must
> > > be used.”
> > > One missing thing seems to be documentation on the scope of -
> > > init_hw_device option applicability. This seems to be a global
> > > option, but in
> > > the example from the commit message encoder actually takes device
> > > from it,
> > > but decoder just ignores it and goes with default device. Why?
> > > This
> > > does not
> > > seem to be right.
> > > 
> > > Can someone, please, shine the light on how all these device
> > > selection
> > > options were supposed to work?
> > 
> > From my understanding, this has evolved somewhat like this:
> > 
> > Initially, it was only possible to use a D3D or VAAPI context that
> > was  internally
> > created by the MSDK. And the MSDK's mechanism for this are the
> > MSDK's
> > enum
> > values (auto sw  hw  auto_any hw_any hw2 hw3 hw4).
> 
> Not fully right. That's correct for Windows. For Linux:
> - msdk library never created VAAPI context internally, this always
> have
> to be specified externally by application
> - Basically because of that msdk on Linux never distinguished between
> hw,hw2,hw3,etc. - it only was concerned about sw vs. hw.
> 
> > 
> > For that reason, those values were made the possible options for
> > device selection
> > In case of QSV hwaccel. Probably, the initial implementation of QSV
> > hwaccel didn't
> > do much more than connecting the device context between encoder and
> > decoder
> > (probably joining the encoder session to the decoder session).
> > 
> > I think that the documentation text you quoted was added at around
> > that time:
> > 
> > > Unlike most other values, this
> > > option does not enable accelerated decoding (that is used
> > > automatically
> > > whenever a qsv decoder is selected), but accelerated transcoding,
> > > without
> > > copying the frames into the system memory
> > 
> > Later, there were requirements that couldn't be implemented by
> > using
> > the D3D
> > Or VAAPI context that MSDK creates internally. At least these two:
> > 
> > - Interoperability with DXVA decoders on Windows
> >   (requires the D3D context to be created externally)
> > - VPP processing (IIRC it requires or required custom surface
> > allocation and that
> >   Isn't possible with a hw context created by MSDK internally)
> > 
> > To manually create a D3D context on Windows, you need to know the
> > right adapter
> > number of the GPU. But in earlier times of the MSDK dispatcher
> > code,
> > there 
> > was no direct correlation between the hw, hw2, hw3 and hw4 values
> > and
> > the 
> > D3D9 adapter numbers, because the dispatcher was counting Intel
> > devices only
> > (was changed later, but it's still not exactly reliable).
> > 
> > Similar for VAAPI: it wasn't reliably possible to deduct the DRI
> > path
> > from the 
> > MFX_IMPL enum value.
> > 
> > As such, there had to be a way for specifying a D3D9 adapter id on
> > Windows or 
> > DRI node on Linux. The hwaccel_device param already had Its
> > semantics
> > and
> > It wasn't possible to union the possible values, because - guess
> > what
> > - sometimes
> > It was and even still is required to specify hw, hw2, etc. in
> > addition to the D3D adapter 
> > or DRI node (I'd need to look up the cases)).
> 
> I believe this still can be handled via hw_accel to everyone's
> convenience. Basically, what we can do is: extend allowed set of
> values
> w/ drm device specification on Linux, i.e.:
>    device selects a value in ‘MFX_IMPL_*’. Allowed values are:
> auto sw  hw  auto_any hw_any hw2 hw3 hw4 >>>or drm device path. If
> drm
> device path is specified, 'hw' implementation is being used with the
> specified drm device.<<<
> 
> This is extension to the current specification and I don't think that
> it break anything.
> 
> > 
> > All that lead to the introduction of the 'qsv_device' parameter as
> > a
> > separate parameter
> > and it should explain why it had to be a separate parameter (and
> > still has to be).
> 
> I still don't see the full picture. What I am looking for at the
> first
> place is how maintainers and architects envision hwaccel to work in
> general. Basically, there are few ways to specify the device (-
> hwaccel, 
> -init_hw_device, -filter_hw_device), questions are:
> 1. What's scope of applicability of each option w/ explanation why
> each
> of the option is actually needed (as one of the examples: why -
> filter_hw_device is needed and why -init_hw_device can't be used
> instead?)
> 2. Since there are few methods how component can get a device: what's
> the priority order? (for example, if device can be deducted from
> incoming frames, is there a way to override it w/ some command line
> options?)
> 
> > 
> > That's all ugly and awful and I wouldn't want to defend it.
> > 
> > But an approach to make this any better should be well thought out
> > and should not 
> > break anything.
> > 
> > Kind regards,
> > softworkz
> > 
> > PS: Corrections welcome!
> > _______________________________________________
> > 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".
> 
> _______________________________________________
> 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".
Mark Thompson Sept. 2, 2020, 7:37 p.m. UTC | #8
On 02/09/2020 15:36, Rogozhkin, Dmitry V wrote:
> On Wed, 2020-09-02 at 14:21 +0000, Rogozhkin, Dmitry V wrote:
>> On Wed, 2020-09-02 at 08:41 +0000, Soft Works wrote:
>>> ...
> 
> Small suggestion: let's move discussion around -qsv_device and
> -hwaccel_device options entirely to the "ffmpeg_qsv: use
> -hwaccel_device to specify a device for VAAPI backend" thread and a
> return focus on the original patch which is not about -qsv_device, but
> about this command line:
> 
> ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> qsv=hw@va -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264

I wouldn't recommend doing this; using the appropriate acceleration API is preferred to messing with the vendor-specific full-offload decoders:

ffmpeg -init_hw_device vaapi:/dev/dri/renderD129 -hwaccel vaapi -i input.h264 -vf hwmap=derive_device=qsv -c:v hevc_qsv -y output.h264

(Or ... -init_hw_device dxva2:1 -hwaccel dxva2 ...)

> For what I see it uses valid device specifications and still in
> original ffmpeg implementation it results in:
> - decoder is being run on /dev/dri/renderD128
> - encoder is being run on /dev/dri/renderD129

The libmfx decoders need you to set -qsv_device, which is a hack option for ffmpeg to allow selecting a device on them because they doesn't support the normal device selection.

The libmfx encoders do support the normal setup in libavcodec, so ffmpeg matched hevc_qsv to the libmfx device you created.

> In general, per my taste, I would try to use the following device
> specification working with QSV on Linux across all command lines:
> 
> -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device qsv=hw@va
> -filter_hw_device hw
> 
> My primary goal is to make it workable for all pipelines. Hence current
> patch.

Implementing AV_HW_CONFIG_METHOD_HW_DEVICE_CTX would make sense to allow the legacy decoders to use normal device selection, but your patch seems to just set the flag indicating support without actually implementing the feature?

(If you do implement it then you can delete all of the ad-hoc treatment in ffmpeg, like has been done for the other hardware codecs.)

>>> ...
>>
>> I still don't see the full picture. What I am looking for at the
>> first
>> place is how maintainers and architects envision hwaccel to work in
>> general. Basically, there are few ways to specify the device (-
>> hwaccel,
>> -init_hw_device, -filter_hw_device), questions are:
>> 1. What's scope of applicability of each option w/ explanation why
>> each
>> of the option is actually needed (as one of the examples: why -
>> filter_hw_device is needed and why -init_hw_device can't be used
>> instead?
-init_hw_device is a global option which defines a device and adds it to the list in ffmpeg; you can have as many of these as you like.

When setting up a codec which might be able to use a device it tries to match against that list, unless you have an explicit -hwaccel_device option for that input.  If you do, then it looks for the device with that name.

To maintain compatibility with old command-lines where -hwaccel_device took a string interpreted by special code for each API in ffmpeg (now removed), if it doesn't find a device with the given name then it will try to use it to create a new one.

(So "-hwaccel foo -hwaccel_device bar" when "bar" hasn't been previously defined ends up with the effect of "-init_hw_device foo=quux:bar -hwaccel foo -hwaccel_device quux".)

This doesn't apply to the libmfx decoders, because they have their own ad-hoc code.  Getting rid of that so they work in the same way as everything else would be a good thing.

-filter_hw_device selects a device from the list to use while filtering (e.g. by hwupload).

If there is exactly one device in the list when making a filter graph then it will use that, but if there are multiple then it has no way to guess which one you meant so the option is needed.

>> 2. Since there are few methods how component can get a device: what's
>> the priority order? (for example, if device can be deducted from
>> incoming frames, is there a way to override it w/ some command line
>> options?)

Devices from incoming frames have to win, because they are stored on that device - it doesn't make sense to select a different device at that point (you would need a separate download/upload or mapping step to do that).

- Mark
Soft Works Sept. 2, 2020, 9:44 p.m. UTC | #9
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Wednesday, September 2, 2020 9:38 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use
> initialized device
> 
> On 02/09/2020 15:36, Rogozhkin, Dmitry V wrote:
> > On Wed, 2020-09-02 at 14:21 +0000, Rogozhkin, Dmitry V wrote:
> >> On Wed, 2020-09-02 at 08:41 +0000, Soft Works wrote:
> >>> ...
> >
> > Small suggestion: let's move discussion around -qsv_device and
> > -hwaccel_device options entirely to the "ffmpeg_qsv: use
> > -hwaccel_device to specify a device for VAAPI backend" thread and a
> > return focus on the original patch which is not about -qsv_device, but
> > about this command line:
> >
> > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> > qsv=hw@va -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> 
> I wouldn't recommend doing this; using the appropriate acceleration API is
> preferred to messing with the vendor-specific full-offload decoders:
> 
> ffmpeg -init_hw_device vaapi:/dev/dri/renderD129 -hwaccel vaapi -i
> input.h264 -vf hwmap=derive_device=qsv -c:v hevc_qsv -y output.h264
> 
> (Or ... -init_hw_device dxva2:1 -hwaccel dxva2 ...)

Hi Mark,

In the latter case I have to disagree. We've gone through so many 
Tests and the QSV decoders have almost always resulted in better 
overall performance compared to their DXVA2 and D3D11VA counterparts,
when using a full hw pipeline qsv_dec > qsv_vpp > qsv_enc.

Also, capabilities may differ (supported color formats, resolutions,
profiles..)

Kind regards,
softworkz
Haihao Xiang Sept. 3, 2020, 12:42 a.m. UTC | #10
On Wed, 2020-09-02 at 20:37 +0100, Mark Thompson wrote:
> On 02/09/2020 15:36, Rogozhkin, Dmitry V wrote:
> > On Wed, 2020-09-02 at 14:21 +0000, Rogozhkin, Dmitry V wrote:
> > > On Wed, 2020-09-02 at 08:41 +0000, Soft Works wrote:
> > > > ...
> > 
> > Small suggestion: let's move discussion around -qsv_device and
> > -hwaccel_device options entirely to the "ffmpeg_qsv: use
> > -hwaccel_device to specify a device for VAAPI backend" thread and a
> > return focus on the original patch which is not about -qsv_device, but
> > about this command line:
> > 
> > ffmpeg -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device
> > qsv=hw@va -c:v h264_qsv -i input.h264 -c:v hevc_qsv -y output.h264
> 
> I wouldn't recommend doing this; using the appropriate acceleration API is
> preferred to messing with the vendor-specific full-offload decoders:
> 
> ffmpeg -init_hw_device vaapi:/dev/dri/renderD129 -hwaccel vaapi -i input.h264
> -vf hwmap=derive_device=qsv -c:v hevc_qsv -y output.h264
> 
> (Or ... -init_hw_device dxva2:1 -hwaccel dxva2 ...)
> 
> > For what I see it uses valid device specifications and still in
> > original ffmpeg implementation it results in:
> > - decoder is being run on /dev/dri/renderD128
> > - encoder is being run on /dev/dri/renderD129
> 
> The libmfx decoders need you to set -qsv_device, which is a hack option for
> ffmpeg to allow selecting a device on them because they doesn't support the
> normal device selection.
> 

How about to extend -hwaccel_device to accept drm path for QSV on Linux in
another thread? According to https://trac.ffmpeg.org/ticket/7649, -qsv_device
was added to workaround device select issue on Linux. We may deprecate
-qsv_device once we allow -hwaccel_device to accept drm path.

> The libmfx encoders do support the normal setup in libavcodec, so ffmpeg
> matched hevc_qsv to the libmfx device you created.
> 
> > In general, per my taste, I would try to use the following device
> > specification working with QSV on Linux across all command lines:
> > 
> > -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device qsv=hw@va
> > -filter_hw_device hw
> > 
> > My primary goal is to make it workable for all pipelines. Hence current
> > patch.
> 
> Implementing AV_HW_CONFIG_METHOD_HW_DEVICE_CTX would make sense to allow the
> legacy decoders to use normal device selection, but your patch seems to just
> set the flag indicating support without actually implementing the feature?
> 
> (If you do implement it then you can delete all of the ad-hoc treatment in
> ffmpeg, like has been done for the other hardware codecs.)

AVCodecContext.hw_device_ctx is set in ffmpeg_hw.c as soon as
AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX is set in AVCodecHWConfig. And
AVCodecContext.hw_device_ctx has been taken into account in the qsv path.

Thanks
Haihao

> 
> > > > ...
> > > 
> > > I still don't see the full picture. What I am looking for at the
> > > first
> > > place is how maintainers and architects envision hwaccel to work in
> > > general. Basically, there are few ways to specify the device (-
> > > hwaccel,
> > > -init_hw_device, -filter_hw_device), questions are:
> > > 1. What's scope of applicability of each option w/ explanation why
> > > each
> > > of the option is actually needed (as one of the examples: why -
> > > filter_hw_device is needed and why -init_hw_device can't be used
> > > instead?
> 
> -init_hw_device is a global option which defines a device and adds it to the
> list in ffmpeg; you can have as many of these as you like.
> 
> When setting up a codec which might be able to use a device it tries to match
> against that list, unless you have an explicit -hwaccel_device option for that
> input.  If you do, then it looks for the device with that name.
> 
> To maintain compatibility with old command-lines where -hwaccel_device took a
> string interpreted by special code for each API in ffmpeg (now removed), if it
> doesn't find a device with the given name then it will try to use it to create
> a new one.
> 
> (So "-hwaccel foo -hwaccel_device bar" when "bar" hasn't been previously
> defined ends up with the effect of "-init_hw_device foo=quux:bar -hwaccel foo
> -hwaccel_device quux".)
> 
> This doesn't apply to the libmfx decoders, because they have their own ad-hoc
> code.  Getting rid of that so they work in the same way as everything else
> would be a good thing.
> 
> -filter_hw_device selects a device from the list to use while filtering (e.g.
> by hwupload).
> 
> If there is exactly one device in the list when making a filter graph then it
> will use that, but if there are multiple then it has no way to guess which one
> you meant so the option is needed.
> 
> > > 2. Since there are few methods how component can get a device: what's
> > > the priority order? (for example, if device can be deducted from
> > > incoming frames, is there a way to override it w/ some command line
> > > options?)
> 
> Devices from incoming frames have to win, because they are stored on that
> device - it doesn't make sense to select a different device at that point (you
> would need a separate download/upload or mapping step to do that).
> 
> - Mark
> _______________________________________________
> 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".
Rogozhkin, Dmitry V Sept. 3, 2020, 1:02 a.m. UTC | #11
On Wed, 2020-09-02 at 20:37 +0100, Mark Thompson wrote:
> > In general, per my taste, I would try to use the following device
> > specification working with QSV on Linux across all command lines:
> > 
> > -init_hw_device vaapi=va:/dev/dri/renderD129 -init_hw_device 
> > qsv=hw@va
> > -filter_hw_device hw
> > 
> > My primary goal is to make it workable for all pipelines. Hence
> > current
> > patch.
> 
> Implementing AV_HW_CONFIG_METHOD_HW_DEVICE_CTX would make sense to
> allow the legacy decoders to use normal device selection, but your
> patch seems to just set the flag indicating support without actually
> implementing the feature?

Because feature just worked as soon as I added flag indicating the
support. For what I see in the code, implementation is already there,
it just was not properly enabled. Here it is:
https://github.com/FFmpeg/FFmpeg/blob/7b1ed4b53a4b32865dd9990f555929e803f64b64/libavcodec/qsvdec.c#L130. It seems you Mark was original author of this piece of code, but
later it was patched by Zhong. Can you folks try to evaluate whether
current implementation is enough? We've give it a try via our CI
regression tests which all pass (Linux only). This gives some assurity,
but still.

> 
> (If you do implement it then you can delete all of the ad-hoc
> treatment in ffmpeg, like has been done for the other hardware
> codecs.)

I like deleting code:). Ok, this sounds good. Let's try to understand
what might be missing in the current implementation since I honestly
don't see any gaps - it just works.
Rogozhkin, Dmitry V Sept. 3, 2020, 1:12 a.m. UTC | #12
On Wed, 2020-09-02 at 20:37 +0100, Mark Thompson wrote:
> > > I still don't see the full picture. What I am looking for at the
> > > first
> > > place is how maintainers and architects envision hwaccel to work
> > > in
> > > general. Basically, there are few ways to specify the device (-
> > > hwaccel,
> > > -init_hw_device, -filter_hw_device), questions are:
> > > 1. What's scope of applicability of each option w/ explanation
> > > why
> > > each
> > > of the option is actually needed (as one of the examples: why -
> > > filter_hw_device is needed and why -init_hw_device can't be used
> > > instead?
> 
> -init_hw_device is a global option which defines a device and adds it
> to the list in ffmpeg; you can have as many of these as you like.
> 
> When setting up a codec which might be able to use a device it tries
> to match against that list, unless you have an explicit
> -hwaccel_device option for that input.  If you do, then it looks for
> the device with that name.
> 
> To maintain compatibility with old command-lines where
> -hwaccel_device took a string interpreted by special code for each
> API in ffmpeg (now removed), if it doesn't find a device with the
> given name then it will try to use it to create a new one.
> 
> (So "-hwaccel foo -hwaccel_device bar" when "bar" hasn't been
> previously defined ends up with the effect of "-init_hw_device
> foo=quux:bar -hwaccel foo -hwaccel_device quux".)
> 
> This doesn't apply to the libmfx decoders, because they have their
> own ad-hoc code.  Getting rid of that so they work in the same way as
> everything else would be a good thing.
> 
> -filter_hw_device selects a device from the list to use while
> filtering (e.g. by hwupload).
> 
> If there is exactly one device in the list when making a filter graph
> then it will use that, but if there are multiple then it has no way
> to guess which one you meant so the option is needed.

Hm. I am pretty sure I saw the case when for one of qsv pipelines when
I was forced to add -filter_hw_device to make things workable thought
there was only one device specified. That's why I was specially asking
about the filter option. This was long ago however and I don't remember
what exactly I ran. Besides, this might have messed up with the fact
that qsv decoder don't work with -init_hw_device... I think I need to
pay attention on this long past use case after we will deal with the
decoder story... If I will note some strange behavior, I'll post
update.

> > > 2. Since there are few methods how component can get a device:
> > > what's
> > > the priority order? (for example, if device can be deducted from
> > > incoming frames, is there a way to override it w/ some command
> > > line
> > > options?)
> 
> Devices from incoming frames have to win, because they are stored on
> that device - it doesn't make sense to select a different device at
> that point (you would need a separate download/upload or mapping step
> to do that).
> 
> - Mark

Thank you, Mark. Your description greatly helps and aligns with
expectations I have.

>
Rogozhkin, Dmitry V Sept. 4, 2020, 6:24 p.m. UTC | #13
On Thu, 2020-09-03 at 01:02 +0000, Rogozhkin, Dmitry V wrote:
> > 
> > (If you do implement it then you can delete all of the ad-hoc
> > treatment in ffmpeg, like has been done for the other hardware
> > codecs.)
> 
> I like deleting code:). Ok, this sounds good. Let's try to understand
> what might be missing in the current implementation since I honestly
> don't see any gaps - it just works.

@Mark. We did internal review and believe that DEVICE_CTX path is
actually ready to be used and just needs to be activated. Can you,
please, let me know how you would like to proceed:
1. We can either consider review and apply the fix first (this patch)
then deal with ad_hoc in non-related patch series
2. Or we can go with the bigger patch series right away and address
both device_ctx + ad_hoc

2nd variant might require longer time to verify and review which would
hold the fix. What are your thoughts?
Mark Thompson Sept. 6, 2020, 2:25 p.m. UTC | #14
On 04/09/2020 19:24, Rogozhkin, Dmitry V wrote:
> On Thu, 2020-09-03 at 01:02 +0000, Rogozhkin, Dmitry V wrote:
>>>
>>> (If you do implement it then you can delete all of the ad-hoc
>>> treatment in ffmpeg, like has been done for the other hardware
>>> codecs.)
>>
>> I like deleting code:). Ok, this sounds good. Let's try to understand
>> what might be missing in the current implementation since I honestly
>> don't see any gaps - it just works.
> 
> @Mark. We did internal review and believe that DEVICE_CTX path is
> actually ready to be used and just needs to be activated. Can you,
> please, let me know how you would like to proceed:
> 1. We can either consider review and apply the fix first (this patch)
> then deal with ad_hoc in non-related patch series
> 2. Or we can go with the bigger patch series right away and address
> both device_ctx + ad_hoc
> 
> 2nd variant might require longer time to verify and review which would
> hold the fix. What are your thoughts?

I'm not sure what you have tested, because it definitely doesn't work.

If you return the hardware surface format from get_format() with METHOD_HW_DEVICE_CTX then it just ignores you and gives you software frames anyway, because it only supports that case with METHOD_HW_FRAMES_CTX.

For example, with below patch to test it in the hw_decode example:

$ doc/examples/hw_decode qsv test.264 /dev/null
Assertion frame->format == AV_PIX_FMT_QSV failed at src/doc/examples/hw_decode.c:108
Aborted

(It incorrectly returned an NV12 frame.)

- Mark


diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
index 71be6e6709..45f401a179 100644
--- a/doc/examples/hw_decode.c
+++ b/doc/examples/hw_decode.c
@@ -105,6 +105,8 @@ static int decode_write(AVCodecContext *avctx, AVPacket *packet)
              goto fail;
          }

+        av_assert0(frame->format == AV_PIX_FMT_QSV);
+
          if (frame->format == hw_pix_fmt) {
              /* retrieve data from GPU to CPU */
              if ((ret = av_hwframe_transfer_data(sw_frame, frame, 0)) < 0) {
@@ -191,6 +193,8 @@ int main(int argc, char *argv[])
      }
      video_stream = ret;

+    decoder = avcodec_find_decoder_by_name("h264_qsv");
+
      for (i = 0;; i++) {
          const AVCodecHWConfig *config = avcodec_get_hw_config(decoder, i);
          if (!config) {
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index fc25dc73e5..f2fac17354 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -47,7 +47,8 @@ const AVCodecHWConfigInternal *ff_qsv_hw_configs[] = {
      &(const AVCodecHWConfigInternal) {
          .public = {
              .pix_fmt     = AV_PIX_FMT_QSV,
-            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
+            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX |
+                           AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
                             AV_CODEC_HW_CONFIG_METHOD_AD_HOC,
              .device_type = AV_HWDEVICE_TYPE_QSV,
          },
Rogozhkin, Dmitry V Sept. 8, 2020, 5:51 p.m. UTC | #15
On Sun, 2020-09-06 at 15:25 +0100, Mark Thompson wrote:
> On 04/09/2020 19:24, Rogozhkin, Dmitry V wrote:
> > On Thu, 2020-09-03 at 01:02 +0000, Rogozhkin, Dmitry V wrote:
> > > > 
> > > > (If you do implement it then you can delete all of the ad-hoc
> > > > treatment in ffmpeg, like has been done for the other hardware
> > > > codecs.)
> > > 
> > > I like deleting code:). Ok, this sounds good. Let's try to
> > > understand
> > > what might be missing in the current implementation since I
> > > honestly
> > > don't see any gaps - it just works.
> > 
> > @Mark. We did internal review and believe that DEVICE_CTX path is
> > actually ready to be used and just needs to be activated. Can you,
> > please, let me know how you would like to proceed:
> > 1. We can either consider review and apply the fix first (this
> > patch)
> > then deal with ad_hoc in non-related patch series
> > 2. Or we can go with the bigger patch series right away and address
> > both device_ctx + ad_hoc
> > 
> > 2nd variant might require longer time to verify and review which
> > would
> > hold the fix. What are your thoughts?
> 
> I'm not sure what you have tested, because it definitely doesn't
> work.

I was testing transcode and indeed I overlooked that it falls into
using system memory instead of HW frames. My fault.

> 
> If you return the hardware surface format from get_format() with
> METHOD_HW_DEVICE_CTX then it just ignores you and gives you software
> frames anyway, because it only supports that case with
> METHOD_HW_FRAMES_CTX.

Can you, please, guide me towards what should be implemented/fixed in
qsv path? I afraid that implementing this fully on my own is beyond of
my current knowledge of qsv path in ffmpeg. Some guidance would be
appreciated. Maybe there is implementation for some other accel type I
can refer to?

> 
> For example, with below patch to test it in the hw_decode example:
> 
> $ doc/examples/hw_decode qsv test.264 /dev/null
> Assertion frame->format == AV_PIX_FMT_QSV failed at
> src/doc/examples/hw_decode.c:108
> Aborted
> 
> (It incorrectly returned an NV12 frame.)
> 
> - Mark
> 
> 
> diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
> index 71be6e6709..45f401a179 100644
> --- a/doc/examples/hw_decode.c
> +++ b/doc/examples/hw_decode.c
> @@ -105,6 +105,8 @@ static int decode_write(AVCodecContext *avctx,
> AVPacket *packet)
>               goto fail;
>           }
> 
> +        av_assert0(frame->format == AV_PIX_FMT_QSV);
> +
>           if (frame->format == hw_pix_fmt) {
>               /* retrieve data from GPU to CPU */
>               if ((ret = av_hwframe_transfer_data(sw_frame, frame,
> 0)) < 0) {
> @@ -191,6 +193,8 @@ int main(int argc, char *argv[])
>       }
>       video_stream = ret;
> 
> +    decoder = avcodec_find_decoder_by_name("h264_qsv");
> +
>       for (i = 0;; i++) {
>           const AVCodecHWConfig *config =
> avcodec_get_hw_config(decoder, i);
>           if (!config) {
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
> index fc25dc73e5..f2fac17354 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -47,7 +47,8 @@ const AVCodecHWConfigInternal *ff_qsv_hw_configs[]
> = {
>       &(const AVCodecHWConfigInternal) {
>           .public = {
>               .pix_fmt     = AV_PIX_FMT_QSV,
> -            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
> +            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX |
> +                           AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
>                              AV_CODEC_HW_CONFIG_METHOD_AD_HOC,
>               .device_type = AV_HWDEVICE_TYPE_QSV,
>           },
> _______________________________________________
> 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/qsvdec.c b/libavcodec/qsvdec.c
index fc25dc7..f2fac17 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -47,7 +47,8 @@  const AVCodecHWConfigInternal *ff_qsv_hw_configs[] = {
     &(const AVCodecHWConfigInternal) {
         .public = {
             .pix_fmt     = AV_PIX_FMT_QSV,
-            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
+            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX |
+                           AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
                            AV_CODEC_HW_CONFIG_METHOD_AD_HOC,
             .device_type = AV_HWDEVICE_TYPE_QSV,
         },