[FFmpeg-devel,08/13] ffmpeg: Use codec hardware config to configure hwaccels

Removes specific support for all hwaccels supported by the generic code
(DXVA2, D3D11VA, NVDEC, VAAPI, VDPAU and videotoolbox).
---
 fftools/ffmpeg.c     |  77 +++++++++++-----
 fftools/ffmpeg.h     |  10 +--
 fftools/ffmpeg_hw.c  | 244 +++++++++++++++++++++++++++++++++++----------------
 fftools/ffmpeg_opt.c |  55 ++++++------
 4 files changed, 250 insertions(+), 136 deletions(-)

On Sat, 18 Nov 2017 18:47:08 +0000
Mark Thompson <sw@jkqxz.net> wrote:

> Removes specific support for all hwaccels supported by the generic
> code (DXVA2, D3D11VA, NVDEC, VAAPI, VDPAU and videotoolbox).
> ---
>  fftools/ffmpeg.c     |  77 +++++++++++-----
>  fftools/ffmpeg.h     |  10 +--
>  fftools/ffmpeg_hw.c  | 244
> +++++++++++++++++++++++++++++++++++----------------
> fftools/ffmpeg_opt.c |  55 ++++++------ 4 files changed, 250
> insertions(+), 136 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index babd85f7bc..acff815e74 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2782,45 +2782,77 @@ fail:
>      av_freep(&avc);
>  }
>  
> -static const HWAccel *get_hwaccel(enum AVPixelFormat pix_fmt, enum
> HWAccelID selected_hwaccel_id) -{
> -    int i;
> -    for (i = 0; hwaccels[i].name; i++)
> -        if (hwaccels[i].pix_fmt == pix_fmt &&
> -            (!selected_hwaccel_id || selected_hwaccel_id ==
> HWACCEL_AUTO || hwaccels[i].id == selected_hwaccel_id))
> -            return &hwaccels[i];
> -    return NULL;
> -}
> -
>  static enum AVPixelFormat get_format(AVCodecContext *s, const enum
> AVPixelFormat *pix_fmts) {
>      InputStream *ist = s->opaque;
>      const enum AVPixelFormat *p;
>      int ret;
>  
> -    for (p = pix_fmts; *p != -1; p++) {
> +    for (p = pix_fmts; *p != AV_PIX_FMT_NONE; p++) {
>          const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(*p);
> -        const HWAccel *hwaccel;
> +        const AVCodecHWConfig  *config = NULL;
> +        int i;
>  
>          if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
>              break;
>  
> -        hwaccel = get_hwaccel(*p, ist->hwaccel_id);
> -        if (!hwaccel ||
> -            (ist->active_hwaccel_id && ist->active_hwaccel_id !=
> hwaccel->id) ||
> -            (ist->hwaccel_id != HWACCEL_AUTO && ist->hwaccel_id !=
> hwaccel->id))
> -            continue;
> +        if (ist->hwaccel_id == HWACCEL_GENERIC ||
> +            ist->hwaccel_id == HWACCEL_AUTO) {
> +            for (i = 0;; i++) {
> +                config = avcodec_get_hw_config(s->codec, i);
> +                if (!config)
> +                    break;
> +                if (!(config->methods &
> +                      AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
> +                    continue;

Just to be explicit, only METHOD_HW_DEVICE_CTX hwaccels can be
generically initialized, so that's why you exclude any other method?

> +                if (config->pix_fmt == *p)
> +                    break;
> +            }
> +        }
> +        if (config) {
> +            if (config->device_type != ist->hwaccel_device_type) {
> +                // Different hwaccel offered, ignore.
> +                continue;
> +            }
>  
> -        ret = hwaccel->init(s);
> -        if (ret < 0) {
> -            if (ist->hwaccel_id == hwaccel->id) {
> +            ret = hwaccel_decode_init(s);
> +            if (ret < 0) {
> +                if (ist->hwaccel_id == HWACCEL_GENERIC) {
> +                    av_log(NULL, AV_LOG_FATAL,
> +                           "%s hwaccel requested for input stream
> #%d:%d, "
> +                           "but cannot be initialized.\n",
> +
> av_hwdevice_get_type_name(config->device_type),
> +                           ist->file_index, ist->st->index);
> +                    return AV_PIX_FMT_NONE;
> +                }
> +                continue;
> +            }
> +        } else {
> +            const HWAccel *hwaccel = NULL;
> +            int i;
> +            for (i = 0; hwaccels[i].name; i++) {
> +                if (hwaccels[i].pix_fmt == *p) {
> +                    hwaccel = &hwaccels[i];
> +                    break;

This can also overrun the NULL terminator right? Or are we lucky
because 'name' is at offset zero inside the struct and there's no
dereference taking place.

I see this pattern has been used previously so it obviously works out.

> +                }
> +            }
> +            if (!hwaccel) {
> +                // No hwaccel supporting this pixfmt.
> +                continue;
> +            }
> +            if (hwaccel->id != ist->hwaccel_id) {
> +                // Does not match requested hwaccel.
> +                continue;
> +            }
> +
> +            ret = hwaccel->init(s);
> +            if (ret < 0) {
>                  av_log(NULL, AV_LOG_FATAL,
>                         "%s hwaccel requested for input stream
> #%d:%d, " "but cannot be initialized.\n", hwaccel->name,
>                         ist->file_index, ist->st->index);
>                  return AV_PIX_FMT_NONE;
>              }
> -            continue;
>          }
>  
>          if (ist->hw_frames_ctx) {
> @@ -2829,8 +2861,7 @@ static enum AVPixelFormat
> get_format(AVCodecContext *s, const enum AVPixelFormat return
> AV_PIX_FMT_NONE; }
>  
> -        ist->active_hwaccel_id = hwaccel->id;
> -        ist->hwaccel_pix_fmt   = *p;
> +        ist->hwaccel_pix_fmt = *p;
>          break;
>      }
>  
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index e0977e1bf1..8bb5bae862 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -61,14 +61,9 @@
>  enum HWAccelID {
>      HWACCEL_NONE = 0,
>      HWACCEL_AUTO,
> -    HWACCEL_VDPAU,
> -    HWACCEL_DXVA2,
> -    HWACCEL_VIDEOTOOLBOX,
> +    HWACCEL_GENERIC,
>      HWACCEL_QSV,
> -    HWACCEL_VAAPI,
>      HWACCEL_CUVID,
> -    HWACCEL_D3D11VA,
> -    HWACCEL_NVDEC,
>  };
>  
>  typedef struct HWAccel {
> @@ -76,7 +71,6 @@ typedef struct HWAccel {
>      int (*init)(AVCodecContext *s);
>      enum HWAccelID id;
>      enum AVPixelFormat pix_fmt;
> -    enum AVHWDeviceType device_type;
>  } HWAccel;
>  
>  typedef struct HWDevice {
> @@ -370,11 +364,11 @@ typedef struct InputStream {
>  
>      /* hwaccel options */
>      enum HWAccelID hwaccel_id;
> +    enum AVHWDeviceType hwaccel_device_type;
>      char  *hwaccel_device;
>      enum AVPixelFormat hwaccel_output_format;
>  
>      /* hwaccel context */
> -    enum HWAccelID active_hwaccel_id;
>      void  *hwaccel_ctx;
>      void (*hwaccel_uninit)(AVCodecContext *s);
>      int  (*hwaccel_get_buffer)(AVCodecContext *s, AVFrame *frame,
> int flags); diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
> index a4d1cada59..2ec1813854 100644
> --- a/fftools/ffmpeg_hw.c
> +++ b/fftools/ffmpeg_hw.c
> @@ -64,6 +64,31 @@ static HWDevice *hw_device_add(void)
>      return hw_devices[nb_hw_devices++];
>  }
>  
> +static char *hw_device_default_name(enum AVHWDeviceType type)
> +{
> +    // Make an automatic name of the form "type%d".  We arbitrarily
> +    // limit at 1000 anonymous devices of the same type - there is
> +    // probably something else very wrong if you get to this limit.
> +    const char *type_name = av_hwdevice_get_type_name(type);
> +    char *name;
> +    size_t index_pos;
> +    int index, index_limit = 1000;
> +    index_pos = strlen(type_name);
> +    name = av_malloc(index_pos + 4);
> +    if (!name)
> +        return NULL;
> +    for (index = 0; index < index_limit; index++) {
> +        snprintf(name, index_pos + 4, "%s%d", type_name, index);
> +        if (!hw_device_get_by_name(name))
> +            break;
> +    }
> +    if (index >= index_limit) {
> +        av_freep(&name);
> +        return NULL;
> +    }
> +    return name;
> +}
> +
>  int hw_device_init_from_string(const char *arg, HWDevice **dev_out)
>  {
>      // "type=name:device,key=value,key2=value2"
> @@ -111,27 +136,11 @@ int hw_device_init_from_string(const char *arg,
> HWDevice **dev_out) 
>          p += 1 + k;
>      } else {
> -        // Give the device an automatic name of the form "type%d".
> -        // We arbitrarily limit at 1000 anonymous devices of the same
> -        // type - there is probably something else very wrong if you
> -        // get to this limit.
> -        size_t index_pos;
> -        int index, index_limit = 1000;
> -        index_pos = strlen(type_name);
> -        name = av_malloc(index_pos + 4);
> +        name = hw_device_default_name(type);
>          if (!name) {
>              err = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        for (index = 0; index < index_limit; index++) {
> -            snprintf(name, index_pos + 4, "%s%d", type_name, index);
> -            if (!hw_device_get_by_name(name))
> -                break;
> -        }
> -        if (index >= index_limit) {
> -            errmsg = "too many devices";
> -            goto invalid;
> -        }
>      }
>  
>      if (!*p) {
> @@ -214,6 +223,49 @@ fail:
>      goto done;
>  }
>  
> +static int hw_device_init_from_type(enum AVHWDeviceType type,
> +                                    const char *device,
> +                                    HWDevice **dev_out)
> +{
> +    AVBufferRef *device_ref = NULL;
> +    HWDevice *dev;
> +    char *name;
> +    int err;
> +
> +    name = hw_device_default_name(type);
> +    if (!name) {
> +        err = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    err = av_hwdevice_ctx_create(&device_ref, type, device, NULL, 0);
> +    if (err < 0) {
> +        av_log(NULL, AV_LOG_ERROR,
> +               "Device creation failed: %d.\n", err);
> +        goto fail;
> +    }
> +
> +    dev = hw_device_add();
> +    if (!dev) {
> +        err = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    dev->name = name;
> +    dev->type = type;
> +    dev->device_ref = device_ref;
> +
> +    if (dev_out)
> +        *dev_out = dev;
> +
> +    return 0;
> +
> +fail:
> +    av_freep(&name);
> +    av_buffer_unref(&device_ref);
> +    return err;
> +}
> +
>  void hw_device_free_all(void)
>  {
>      int i;
> @@ -226,80 +278,130 @@ void hw_device_free_all(void)
>      nb_hw_devices = 0;
>  }
>  
> -static enum AVHWDeviceType hw_device_match_type_by_hwaccel(enum
> HWAccelID hwaccel_id) +static HWDevice
> *hw_device_match_by_codec(const AVCodec *codec) {
> +    const AVCodecHWConfig *config;
> +    HWDevice *dev;
>      int i;
> -    if (hwaccel_id == HWACCEL_NONE)
> -        return AV_HWDEVICE_TYPE_NONE;
> -    for (i = 0; hwaccels[i].name; i++) {
> -        if (hwaccels[i].id == hwaccel_id)
> -            return hwaccels[i].device_type;
> +    for (i = 0;; i++) {
> +        config = avcodec_get_hw_config(codec, i);
> +        if (!config)
> +            return NULL;
> +        if (!(config->methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
> +            continue;
> +        dev = hw_device_get_by_type(config->device_type);
> +        if (dev)
> +            return dev;
>      }
> -    return AV_HWDEVICE_TYPE_NONE;
> -}
> -
> -static enum AVHWDeviceType hw_device_match_type_in_name(const char
> *codec_name) -{
> -    const char *type_name;
> -    enum AVHWDeviceType type;
> -    for (type = av_hwdevice_iterate_types(AV_HWDEVICE_TYPE_NONE);
> -         type != AV_HWDEVICE_TYPE_NONE;
> -         type = av_hwdevice_iterate_types(type)) {
> -        type_name = av_hwdevice_get_type_name(type);
> -        if (strstr(codec_name, type_name))
> -            return type;
> -    }
> -    return AV_HWDEVICE_TYPE_NONE;
>  }
>  
>  int hw_device_setup_for_decode(InputStream *ist)
>  {
> +    const AVCodecHWConfig *config;
>      enum AVHWDeviceType type;
> -    HWDevice *dev;
> -    int err;
> +    HWDevice *dev = NULL;
> +    int err, auto_device = 0;
>  
>      if (ist->hwaccel_device) {
>          dev = hw_device_get_by_name(ist->hwaccel_device);
>          if (!dev) {
> -            char *tmp;
> -            type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
> -            if (type == AV_HWDEVICE_TYPE_NONE) {
> -                // No match - this isn't necessarily invalid, though,
> -                // because an explicit device might not be needed or
> -                // the hwaccel setup could be handled elsewhere.
> +            if (ist->hwaccel_id == HWACCEL_AUTO) {
> +                auto_device = 1;
> +            } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
> +                type = ist->hwaccel_device_type;
> +                err = hw_device_init_from_type(type,
> ist->hwaccel_device,
> +                                               &dev);
> +            } else {
> +                // This will be dealt with by API-specific
> initialisation
> +                // (using hwaccel_device), so nothing further needed
> here. return 0;
>              }
> -            tmp = av_asprintf("%s:%s",
> av_hwdevice_get_type_name(type),
> -                              ist->hwaccel_device);
> -            if (!tmp)
> -                return AVERROR(ENOMEM);
> -            err = hw_device_init_from_string(tmp, &dev);
> -            av_free(tmp);
> -            if (err < 0)
> -                return err;
> +        } else {
> +            if (ist->hwaccel_id == HWACCEL_AUTO) {
> +                ist->hwaccel_device_type = dev->type;
> +            } else if (ist->hwaccel_device_type != dev->type) {
> +                av_log(ist->dec_ctx, AV_LOG_ERROR, "Invalid hwaccel
> device "
> +                       "specified for decoder: device %s of type %s
> is not "
> +                       "usable with hwaccel %s.\n", dev->name,
> +                       av_hwdevice_get_type_name(dev->type),
> +
> av_hwdevice_get_type_name(ist->hwaccel_device_type));
> +                return AVERROR(EINVAL);
> +            }
>          }
>      } else {
> -        if (ist->hwaccel_id != HWACCEL_NONE)
> -            type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
> -        else
> -            type = hw_device_match_type_in_name(ist->dec->name);
> -        if (type != AV_HWDEVICE_TYPE_NONE) {
> +        if (ist->hwaccel_id == HWACCEL_AUTO) {
> +            auto_device = 1;
> +        } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
> +            type = ist->hwaccel_device_type;
>              dev = hw_device_get_by_type(type);
> +            if (!dev)
> +                err = hw_device_init_from_type(type, NULL, &dev);
> +        } else {
> +            dev = hw_device_match_by_codec(ist->dec);
>              if (!dev) {
> -
> hw_device_init_from_string(av_hwdevice_get_type_name(type),
> +                // No device for this codec, but not using generic
> hwaccel
> +                // and therefore may well not need one - ignore.
> +                return 0;
> +            }
> +        }
> +    }
> +
> +    if (auto_device) {
> +        int i;
> +        if (!avcodec_get_hw_config(ist->dec, 0)) {
> +            // Decoder does not support any hardware devices.
> +            return 0;
> +        }
> +        for (i = 0; !dev; i++) {
> +            config = avcodec_get_hw_config(ist->dec, i);
> +            if (!config)
> +                break;
> +            type = config->device_type;
> +            dev = hw_device_get_by_type(type);
> +            if (dev) {
> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
> +                       "hwaccel type %s with existing device %s.\n",
> +                       av_hwdevice_get_type_name(type), dev->name);
> +            }
> +        }
> +        for (i = 0; !dev; i++) {
> +            config = avcodec_get_hw_config(ist->dec, i);
> +            if (!config)
> +                break;
> +            type = config->device_type;
> +            // Try to make a new device of this type.
> +            err = hw_device_init_from_type(type, ist->hwaccel_device,
>                                             &dev);
> +            if (err < 0) {
> +                // Can't make a device of this type.
> +                continue;
> +            }
> +            if (ist->hwaccel_device) {
> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
> +                       "hwaccel type %s with new device created "
> +                       "from %s.\n", av_hwdevice_get_type_name(type),
> +                       ist->hwaccel_device);
> +            } else {
> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
> +                       "hwaccel type %s with new default device.\n",
> +                       av_hwdevice_get_type_name(type));
>              }
> +        }
> +        if (dev) {
> +            ist->hwaccel_device_type = type;
>          } else {
> -            // No device required.
> +            av_log(ist->dec_ctx, AV_LOG_INFO, "Auto hwaccel "
> +                   "disabled: no device found.\n");
> +            ist->hwaccel_id = HWACCEL_NONE;
>              return 0;
>          }
>      }
>  
>      if (!dev) {
> -        av_log(ist->dec_ctx, AV_LOG_WARNING, "No device available "
> -               "for decoder (device type %s for codec %s).\n",
> +        av_log(ist->dec_ctx, AV_LOG_ERROR, "No device available "
> +               "for decoder: device type %s needed for codec %s.\n",
>                 av_hwdevice_get_type_name(type), ist->dec->name);
> -        return 0;
> +        return err;
>      }
>  
>      ist->dec_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
> @@ -311,24 +413,16 @@ int hw_device_setup_for_decode(InputStream *ist)
>  
>  int hw_device_setup_for_encode(OutputStream *ost)
>  {
> -    enum AVHWDeviceType type;
>      HWDevice *dev;
>  
> -    type = hw_device_match_type_in_name(ost->enc->name);
> -    if (type != AV_HWDEVICE_TYPE_NONE) {
> -        dev = hw_device_get_by_type(type);
> -        if (!dev) {
> -            av_log(ost->enc_ctx, AV_LOG_WARNING, "No device
> available "
> -                   "for encoder (device type %s for codec %s).\n",
> -                   av_hwdevice_get_type_name(type), ost->enc->name);
> -            return 0;
> -        }
> +    dev = hw_device_match_by_codec(ost->enc);
> +    if (dev) {
>          ost->enc_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
>          if (!ost->enc_ctx->hw_device_ctx)
>              return AVERROR(ENOMEM);
>          return 0;
>      } else {
> -        // No device required.
> +        // No device required, or no device available.
>          return 0;
>      }
>  }
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 47d384166c..8ccd123c03 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -66,37 +66,11 @@
>  }
>  
>  const HWAccel hwaccels[] = {
> -#if HAVE_VDPAU_X11
> -    { "vdpau", hwaccel_decode_init, HWACCEL_VDPAU, AV_PIX_FMT_VDPAU,
> -      AV_HWDEVICE_TYPE_VDPAU },
> -#endif
> -#if CONFIG_D3D11VA
> -    { "d3d11va", hwaccel_decode_init, HWACCEL_D3D11VA,
> AV_PIX_FMT_D3D11,
> -      AV_HWDEVICE_TYPE_D3D11VA },
> -#endif
> -#if CONFIG_DXVA2
> -    { "dxva2", hwaccel_decode_init, HWACCEL_DXVA2,
> AV_PIX_FMT_DXVA2_VLD,
> -      AV_HWDEVICE_TYPE_DXVA2 },
> -#endif
> -#if CONFIG_VIDEOTOOLBOX
> -    { "videotoolbox",   videotoolbox_init,   HWACCEL_VIDEOTOOLBOX,
> AV_PIX_FMT_VIDEOTOOLBOX,
> -      AV_HWDEVICE_TYPE_NONE },
> -#endif
>  #if CONFIG_LIBMFX
> -    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV,
> -      AV_HWDEVICE_TYPE_NONE },
> -#endif
> -#if CONFIG_VAAPI
> -    { "vaapi", hwaccel_decode_init, HWACCEL_VAAPI, AV_PIX_FMT_VAAPI,
> -      AV_HWDEVICE_TYPE_VAAPI },
> -#endif
> -#if CONFIG_NVDEC
> -    { "nvdec", hwaccel_decode_init, HWACCEL_NVDEC, AV_PIX_FMT_CUDA,
> -       AV_HWDEVICE_TYPE_CUDA },
> +    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
>  #endif
>  #if CONFIG_CUVID
> -    { "cuvid", cuvid_init, HWACCEL_CUVID, AV_PIX_FMT_CUDA,
> -      AV_HWDEVICE_TYPE_NONE },
> +    { "cuvid", cuvid_init, HWACCEL_CUVID, AV_PIX_FMT_CUDA },
>  #endif
>      { 0 },
>  };
> @@ -194,12 +168,15 @@ static void init_options(OptionsContext *o)
>  
>  static int show_hwaccels(void *optctx, const char *opt, const char
> *arg) {
> +    enum AVHWDeviceType type = AV_HWDEVICE_TYPE_NONE;
>      int i;
>  
>      printf("Hardware acceleration methods:\n");
> -    for (i = 0; hwaccels[i].name; i++) {
> +    while ((type = av_hwdevice_iterate_types(type)) !=
> +           AV_HWDEVICE_TYPE_NONE)
> +        printf("%s\n", av_hwdevice_get_type_name(type));
> +    for (i = 0; hwaccels[i].name; i++)
>          printf("%s\n", hwaccels[i].name);
> -    }
>      printf("\n");
>      return 0;
>  }
> @@ -819,11 +796,16 @@ static void add_input_streams(OptionsContext
> *o, AVFormatContext *ic) 
>              MATCH_PER_STREAM_OPT(hwaccels, str, hwaccel, ic, st);
>              if (hwaccel) {
> +                // The NVDEC hwaccels use a CUDA device, so remap
> the name here.
> +                if (!strcmp(hwaccel, "nvdec"))
> +                    hwaccel = "cuda";
> +

This is a bit unfortunate. Can't we get the device type by matching
names from ist->dec->hw_configs[i]->hwaccel and then using
hw_configs[i]->device_type? That's admittedly clunky, of course.

>                  if (!strcmp(hwaccel, "none"))
>                      ist->hwaccel_id = HWACCEL_NONE;
>                  else if (!strcmp(hwaccel, "auto"))
>                      ist->hwaccel_id = HWACCEL_AUTO;
>                  else {
> +                    enum AVHWDeviceType type;
>                      int i;
>                      for (i = 0; hwaccels[i].name; i++) {
>                          if (!strcmp(hwaccels[i].name, hwaccel)) {
> @@ -833,9 +815,22 @@ static void add_input_streams(OptionsContext *o,
> AVFormatContext *ic) }
>  
>                      if (!ist->hwaccel_id) {
> +                        type =
> av_hwdevice_find_type_by_name(hwaccel);
> +                        if (type != AV_HWDEVICE_TYPE_NONE) {
> +                            ist->hwaccel_id = HWACCEL_GENERIC;
> +                            ist->hwaccel_device_type = type;
> +                        }
> +                    }
> +
> +                    if (!ist->hwaccel_id) {
>                          av_log(NULL, AV_LOG_FATAL, "Unrecognized
> hwaccel: %s.\n", hwaccel);
>                          av_log(NULL, AV_LOG_FATAL, "Supported
> hwaccels: ");
> +                        type = AV_HWDEVICE_TYPE_NONE;
> +                        while ((type =
> av_hwdevice_iterate_types(type)) !=
> +                               AV_HWDEVICE_TYPE_NONE)
> +                            av_log(NULL, AV_LOG_FATAL, "%s ",
> +                                   av_hwdevice_get_type_name(type));
>                          for (i = 0; hwaccels[i].name; i++)
>                              av_log(NULL, AV_LOG_FATAL, "%s ",
> hwaccels[i].name); av_log(NULL, AV_LOG_FATAL, "\n");


Rest is LGTM.

--phil

On 22/11/17 04:28, Philip Langdale wrote:
> On Sat, 18 Nov 2017 18:47:08 +0000
> Mark Thompson <sw@jkqxz.net> wrote:
> 
>> Removes specific support for all hwaccels supported by the generic
>> code (DXVA2, D3D11VA, NVDEC, VAAPI, VDPAU and videotoolbox).
>> ---
>>  fftools/ffmpeg.c     |  77 +++++++++++-----
>>  fftools/ffmpeg.h     |  10 +--
>>  fftools/ffmpeg_hw.c  | 244
>> +++++++++++++++++++++++++++++++++++----------------
>> fftools/ffmpeg_opt.c |  55 ++++++------ 4 files changed, 250
>> insertions(+), 136 deletions(-)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index babd85f7bc..acff815e74 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -2782,45 +2782,77 @@ fail:
>>      av_freep(&avc);
>>  }
>>  
>> -static const HWAccel *get_hwaccel(enum AVPixelFormat pix_fmt, enum
>> HWAccelID selected_hwaccel_id) -{
>> -    int i;
>> -    for (i = 0; hwaccels[i].name; i++)
>> -        if (hwaccels[i].pix_fmt == pix_fmt &&
>> -            (!selected_hwaccel_id || selected_hwaccel_id ==
>> HWACCEL_AUTO || hwaccels[i].id == selected_hwaccel_id))
>> -            return &hwaccels[i];
>> -    return NULL;
>> -}
>> -
>>  static enum AVPixelFormat get_format(AVCodecContext *s, const enum
>> AVPixelFormat *pix_fmts) {
>>      InputStream *ist = s->opaque;
>>      const enum AVPixelFormat *p;
>>      int ret;
>>  
>> -    for (p = pix_fmts; *p != -1; p++) {
>> +    for (p = pix_fmts; *p != AV_PIX_FMT_NONE; p++) {
>>          const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(*p);
>> -        const HWAccel *hwaccel;
>> +        const AVCodecHWConfig  *config = NULL;
>> +        int i;
>>  
>>          if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
>>              break;
>>  
>> -        hwaccel = get_hwaccel(*p, ist->hwaccel_id);
>> -        if (!hwaccel ||
>> -            (ist->active_hwaccel_id && ist->active_hwaccel_id !=
>> hwaccel->id) ||
>> -            (ist->hwaccel_id != HWACCEL_AUTO && ist->hwaccel_id !=
>> hwaccel->id))
>> -            continue;
>> +        if (ist->hwaccel_id == HWACCEL_GENERIC ||
>> +            ist->hwaccel_id == HWACCEL_AUTO) {
>> +            for (i = 0;; i++) {
>> +                config = avcodec_get_hw_config(s->codec, i);
>> +                if (!config)
>> +                    break;
>> +                if (!(config->methods &
>> +                      AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
>> +                    continue;
> 
> Just to be explicit, only METHOD_HW_DEVICE_CTX hwaccels can be
> generically initialized, so that's why you exclude any other method?

Correct.  METHOD_INTERNAL can work without any of this stuff, while the other two require further specific setup which we don't deal with here (can be done via the old API-specific mechanism, as it is for CUVID and libmfx).

>> +                if (config->pix_fmt == *p)
>> +                    break;
>> +            }
>> +        }
>> +        if (config) {
>> +            if (config->device_type != ist->hwaccel_device_type) {
>> +                // Different hwaccel offered, ignore.
>> +                continue;
>> +            }
>>  
>> -        ret = hwaccel->init(s);
>> -        if (ret < 0) {
>> -            if (ist->hwaccel_id == hwaccel->id) {
>> +            ret = hwaccel_decode_init(s);
>> +            if (ret < 0) {
>> +                if (ist->hwaccel_id == HWACCEL_GENERIC) {
>> +                    av_log(NULL, AV_LOG_FATAL,
>> +                           "%s hwaccel requested for input stream
>> #%d:%d, "
>> +                           "but cannot be initialized.\n",
>> +
>> av_hwdevice_get_type_name(config->device_type),
>> +                           ist->file_index, ist->st->index);
>> +                    return AV_PIX_FMT_NONE;
>> +                }
>> +                continue;
>> +            }
>> +        } else {
>> +            const HWAccel *hwaccel = NULL;
>> +            int i;
>> +            for (i = 0; hwaccels[i].name; i++) {
>> +                if (hwaccels[i].pix_fmt == *p) {
>> +                    hwaccel = &hwaccels[i];
>> +                    break;
> 
> This can also overrun the NULL terminator right? Or are we lucky
> because 'name' is at offset zero inside the struct and there's no
> dereference taking place.
> 
> I see this pattern has been used previously so it obviously works out.

I don't see a problem?  If hwaccels[i].name isn't set then we are looking at the sentinel entry in the list and the loop ends.

>> +                }
>> +            }
>> +            if (!hwaccel) {
>> +                // No hwaccel supporting this pixfmt.
>> +                continue;
>> +            }
>> +            if (hwaccel->id != ist->hwaccel_id) {
>> +                // Does not match requested hwaccel.
>> +                continue;
>> +            }
>> +
>> +            ret = hwaccel->init(s);
>> +            if (ret < 0) {
>>                  av_log(NULL, AV_LOG_FATAL,
>>                         "%s hwaccel requested for input stream
>> #%d:%d, " "but cannot be initialized.\n", hwaccel->name,
>>                         ist->file_index, ist->st->index);
>>                  return AV_PIX_FMT_NONE;
>>              }
>> -            continue;
>>          }
>>  
>>          if (ist->hw_frames_ctx) {
>> @@ -2829,8 +2861,7 @@ static enum AVPixelFormat
>> get_format(AVCodecContext *s, const enum AVPixelFormat return
>> AV_PIX_FMT_NONE; }
>>  
>> -        ist->active_hwaccel_id = hwaccel->id;
>> -        ist->hwaccel_pix_fmt   = *p;
>> +        ist->hwaccel_pix_fmt = *p;
>>          break;
>>      }
>>  
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index e0977e1bf1..8bb5bae862 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -61,14 +61,9 @@
>>  enum HWAccelID {
>>      HWACCEL_NONE = 0,
>>      HWACCEL_AUTO,
>> -    HWACCEL_VDPAU,
>> -    HWACCEL_DXVA2,
>> -    HWACCEL_VIDEOTOOLBOX,
>> +    HWACCEL_GENERIC,
>>      HWACCEL_QSV,
>> -    HWACCEL_VAAPI,
>>      HWACCEL_CUVID,
>> -    HWACCEL_D3D11VA,
>> -    HWACCEL_NVDEC,
>>  };
>>  
>>  typedef struct HWAccel {
>> @@ -76,7 +71,6 @@ typedef struct HWAccel {
>>      int (*init)(AVCodecContext *s);
>>      enum HWAccelID id;
>>      enum AVPixelFormat pix_fmt;
>> -    enum AVHWDeviceType device_type;
>>  } HWAccel;
>>  
>>  typedef struct HWDevice {
>> @@ -370,11 +364,11 @@ typedef struct InputStream {
>>  
>>      /* hwaccel options */
>>      enum HWAccelID hwaccel_id;
>> +    enum AVHWDeviceType hwaccel_device_type;
>>      char  *hwaccel_device;
>>      enum AVPixelFormat hwaccel_output_format;
>>  
>>      /* hwaccel context */
>> -    enum HWAccelID active_hwaccel_id;
>>      void  *hwaccel_ctx;
>>      void (*hwaccel_uninit)(AVCodecContext *s);
>>      int  (*hwaccel_get_buffer)(AVCodecContext *s, AVFrame *frame,
>> int flags); diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
>> index a4d1cada59..2ec1813854 100644
>> --- a/fftools/ffmpeg_hw.c
>> +++ b/fftools/ffmpeg_hw.c
>> @@ -64,6 +64,31 @@ static HWDevice *hw_device_add(void)
>>      return hw_devices[nb_hw_devices++];
>>  }
>>  
>> +static char *hw_device_default_name(enum AVHWDeviceType type)
>> +{
>> +    // Make an automatic name of the form "type%d".  We arbitrarily
>> +    // limit at 1000 anonymous devices of the same type - there is
>> +    // probably something else very wrong if you get to this limit.
>> +    const char *type_name = av_hwdevice_get_type_name(type);
>> +    char *name;
>> +    size_t index_pos;
>> +    int index, index_limit = 1000;
>> +    index_pos = strlen(type_name);
>> +    name = av_malloc(index_pos + 4);
>> +    if (!name)
>> +        return NULL;
>> +    for (index = 0; index < index_limit; index++) {
>> +        snprintf(name, index_pos + 4, "%s%d", type_name, index);
>> +        if (!hw_device_get_by_name(name))
>> +            break;
>> +    }
>> +    if (index >= index_limit) {
>> +        av_freep(&name);
>> +        return NULL;
>> +    }
>> +    return name;
>> +}
>> +
>>  int hw_device_init_from_string(const char *arg, HWDevice **dev_out)
>>  {
>>      // "type=name:device,key=value,key2=value2"
>> @@ -111,27 +136,11 @@ int hw_device_init_from_string(const char *arg,
>> HWDevice **dev_out) 
>>          p += 1 + k;
>>      } else {
>> -        // Give the device an automatic name of the form "type%d".
>> -        // We arbitrarily limit at 1000 anonymous devices of the same
>> -        // type - there is probably something else very wrong if you
>> -        // get to this limit.
>> -        size_t index_pos;
>> -        int index, index_limit = 1000;
>> -        index_pos = strlen(type_name);
>> -        name = av_malloc(index_pos + 4);
>> +        name = hw_device_default_name(type);
>>          if (!name) {
>>              err = AVERROR(ENOMEM);
>>              goto fail;
>>          }
>> -        for (index = 0; index < index_limit; index++) {
>> -            snprintf(name, index_pos + 4, "%s%d", type_name, index);
>> -            if (!hw_device_get_by_name(name))
>> -                break;
>> -        }
>> -        if (index >= index_limit) {
>> -            errmsg = "too many devices";
>> -            goto invalid;
>> -        }
>>      }
>>  
>>      if (!*p) {
>> @@ -214,6 +223,49 @@ fail:
>>      goto done;
>>  }
>>  
>> +static int hw_device_init_from_type(enum AVHWDeviceType type,
>> +                                    const char *device,
>> +                                    HWDevice **dev_out)
>> +{
>> +    AVBufferRef *device_ref = NULL;
>> +    HWDevice *dev;
>> +    char *name;
>> +    int err;
>> +
>> +    name = hw_device_default_name(type);
>> +    if (!name) {
>> +        err = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>> +
>> +    err = av_hwdevice_ctx_create(&device_ref, type, device, NULL, 0);
>> +    if (err < 0) {
>> +        av_log(NULL, AV_LOG_ERROR,
>> +               "Device creation failed: %d.\n", err);
>> +        goto fail;
>> +    }
>> +
>> +    dev = hw_device_add();
>> +    if (!dev) {
>> +        err = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>> +
>> +    dev->name = name;
>> +    dev->type = type;
>> +    dev->device_ref = device_ref;
>> +
>> +    if (dev_out)
>> +        *dev_out = dev;
>> +
>> +    return 0;
>> +
>> +fail:
>> +    av_freep(&name);
>> +    av_buffer_unref(&device_ref);
>> +    return err;
>> +}
>> +
>>  void hw_device_free_all(void)
>>  {
>>      int i;
>> @@ -226,80 +278,130 @@ void hw_device_free_all(void)
>>      nb_hw_devices = 0;
>>  }
>>  
>> -static enum AVHWDeviceType hw_device_match_type_by_hwaccel(enum
>> HWAccelID hwaccel_id) +static HWDevice
>> *hw_device_match_by_codec(const AVCodec *codec) {
>> +    const AVCodecHWConfig *config;
>> +    HWDevice *dev;
>>      int i;
>> -    if (hwaccel_id == HWACCEL_NONE)
>> -        return AV_HWDEVICE_TYPE_NONE;
>> -    for (i = 0; hwaccels[i].name; i++) {
>> -        if (hwaccels[i].id == hwaccel_id)
>> -            return hwaccels[i].device_type;
>> +    for (i = 0;; i++) {
>> +        config = avcodec_get_hw_config(codec, i);
>> +        if (!config)
>> +            return NULL;
>> +        if (!(config->methods &
>> AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
>> +            continue;
>> +        dev = hw_device_get_by_type(config->device_type);
>> +        if (dev)
>> +            return dev;
>>      }
>> -    return AV_HWDEVICE_TYPE_NONE;
>> -}
>> -
>> -static enum AVHWDeviceType hw_device_match_type_in_name(const char
>> *codec_name) -{
>> -    const char *type_name;
>> -    enum AVHWDeviceType type;
>> -    for (type = av_hwdevice_iterate_types(AV_HWDEVICE_TYPE_NONE);
>> -         type != AV_HWDEVICE_TYPE_NONE;
>> -         type = av_hwdevice_iterate_types(type)) {
>> -        type_name = av_hwdevice_get_type_name(type);
>> -        if (strstr(codec_name, type_name))
>> -            return type;
>> -    }
>> -    return AV_HWDEVICE_TYPE_NONE;
>>  }
>>  
>>  int hw_device_setup_for_decode(InputStream *ist)
>>  {
>> +    const AVCodecHWConfig *config;
>>      enum AVHWDeviceType type;
>> -    HWDevice *dev;
>> -    int err;
>> +    HWDevice *dev = NULL;
>> +    int err, auto_device = 0;
>>  
>>      if (ist->hwaccel_device) {
>>          dev = hw_device_get_by_name(ist->hwaccel_device);
>>          if (!dev) {
>> -            char *tmp;
>> -            type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
>> -            if (type == AV_HWDEVICE_TYPE_NONE) {
>> -                // No match - this isn't necessarily invalid, though,
>> -                // because an explicit device might not be needed or
>> -                // the hwaccel setup could be handled elsewhere.
>> +            if (ist->hwaccel_id == HWACCEL_AUTO) {
>> +                auto_device = 1;
>> +            } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
>> +                type = ist->hwaccel_device_type;
>> +                err = hw_device_init_from_type(type,
>> ist->hwaccel_device,
>> +                                               &dev);
>> +            } else {
>> +                // This will be dealt with by API-specific
>> initialisation
>> +                // (using hwaccel_device), so nothing further needed
>> here. return 0;
>>              }
>> -            tmp = av_asprintf("%s:%s",
>> av_hwdevice_get_type_name(type),
>> -                              ist->hwaccel_device);
>> -            if (!tmp)
>> -                return AVERROR(ENOMEM);
>> -            err = hw_device_init_from_string(tmp, &dev);
>> -            av_free(tmp);
>> -            if (err < 0)
>> -                return err;
>> +        } else {
>> +            if (ist->hwaccel_id == HWACCEL_AUTO) {
>> +                ist->hwaccel_device_type = dev->type;
>> +            } else if (ist->hwaccel_device_type != dev->type) {
>> +                av_log(ist->dec_ctx, AV_LOG_ERROR, "Invalid hwaccel
>> device "
>> +                       "specified for decoder: device %s of type %s
>> is not "
>> +                       "usable with hwaccel %s.\n", dev->name,
>> +                       av_hwdevice_get_type_name(dev->type),
>> +
>> av_hwdevice_get_type_name(ist->hwaccel_device_type));
>> +                return AVERROR(EINVAL);
>> +            }
>>          }
>>      } else {
>> -        if (ist->hwaccel_id != HWACCEL_NONE)
>> -            type = hw_device_match_type_by_hwaccel(ist->hwaccel_id);
>> -        else
>> -            type = hw_device_match_type_in_name(ist->dec->name);
>> -        if (type != AV_HWDEVICE_TYPE_NONE) {
>> +        if (ist->hwaccel_id == HWACCEL_AUTO) {
>> +            auto_device = 1;
>> +        } else if (ist->hwaccel_id == HWACCEL_GENERIC) {
>> +            type = ist->hwaccel_device_type;
>>              dev = hw_device_get_by_type(type);
>> +            if (!dev)
>> +                err = hw_device_init_from_type(type, NULL, &dev);
>> +        } else {
>> +            dev = hw_device_match_by_codec(ist->dec);
>>              if (!dev) {
>> -
>> hw_device_init_from_string(av_hwdevice_get_type_name(type),
>> +                // No device for this codec, but not using generic
>> hwaccel
>> +                // and therefore may well not need one - ignore.
>> +                return 0;
>> +            }
>> +        }
>> +    }
>> +
>> +    if (auto_device) {
>> +        int i;
>> +        if (!avcodec_get_hw_config(ist->dec, 0)) {
>> +            // Decoder does not support any hardware devices.
>> +            return 0;
>> +        }
>> +        for (i = 0; !dev; i++) {
>> +            config = avcodec_get_hw_config(ist->dec, i);
>> +            if (!config)
>> +                break;
>> +            type = config->device_type;
>> +            dev = hw_device_get_by_type(type);
>> +            if (dev) {
>> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
>> +                       "hwaccel type %s with existing device %s.\n",
>> +                       av_hwdevice_get_type_name(type), dev->name);
>> +            }
>> +        }
>> +        for (i = 0; !dev; i++) {
>> +            config = avcodec_get_hw_config(ist->dec, i);
>> +            if (!config)
>> +                break;
>> +            type = config->device_type;
>> +            // Try to make a new device of this type.
>> +            err = hw_device_init_from_type(type, ist->hwaccel_device,
>>                                             &dev);
>> +            if (err < 0) {
>> +                // Can't make a device of this type.
>> +                continue;
>> +            }
>> +            if (ist->hwaccel_device) {
>> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
>> +                       "hwaccel type %s with new device created "
>> +                       "from %s.\n", av_hwdevice_get_type_name(type),
>> +                       ist->hwaccel_device);
>> +            } else {
>> +                av_log(ist->dec_ctx, AV_LOG_INFO, "Using auto "
>> +                       "hwaccel type %s with new default device.\n",
>> +                       av_hwdevice_get_type_name(type));
>>              }
>> +        }
>> +        if (dev) {
>> +            ist->hwaccel_device_type = type;
>>          } else {
>> -            // No device required.
>> +            av_log(ist->dec_ctx, AV_LOG_INFO, "Auto hwaccel "
>> +                   "disabled: no device found.\n");
>> +            ist->hwaccel_id = HWACCEL_NONE;
>>              return 0;
>>          }
>>      }
>>  
>>      if (!dev) {
>> -        av_log(ist->dec_ctx, AV_LOG_WARNING, "No device available "
>> -               "for decoder (device type %s for codec %s).\n",
>> +        av_log(ist->dec_ctx, AV_LOG_ERROR, "No device available "
>> +               "for decoder: device type %s needed for codec %s.\n",
>>                 av_hwdevice_get_type_name(type), ist->dec->name);
>> -        return 0;
>> +        return err;
>>      }
>>  
>>      ist->dec_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
>> @@ -311,24 +413,16 @@ int hw_device_setup_for_decode(InputStream *ist)
>>  
>>  int hw_device_setup_for_encode(OutputStream *ost)
>>  {
>> -    enum AVHWDeviceType type;
>>      HWDevice *dev;
>>  
>> -    type = hw_device_match_type_in_name(ost->enc->name);
>> -    if (type != AV_HWDEVICE_TYPE_NONE) {
>> -        dev = hw_device_get_by_type(type);
>> -        if (!dev) {
>> -            av_log(ost->enc_ctx, AV_LOG_WARNING, "No device
>> available "
>> -                   "for encoder (device type %s for codec %s).\n",
>> -                   av_hwdevice_get_type_name(type), ost->enc->name);
>> -            return 0;
>> -        }
>> +    dev = hw_device_match_by_codec(ost->enc);
>> +    if (dev) {
>>          ost->enc_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
>>          if (!ost->enc_ctx->hw_device_ctx)
>>              return AVERROR(ENOMEM);
>>          return 0;
>>      } else {
>> -        // No device required.
>> +        // No device required, or no device available.
>>          return 0;
>>      }
>>  }
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index 47d384166c..8ccd123c03 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -66,37 +66,11 @@
>>  }
>>  
>>  const HWAccel hwaccels[] = {
>> -#if HAVE_VDPAU_X11
>> -    { "vdpau", hwaccel_decode_init, HWACCEL_VDPAU, AV_PIX_FMT_VDPAU,
>> -      AV_HWDEVICE_TYPE_VDPAU },
>> -#endif
>> -#if CONFIG_D3D11VA
>> -    { "d3d11va", hwaccel_decode_init, HWACCEL_D3D11VA,
>> AV_PIX_FMT_D3D11,
>> -      AV_HWDEVICE_TYPE_D3D11VA },
>> -#endif
>> -#if CONFIG_DXVA2
>> -    { "dxva2", hwaccel_decode_init, HWACCEL_DXVA2,
>> AV_PIX_FMT_DXVA2_VLD,
>> -      AV_HWDEVICE_TYPE_DXVA2 },
>> -#endif
>> -#if CONFIG_VIDEOTOOLBOX
>> -    { "videotoolbox",   videotoolbox_init,   HWACCEL_VIDEOTOOLBOX,
>> AV_PIX_FMT_VIDEOTOOLBOX,
>> -      AV_HWDEVICE_TYPE_NONE },
>> -#endif
>>  #if CONFIG_LIBMFX
>> -    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV,
>> -      AV_HWDEVICE_TYPE_NONE },
>> -#endif
>> -#if CONFIG_VAAPI
>> -    { "vaapi", hwaccel_decode_init, HWACCEL_VAAPI, AV_PIX_FMT_VAAPI,
>> -      AV_HWDEVICE_TYPE_VAAPI },
>> -#endif
>> -#if CONFIG_NVDEC
>> -    { "nvdec", hwaccel_decode_init, HWACCEL_NVDEC, AV_PIX_FMT_CUDA,
>> -       AV_HWDEVICE_TYPE_CUDA },
>> +    { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV },
>>  #endif
>>  #if CONFIG_CUVID
>> -    { "cuvid", cuvid_init, HWACCEL_CUVID, AV_PIX_FMT_CUDA,
>> -      AV_HWDEVICE_TYPE_NONE },
>> +    { "cuvid", cuvid_init, HWACCEL_CUVID, AV_PIX_FMT_CUDA },
>>  #endif
>>      { 0 },
>>  };
>> @@ -194,12 +168,15 @@ static void init_options(OptionsContext *o)
>>  
>>  static int show_hwaccels(void *optctx, const char *opt, const char
>> *arg) {
>> +    enum AVHWDeviceType type = AV_HWDEVICE_TYPE_NONE;
>>      int i;
>>  
>>      printf("Hardware acceleration methods:\n");
>> -    for (i = 0; hwaccels[i].name; i++) {
>> +    while ((type = av_hwdevice_iterate_types(type)) !=
>> +           AV_HWDEVICE_TYPE_NONE)
>> +        printf("%s\n", av_hwdevice_get_type_name(type));
>> +    for (i = 0; hwaccels[i].name; i++)
>>          printf("%s\n", hwaccels[i].name);
>> -    }
>>      printf("\n");
>>      return 0;
>>  }
>> @@ -819,11 +796,16 @@ static void add_input_streams(OptionsContext
>> *o, AVFormatContext *ic) 
>>              MATCH_PER_STREAM_OPT(hwaccels, str, hwaccel, ic, st);
>>              if (hwaccel) {
>> +                // The NVDEC hwaccels use a CUDA device, so remap
>> the name here.
>> +                if (!strcmp(hwaccel, "nvdec"))
>> +                    hwaccel = "cuda";
>> +
> 
> This is a bit unfortunate. Can't we get the device type by matching
> names from ist->dec->hw_configs[i]->hwaccel and then using
> hw_configs[i]->device_type? That's admittedly clunky, of course.

AVHWAccel isn't user-visible at all any more, so we can't match via anything in the hwaccel itself.

Not sure if there's a better method - the only visible string name is the device one, so that's what I've matched on.  (From the point of view of basically everything except configure, the NVDEC hwaccel is really the CUDA hwaccel - nothing looks at the string name, they just see that it uses a CUDA device and generates CUDA frames.)

>>                  if (!strcmp(hwaccel, "none"))
>>                      ist->hwaccel_id = HWACCEL_NONE;
>>                  else if (!strcmp(hwaccel, "auto"))
>>                      ist->hwaccel_id = HWACCEL_AUTO;
>>                  else {
>> +                    enum AVHWDeviceType type;
>>                      int i;
>>                      for (i = 0; hwaccels[i].name; i++) {
>>                          if (!strcmp(hwaccels[i].name, hwaccel)) {
>> @@ -833,9 +815,22 @@ static void add_input_streams(OptionsContext *o,
>> AVFormatContext *ic) }
>>  
>>                      if (!ist->hwaccel_id) {
>> +                        type =
>> av_hwdevice_find_type_by_name(hwaccel);
>> +                        if (type != AV_HWDEVICE_TYPE_NONE) {
>> +                            ist->hwaccel_id = HWACCEL_GENERIC;
>> +                            ist->hwaccel_device_type = type;
>> +                        }
>> +                    }
>> +
>> +                    if (!ist->hwaccel_id) {
>>                          av_log(NULL, AV_LOG_FATAL, "Unrecognized
>> hwaccel: %s.\n", hwaccel);
>>                          av_log(NULL, AV_LOG_FATAL, "Supported
>> hwaccels: ");
>> +                        type = AV_HWDEVICE_TYPE_NONE;
>> +                        while ((type =
>> av_hwdevice_iterate_types(type)) !=
>> +                               AV_HWDEVICE_TYPE_NONE)
>> +                            av_log(NULL, AV_LOG_FATAL, "%s ",
>> +                                   av_hwdevice_get_type_name(type));
>>                          for (i = 0; hwaccels[i].name; i++)
>>                              av_log(NULL, AV_LOG_FATAL, "%s ",
>> hwaccels[i].name); av_log(NULL, AV_LOG_FATAL, "\n");
> 
> 
> Rest is LGTM.

Thanks,

- Mark