[FFmpeg-devel] [PATCH] ffmpeg_hw: make hardware selection for filters more user friendly

[Xiang, Haihao]

On Wed, 2021-08-04 at 05:17 +0000, Soft Works wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Haihao Xiang
> > Sent: Tuesday, 29 June 2021 03:46
> > To: ffmpeg-devel at ffmpeg.org
> > Cc: Haihao Xiang <haihao.xiang at intel.com>
> > Subject: [FFmpeg-devel] [PATCH] ffmpeg_hw: make hardware selection for
> > filters more user friendly
> > 
> > When a device is derived from a source device, there are at least 2 devices,
> > and usually the derived device is the expected device, so let's pick the
> > last
> > device if user doesn't specify the filter device with filter_hw_device
> > option
> > 
> > After applying this patch, the command below can 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
> > ---
> >  fftools/ffmpeg_hw.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c index
> > fc4a5d31d6..14e2cb0177 100644
> > --- a/fftools/ffmpeg_hw.c
> > +++ b/fftools/ffmpeg_hw.c
> > @@ -527,15 +527,21 @@ int hw_device_setup_for_filter(FilterGraph *fg)
> >      HWDevice *dev;
> >      int i;
> > 
> > -    // If the user has supplied exactly one hardware device then just
> > -    // give it straight to every filter for convenience.  If more than
> > -    // one device is available then the user needs to pick one explcitly
> > -    // with the filter_hw_device option.
> > +    // Pick the last hardware device if the user doesn't pick the device
> > for
> > +    // filters explicitly with the filter_hw_device option.
> >      if (filter_hw_device)
> >          dev = filter_hw_device;
> > -    else if (nb_hw_devices == 1)
> > -        dev = hw_devices[0];
> > -    else
> > +    else if (nb_hw_devices > 0) {
> > +        dev = hw_devices[nb_hw_devices - 1];
> > +
> > +        if (nb_hw_devices > 1)
> > +            av_log(NULL, AV_LOG_WARNING, "There are %d hardware devices.
> > device "
> > +                   "%s of type %s is picked for filters by default. Set
> > hardware "
> > +                   "device explicitly with the filter_hw_device option if
> > device "
> > +                   "%s is not usable for filters.\n",
> > +                   nb_hw_devices, dev->name,
> > +                   av_hwdevice_get_type_name(dev->type), dev->name);
> > +    } else
> >          dev = NULL;
> > 
> >      if (dev) {
> > --
> 
> Haihao,
> 
> Thanks for the patch, I hadn't seen it. Due to working off an older baseline
> version,  I wasn't actually aware of the current code which had been added by
> Mark Thompson in April 2020: 
> 
> - when a single hw device is initialized, all filters are automatically set to
> that device context
> - when more than one device is initialized, it silently does nothing
> 
> This leads to the following behavior: you have a working command line with a
> single device. Then you add initialization for a second device and the command
> line stops working.
> 
> @mark - would you mind to comment?
> I suppose the idea was that when there's more than a single device, the user
> needs to set filter_hw_device explicitly. But IMO, this doesn't go well
> together with making an automatic selection: either there's always an
> automatic selection or never, but the current behavior is confusing.
> 
> Now, that there's already an automatic selection for cases with a single
> device, the "never" option is practically off the table. For those reasons,
> Haihao's patch would LGTM.
> 

Could someone help to merge this patch if no more comment ?

Thanks
Haihao