Re: [FFmpeg-devel] [PATCH 08/13] ffmpeg: Use codec hardware config to configure hwaccels
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(&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"
Re: [FFmpeg-devel] [PATCH 08/13] ffmpeg: Use codec hardware config to configure hwaccels
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(&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/ffto
[FFmpeg-devel] [PATCH 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(-) 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; +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; +} +} +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; } HWAc