| Message ID | 20190220025821.31346-7-zhong.li@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2019-02-20 at 10:58 +0800, Zhong Li wrote: > VP9 decoder is supported on Intel kabyLake+ platforms with MSDK > Version 1.19+ > > Signed-off-by: Zhong Li <zhong.li@intel.com> > --- > Changelog | 1 + > configure | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/qsv.c | 5 +++++ > libavcodec/qsvdec_other.c | 46 ++++++++++++++++++++++++++++++++++++- > -- > 5 files changed, 51 insertions(+), 3 deletions(-) > > diff --git a/Changelog b/Changelog > index f289812bfc..141ffd9610 100644 > --- a/Changelog > +++ b/Changelog > @@ -20,6 +20,7 @@ version <next>: > - libaribb24 based ARIB STD-B24 caption support (profiles A and C) > - Support decoding of HEVC 4:4:4 content in nvdec and cuviddec > - Intel QSV-accelerated MJPEG decoding > +- Intel QSV-accelerated VP9 decoding > > > version 4.1: > diff --git a/configure b/configure > index de994673a0..84fbe49bcc 100755 > --- a/configure > +++ b/configure > @@ -3037,6 +3037,7 @@ vp8_v4l2m2m_decoder_deps="v4l2_m2m > vp8_v4l2_m2m" > vp8_v4l2m2m_encoder_deps="v4l2_m2m vp8_v4l2_m2m" > vp9_cuvid_decoder_deps="cuvid" > vp9_mediacodec_decoder_deps="mediacodec" > +vp9_qsv_decoder_select="qsvdec" > vp9_rkmpp_decoder_deps="rkmpp" > vp9_vaapi_encoder_deps="VAEncPictureParameterBufferVP9" > vp9_vaapi_encoder_select="vaapi_encode" > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index 391619c38c..248b8f15b8 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -776,6 +776,7 @@ extern AVCodec ff_vp8_v4l2m2m_encoder; > extern AVCodec ff_vp8_vaapi_encoder; > extern AVCodec ff_vp9_cuvid_decoder; > extern AVCodec ff_vp9_mediacodec_decoder; > +extern AVCodec ff_vp9_qsv_decoder; > extern AVCodec ff_vp9_vaapi_encoder; > > // The iterate API is not usable with ossfuzz due to the excessive > size of binaries created > diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c > index 711fd3df1e..7dcfb04316 100644 > --- a/libavcodec/qsv.c > +++ b/libavcodec/qsv.c > @@ -60,6 +60,11 @@ int ff_qsv_codec_id_to_mfx(enum AVCodecID > codec_id) > #endif > case AV_CODEC_ID_MJPEG: > return MFX_CODEC_JPEG; > +#if QSV_VERSION_ATLEAST(1, 19) > + case AV_CODEC_ID_VP9: > + return MFX_CODEC_VP9; > +#endif > + > default: > break; > } > diff --git a/libavcodec/qsvdec_other.c b/libavcodec/qsvdec_other.c > index 8c9c1e6b13..50bfc818b0 100644 > --- a/libavcodec/qsvdec_other.c > +++ b/libavcodec/qsvdec_other.c > @@ -1,5 +1,5 @@ > /* > - * Intel MediaSDK QSV based MPEG-2, VC-1, VP8 and MJPEG decoders > + * Intel MediaSDK QSV based MPEG-2, VC-1, VP8, MJPEG and VP9 > decoders > * > * copyright (c) 2015 Anton Khirnov > * > @@ -60,8 +60,8 @@ static av_cold int qsv_decode_close(AVCodecContext > *avctx) > { > QSVOtherContext *s = avctx->priv_data; > > -#if CONFIG_VP8_QSV_DECODER > - if (avctx->codec_id == AV_CODEC_ID_VP8) > +#if CONFIG_VP8_QSV_DECODER || CONFIG_VP9_QSV_DECODER Seems to be wrong since AV_CODEC_ID_VP8 is covered by QSV_VERSION_ATLEAST(1, 12) and AV_CODEC_ID_VP9 by QSV_VERSION_ATLEAST(1, 19). Thus, you may step into situation when one of AV_CODEC_ID_* won't be declared... > + if (avctx->codec_id == AV_CODEC_ID_VP8 || avctx->codec_id == > AV_CODEC_ID_VP9) > av_freep(&s->qsv.load_plugins); > #endif > > @@ -90,6 +90,17 @@ static av_cold int qsv_decode_init(AVCodecContext > *avctx) > } > #endif > > +#if CONFIG_VP9_QSV_DECODER > + if (avctx->codec_id == AV_CODEC_ID_VP9) { > + static const char *uid_vp9dec_hw = > "a922394d8d87452f878c51f2fc9b4131"; Should not be actually needed (and I hope it will work:)). VP9 hw plugin is actually a tiny compatibility stub which redirects everything to the mediasdk library. Considering that you just add VP9 decoding support you don't need to care about compatibility (I hope). Hence, you can try to just initialize VP9 decoder directly from the mediasdk library as you are doing for AVC decoder. > + > + av_freep(&s->qsv.load_plugins); > + s->qsv.load_plugins = av_strdup(uid_vp9dec_hw); > + if (!s->qsv.load_plugins) > + return AVERROR(ENOMEM); > + } > +#endif > + > s->packet_fifo = av_fifo_alloc(sizeof(AVPacket)); > if (!s->packet_fifo) { > ret = AVERROR(ENOMEM); > @@ -281,3 +292,32 @@ AVCodec ff_mjpeg_qsv_decoder = { > AV_PIX_FMT_NONE > }, > }; > #endif > + > +#if CONFIG_VP9_QSV_DECODER > +static const AVClass vp9_qsv_class = { > + .class_name = "vp9_qsv", > + .item_name = av_default_item_name, > + .option = options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > +AVCodec ff_vp9_qsv_decoder = { > + .name = "vp9_qsv", > + .long_name = NULL_IF_CONFIG_SMALL("VP9 video (Intel Quick > Sync Video acceleration)"), > + .priv_data_size = sizeof(QSVOtherContext), > + .type = AVMEDIA_TYPE_VIDEO, > + .id = AV_CODEC_ID_VP9, > + .init = qsv_decode_init, > + .decode = qsv_decode_frame, > + .flush = qsv_decode_flush, > + .close = qsv_decode_close, > + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1 | > AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HYBRID, > + .priv_class = &vp9_qsv_class, > + .pix_fmts = (const enum AVPixelFormat[]){ AV_PIX_FMT_NV12, > + AV_PIX_FMT_P010, Order of formats in pix_fmts declarations start to frighten me even more... To the concern I raised in patches #3,4 of this series... > + AV_PIX_FMT_QSV, > + AV_PIX_FMT_NONE > }, > + .hw_configs = ff_qsv_hw_configs, > + .wrapper_name = "qsv", > +}; > +#endif
2019-02-20 3:58 GMT+01:00, Zhong Li <zhong.li@intel.com>: > VP9 decoder is supported on Intel kabyLake+ platforms with MSDK Version > 1.19+ > diff --git a/Changelog b/Changelog > index f289812bfc..141ffd9610 100644 > --- a/Changelog > +++ b/Changelog > @@ -20,6 +20,7 @@ version <next>: > - libaribb24 based ARIB STD-B24 caption support (profiles A and C) > - Support decoding of HEVC 4:4:4 content in nvdec and cuviddec > - Intel QSV-accelerated MJPEG decoding > +- Intel QSV-accelerated VP9 decoding Please merge these lines. Carl Eugen
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Thursday, February 21, 2019 8:23 AM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2 6/6] lavc/qsvdec: Add VP9 decoder > support > > 2019-02-20 3:58 GMT+01:00, Zhong Li <zhong.li@intel.com>: > > VP9 decoder is supported on Intel kabyLake+ platforms with MSDK > > Version 1.19+ > > > diff --git a/Changelog b/Changelog > > index f289812bfc..141ffd9610 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -20,6 +20,7 @@ version <next>: > > - libaribb24 based ARIB STD-B24 caption support (profiles A and C) > > - Support decoding of HEVC 4:4:4 content in nvdec and cuviddec > > > - Intel QSV-accelerated MJPEG decoding > > +- Intel QSV-accelerated VP9 decoding > > Please merge these lines. > > Carl Eugen Ok, will update. Thanks.
> > @@ -90,6 +90,17 @@ static av_cold int qsv_decode_init(AVCodecContext > > *avctx) > > } > > #endif > > > > +#if CONFIG_VP9_QSV_DECODER > > + if (avctx->codec_id == AV_CODEC_ID_VP9) { > > + static const char *uid_vp9dec_hw = > > "a922394d8d87452f878c51f2fc9b4131"; > > Should not be actually needed (and I hope it will work:)). VP9 hw plugin is > actually a tiny compatibility stub which redirects everything to the mediasdk > library. Considering that you just add VP9 decoding support you don't need > to care about compatibility (I hope). Hence, you can try to just initialize VP9 > decoder directly from the mediasdk library as you are doing for AVC decoder. Good point. But my question is that will it broken for the case "the latest ffmpeg + an old version MSDK"? Thus means: 1. Start from the version for MSDK support VP9 decoding, hw plugin is not needed. 2. Or we don't care the compatibility "the latest ffmpeg + an old version MSDK", user should update MSDK. If it is case 1, I am quite happy to remove vp9 hw plugin code. If it is case2, I would say I can't agree. How do you think?
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Li, Zhong > Sent: Thursday, February 21, 2019 2:01 PM > To: Rogozhkin, Dmitry V <dmitry.v.rogozhkin@intel.com>; > ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v2 6/6] lavc/qsvdec: Add VP9 decoder > support > > > > @@ -90,6 +90,17 @@ static av_cold int > qsv_decode_init(AVCodecContext > > > *avctx) > > > } > > > #endif > > > > > > +#if CONFIG_VP9_QSV_DECODER > > > + if (avctx->codec_id == AV_CODEC_ID_VP9) { > > > + static const char *uid_vp9dec_hw = > > > "a922394d8d87452f878c51f2fc9b4131"; > > > > Should not be actually needed (and I hope it will work:)). VP9 hw > > plugin is actually a tiny compatibility stub which redirects > > everything to the mediasdk library. Considering that you just add VP9 > > decoding support you don't need to care about compatibility (I hope). > > Hence, you can try to just initialize VP9 decoder directly from the mediasdk > library as you are doing for AVC decoder. > > Good point. But my question is that will it broken for the case "the latest > ffmpeg + an old version MSDK"? > Thus means: > 1. Start from the version for MSDK support VP9 decoding, hw plugin is not > needed. Sorry for the typo, version->first version. Means at the beginning of vp9 enabled. > 2. Or we don't care the compatibility "the latest ffmpeg + an old version > MSDK", user should update MSDK.
On Thu, 2019-02-21 at 14:01 +0800, Li, Zhong wrote: > > > @@ -90,6 +90,17 @@ static av_cold int > > > qsv_decode_init(AVCodecContext > > > *avctx) > > > } > > > #endif > > > > > > +#if CONFIG_VP9_QSV_DECODER > > > + if (avctx->codec_id == AV_CODEC_ID_VP9) { > > > + static const char *uid_vp9dec_hw = > > > "a922394d8d87452f878c51f2fc9b4131"; > > > > Should not be actually needed (and I hope it will work:)). VP9 hw > > plugin is > > actually a tiny compatibility stub which redirects everything to > > the mediasdk > > library. Considering that you just add VP9 decoding support you > > don't need > > to care about compatibility (I hope). Hence, you can try to just > > initialize VP9 > > decoder directly from the mediasdk library as you are doing for AVC > > decoder. > > Good point. But my question is that will it broken for the case "the > latest ffmpeg + an old version MSDK"? > Thus means: > 1. Start from the version for MSDK support VP9 decoding, hw plugin is > not needed. > 2. Or we don't care the compatibility "the latest ffmpeg + an old > version MSDK", user should update MSDK. > > If it is case 1, I am quite happy to remove vp9 hw plugin code. If it > is case2, I would say I can't agree. > How do you think? This patch adds vp9 decoder support to ffmpeg-qsv. This feature was not actually available previously in ffmpeg-qsv, so there will be versions of ffmpeg which don't support vp9 decoder in qsv path. And potentially there could be some versions of mediasdk which won't provide vp9 decoder at all or in a way anticipated by ffmpeg (without plugin if you will agree to make this change). I don't see a problem here. Instead I see another problem: will you be able to verify that current patch (to initialize vp9 decoder via plugin) actually is capable to work with the old mediasdk versions? And what will happen if it is not or some feature won't work because of some bug in this old mediasdk version? Such a bug either won't be fixed at all or if it will be fixed, mediasdk update will be required which most probably will simply allow to work with vp9 decoder without mediasdk plugin. Thus, I suggest to not complicate things and avoid pursuing hypothetical compatibility with some old version of mediasdk library. Mind also, that avoiding plugin path we actually avoid additional memory allocations, .so loading overhead, etc. More widely, I would actually suggest to revise existing mediasdk plugins loading path and change it from loading plugins at the first place to: 1. Try to initialize component directly from the library 2. If #1 fails, try to initialize component from the plugin This will allow to avoid a cost of plugins loading when you don't actually need to load them and will preserve compatibility with the old versions of mediasdk.
> More widely, I would actually suggest to revise existing mediasdk plugins > loading path and change it from loading plugins at the first place to: > 1. Try to initialize component directly from the library 2. If #1 fails, try to > initialize component from the plugin This will allow to avoid a cost of plugins > loading when you don't actually need to load them and will preserve > compatibility with the old versions of mediasdk. That could be a good idea but only based on a query interface, instead of MSDK initialization failure. There are many reasons may cause the failure but application can't what is the exact reason. I haven't see MFXVideoDECODE_Query() has exposed this, this could be addressed if you have a plan to improve MSDK query functions.
> > +AVCodec ff_vp9_qsv_decoder = { > > + .name = "vp9_qsv", > > + .long_name = NULL_IF_CONFIG_SMALL("VP9 video (Intel > Quick > > Sync Video acceleration)"), > > + .priv_data_size = sizeof(QSVOtherContext), > > + .type = AVMEDIA_TYPE_VIDEO, > > + .id = AV_CODEC_ID_VP9, > > + .init = qsv_decode_init, > > + .decode = qsv_decode_frame, > > + .flush = qsv_decode_flush, > > + .close = qsv_decode_close, > > + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1 | > > AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HYBRID, > > + .priv_class = &vp9_qsv_class, > > + .pix_fmts = (const enum > AVPixelFormat[]){ AV_PIX_FMT_NV12, > > > + > AV_PIX_FMT_P010, > > Order of formats in pix_fmts declarations start to frighten me even more... > To the concern I raised in patches #3,4 of this series... This array here is just to declare the pix_fmt capability list， But in qsvdec.c, the pix_fmts array is to init qsv HW and SW format for frame allocation. It can't be a list for a specified clip, it should be NV12 OR (not and ) P010 format .
diff --git a/Changelog b/Changelog index f289812bfc..141ffd9610 100644 --- a/Changelog +++ b/Changelog @@ -20,6 +20,7 @@ version <next>: - libaribb24 based ARIB STD-B24 caption support (profiles A and C) - Support decoding of HEVC 4:4:4 content in nvdec and cuviddec - Intel QSV-accelerated MJPEG decoding +- Intel QSV-accelerated VP9 decoding version 4.1: diff --git a/configure b/configure index de994673a0..84fbe49bcc 100755 --- a/configure +++ b/configure @@ -3037,6 +3037,7 @@ vp8_v4l2m2m_decoder_deps="v4l2_m2m vp8_v4l2_m2m" vp8_v4l2m2m_encoder_deps="v4l2_m2m vp8_v4l2_m2m" vp9_cuvid_decoder_deps="cuvid" vp9_mediacodec_decoder_deps="mediacodec" +vp9_qsv_decoder_select="qsvdec" vp9_rkmpp_decoder_deps="rkmpp" vp9_vaapi_encoder_deps="VAEncPictureParameterBufferVP9" vp9_vaapi_encoder_select="vaapi_encode" diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index 391619c38c..248b8f15b8 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -776,6 +776,7 @@ extern AVCodec ff_vp8_v4l2m2m_encoder; extern AVCodec ff_vp8_vaapi_encoder; extern AVCodec ff_vp9_cuvid_decoder; extern AVCodec ff_vp9_mediacodec_decoder; +extern AVCodec ff_vp9_qsv_decoder; extern AVCodec ff_vp9_vaapi_encoder; // The iterate API is not usable with ossfuzz due to the excessive size of binaries created diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c index 711fd3df1e..7dcfb04316 100644 --- a/libavcodec/qsv.c +++ b/libavcodec/qsv.c @@ -60,6 +60,11 @@ int ff_qsv_codec_id_to_mfx(enum AVCodecID codec_id) #endif case AV_CODEC_ID_MJPEG: return MFX_CODEC_JPEG; +#if QSV_VERSION_ATLEAST(1, 19) + case AV_CODEC_ID_VP9: + return MFX_CODEC_VP9; +#endif + default: break; } diff --git a/libavcodec/qsvdec_other.c b/libavcodec/qsvdec_other.c index 8c9c1e6b13..50bfc818b0 100644 --- a/libavcodec/qsvdec_other.c +++ b/libavcodec/qsvdec_other.c @@ -1,5 +1,5 @@ /* - * Intel MediaSDK QSV based MPEG-2, VC-1, VP8 and MJPEG decoders + * Intel MediaSDK QSV based MPEG-2, VC-1, VP8, MJPEG and VP9 decoders * * copyright (c) 2015 Anton Khirnov * @@ -60,8 +60,8 @@ static av_cold int qsv_decode_close(AVCodecContext *avctx) { QSVOtherContext *s = avctx->priv_data; -#if CONFIG_VP8_QSV_DECODER - if (avctx->codec_id == AV_CODEC_ID_VP8) +#if CONFIG_VP8_QSV_DECODER || CONFIG_VP9_QSV_DECODER + if (avctx->codec_id == AV_CODEC_ID_VP8 || avctx->codec_id == AV_CODEC_ID_VP9) av_freep(&s->qsv.load_plugins); #endif @@ -90,6 +90,17 @@ static av_cold int qsv_decode_init(AVCodecContext *avctx) } #endif +#if CONFIG_VP9_QSV_DECODER + if (avctx->codec_id == AV_CODEC_ID_VP9) { + static const char *uid_vp9dec_hw = "a922394d8d87452f878c51f2fc9b4131"; + + av_freep(&s->qsv.load_plugins); + s->qsv.load_plugins = av_strdup(uid_vp9dec_hw); + if (!s->qsv.load_plugins) + return AVERROR(ENOMEM); + } +#endif + s->packet_fifo = av_fifo_alloc(sizeof(AVPacket)); if (!s->packet_fifo) { ret = AVERROR(ENOMEM); @@ -281,3 +292,32 @@ AVCodec ff_mjpeg_qsv_decoder = { AV_PIX_FMT_NONE }, }; #endif + +#if CONFIG_VP9_QSV_DECODER +static const AVClass vp9_qsv_class = { + .class_name = "vp9_qsv", + .item_name = av_default_item_name, + .option = options, + .version = LIBAVUTIL_VERSION_INT, +}; + +AVCodec ff_vp9_qsv_decoder = { + .name = "vp9_qsv", + .long_name = NULL_IF_CONFIG_SMALL("VP9 video (Intel Quick Sync Video acceleration)"), + .priv_data_size = sizeof(QSVOtherContext), + .type = AVMEDIA_TYPE_VIDEO, + .id = AV_CODEC_ID_VP9, + .init = qsv_decode_init, + .decode = qsv_decode_frame, + .flush = qsv_decode_flush, + .close = qsv_decode_close, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1 | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HYBRID, + .priv_class = &vp9_qsv_class, + .pix_fmts = (const enum AVPixelFormat[]){ AV_PIX_FMT_NV12, + AV_PIX_FMT_P010, + AV_PIX_FMT_QSV, + AV_PIX_FMT_NONE }, + .hw_configs = ff_qsv_hw_configs, + .wrapper_name = "qsv", +}; +#endif
VP9 decoder is supported on Intel kabyLake+ platforms with MSDK Version 1.19+ Signed-off-by: Zhong Li <zhong.li@intel.com> --- Changelog | 1 + configure | 1 + libavcodec/allcodecs.c | 1 + libavcodec/qsv.c | 5 +++++ libavcodec/qsvdec_other.c | 46 ++++++++++++++++++++++++++++++++++++--- 5 files changed, 51 insertions(+), 3 deletions(-)