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

2017-11-23 Thread Mark Thompson 
On 22/11/17 04:28, Philip Langdale wrote:
> On Sat, 18 Nov 2017 18:47:08 +
> Mark Thompson  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();
>>  }
>>  
>> -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 [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 = [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",

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

2017-11-21 Thread Philip Langdale 
On Sat, 18 Nov 2017 18:47:08 +
Mark Thompson  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();
>  }
>  
> -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 [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 = [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

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

2017-11-18 Thread Mark Thompson 
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();
 }
 
-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 [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;
+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 = [i];
+break;
+}
+}
+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