| Message ID | 20180206081755.24616-1-haihao.xiang@intel.com |
|---|---|
| State | New |
| Headers | show |
On 06/02/18 08:17, Haihao Xiang wrote: > Although VAEntrypointEncSliceLP was added in old version of VAAPI, we > never implemented it for VAAPI H265 encoder before. so it is reasonable > to require VAAPI 1.0 > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > --- > libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c > index f3b4f6c7e26..efe1148127f 100644 > --- a/libavcodec/vaapi_encode_h265.c > +++ b/libavcodec/vaapi_encode_h265.c > @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { > int aud; > int profile; > int level; > + int low_power; > } VAAPIEncodeH265Options; > > > @@ -914,7 +915,18 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext *avctx) > avctx->profile); > return AVERROR(EINVAL); > } > - ctx->va_entrypoint = VAEntrypointEncSlice; > + > + if (opt->low_power) { > +#if VA_CHECK_VERSION(1, 0, 0) > + ctx->va_entrypoint = VAEntrypointEncSliceLP; > +#else > + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " > + "supported with this VAAPI version.\n"); > + return AVERROR(EINVAL); > +#endif > + } else { > + ctx->va_entrypoint = VAEntrypointEncSlice; > + } > > if (avctx->bit_rate > 0) { > if (avctx->rc_max_rate == avctx->bit_rate) > @@ -986,6 +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { > { LEVEL("6.2", 186) }, > #undef LEVEL > > + { "low_power", "Use low-power encoding mode (experimental: only supported " Do you want to copy this comment? > + "on some platforms, does not support all features)", > + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS }, > + > { NULL }, > }; > > How can I test this (and the following patch)? It doesn't appear to exist on any current platform/driver. I don't like how this code is being copied around, but I guess it would work for now. I'll look into doing something more sensible here. - Mark
> On 06/02/18 08:17, Haihao Xiang wrote: > > Although VAEntrypointEncSliceLP was added in old version of VAAPI, we > > never implemented it for VAAPI H265 encoder before. so it is reasonable > > to require VAAPI 1.0 > > > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > > --- > > libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c > > index f3b4f6c7e26..efe1148127f 100644 > > --- a/libavcodec/vaapi_encode_h265.c > > +++ b/libavcodec/vaapi_encode_h265.c > > @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { > > int aud; > > int profile; > > int level; > > + int low_power; > > } VAAPIEncodeH265Options; > > > > > > @@ -914,7 +915,18 @@ static av_cold int > > vaapi_encode_h265_init(AVCodecContext *avctx) > > avctx->profile); > > return AVERROR(EINVAL); > > } > > - ctx->va_entrypoint = VAEntrypointEncSlice; > > + > > + if (opt->low_power) { > > +#if VA_CHECK_VERSION(1, 0, 0) > > + ctx->va_entrypoint = VAEntrypointEncSliceLP; > > +#else > > + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " > > + "supported with this VAAPI version.\n"); > > + return AVERROR(EINVAL); > > +#endif > > + } else { > > + ctx->va_entrypoint = VAEntrypointEncSlice; > > + } > > > > if (avctx->bit_rate > 0) { > > if (avctx->rc_max_rate == avctx->bit_rate) > > @@ -986,6 +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { > > { LEVEL("6.2", 186) }, > > #undef LEVEL > > > > + { "low_power", "Use low-power encoding mode (experimental: only > > supported " > > Do you want to copy this comment? > Actually I want to delete the comments in (), but the limitation for HEVC low power encoding still exist. It would be better to add some VAAPI attributes to query the capability/limitation etc. > > + "on some platforms, does not support all features)", > > + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS }, > > + > > { NULL }, > > }; > > > > > > How can I test this (and the following patch)? It doesn't appear to exist on > any current platform/driver. > Cannon Lake support HEVC low power encoding, do you have this platform? https:/ /github.com/intel/media-driver/ should support this feature but it isn't ready in https://github.com/intel/intel-vaapi-driver. > I don't like how this code is being copied around, but I guess it would work > for now. I'll look into doing something more sensible here. > I also dislike duplicated code. I wanted to add some common options for VAAPI encoder in FFmpeg, such as QP. However low power encoding is not available for some codecs, such as VP8, so I copied&pasted the code here. Another thing is a driver may not support both VAEntrypointEncSliceLP and VAEntrypointEncSlice, so I want to change the value range of low_power to -1, 0, 1. -1 means auto mode, FFmpeg-vaapi will select the first applicable entrypoint, what is your comment for this proposal? Thanks Haihao > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > On 06/02/18 08:17, Haihao Xiang wrote: > > > Although VAEntrypointEncSliceLP was added in old version of VAAPI, we > > > never implemented it for VAAPI H265 encoder before. so it is reasonable > > > to require VAAPI 1.0 > > > > > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > > > --- > > > libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/vaapi_encode_h265.c > > > b/libavcodec/vaapi_encode_h265.c > > > index f3b4f6c7e26..efe1148127f 100644 > > > --- a/libavcodec/vaapi_encode_h265.c > > > +++ b/libavcodec/vaapi_encode_h265.c > > > @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { > > > int aud; > > > int profile; > > > int level; > > > + int low_power; > > > } VAAPIEncodeH265Options; > > > > > > > > > @@ -914,7 +915,18 @@ static av_cold int > > > vaapi_encode_h265_init(AVCodecContext *avctx) > > > avctx->profile); > > > return AVERROR(EINVAL); > > > } > > > - ctx->va_entrypoint = VAEntrypointEncSlice; > > > + > > > + if (opt->low_power) { > > > +#if VA_CHECK_VERSION(1, 0, 0) > > > + ctx->va_entrypoint = VAEntrypointEncSliceLP; > > > +#else > > > + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " > > > + "supported with this VAAPI version.\n"); > > > + return AVERROR(EINVAL); > > > +#endif > > > + } else { > > > + ctx->va_entrypoint = VAEntrypointEncSlice; > > > + } > > > > > > if (avctx->bit_rate > 0) { > > > if (avctx->rc_max_rate == avctx->bit_rate) > > > @@ -986,6 +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { > > > { LEVEL("6.2", 186) }, > > > #undef LEVEL > > > > > > + { "low_power", "Use low-power encoding mode (experimental: only > > > supported " > > > > Do you want to copy this comment? > > > > Actually I want to delete the comments in (), but the limitation for HEVC low > power encoding still exist. It would be better to add some VAAPI attributes to > query the capability/limitation etc. > > > > > + "on some platforms, does not support all features)", > > > + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS }, > > > + > > > { NULL }, > > > }; > > > > > > > > > > How can I test this (and the following patch)? It doesn't appear to exist > > on > > any current platform/driver. > > > > Cannon Lake support HEVC low power encoding, do you have this > platform? https:/ > /github.com/intel/media-driver/ should support this feature but it isn't ready > in https://github.com/intel/intel-vaapi-driver. > > > I don't like how this code is being copied around, but I guess it would work > > for now. I'll look into doing something more sensible here. > > > > I also dislike duplicated code. I wanted to add some common options for VAAPI > encoder in FFmpeg, such as QP. However low power encoding is not available for > some codecs, such as VP8, so I copied&pasted the code here. > > Another thing is a driver may not support both VAEntrypointEncSliceLP and > VAEntrypointEncSlice, so I want to change the value range of low_power to -1, > 0, > 1. -1 means auto mode, FFmpeg-vaapi will select the first applicable > entrypoint, > what is your comment for this proposal? Hi Mark, Do you have any comment to the above proposal? https://github.com/xhaihao/media- driver only supports VAEntrypointEncSliceLP for vp9 on Cannonlake. With the above proposal, the end user doesn't need to know which entrypoint is supported by the underly driver. Thank Haihao
On 09/02/18 01:23, Xiang, Haihao wrote: > >> On 06/02/18 08:17, Haihao Xiang wrote: >>> Although VAEntrypointEncSliceLP was added in old version of VAAPI, we >>> never implemented it for VAAPI H265 encoder before. so it is reasonable >>> to require VAAPI 1.0 >>> >>> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> >>> --- >>> libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- >>> 1 file changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c >>> index f3b4f6c7e26..efe1148127f 100644 >>> --- a/libavcodec/vaapi_encode_h265.c >>> +++ b/libavcodec/vaapi_encode_h265.c >>> @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { >>> int aud; >>> int profile; >>> int level; >>> + int low_power; >>> } VAAPIEncodeH265Options; >>> >>> >>> @@ -914,7 +915,18 @@ static av_cold int >>> vaapi_encode_h265_init(AVCodecContext *avctx) >>> avctx->profile); >>> return AVERROR(EINVAL); >>> } >>> - ctx->va_entrypoint = VAEntrypointEncSlice; >>> + >>> + if (opt->low_power) { >>> +#if VA_CHECK_VERSION(1, 0, 0) >>> + ctx->va_entrypoint = VAEntrypointEncSliceLP; >>> +#else >>> + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " >>> + "supported with this VAAPI version.\n"); >>> + return AVERROR(EINVAL); >>> +#endif >>> + } else { >>> + ctx->va_entrypoint = VAEntrypointEncSlice; >>> + } >>> >>> if (avctx->bit_rate > 0) { >>> if (avctx->rc_max_rate == avctx->bit_rate) >>> @@ -986,6 +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { >>> { LEVEL("6.2", 186) }, >>> #undef LEVEL >>> >>> + { "low_power", "Use low-power encoding mode (experimental: only >>> supported " >> >> Do you want to copy this comment? >> > > Actually I want to delete the comments in (), but the limitation for HEVC low > power encoding still exist. It would be better to add some VAAPI attributes to > query the capability/limitation etc. > > >>> + "on some platforms, does not support all features)", >>> + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS }, >>> + >>> { NULL }, >>> }; >>> >>> >> >> How can I test this (and the following patch)? It doesn't appear to exist on >> any current platform/driver. >> > > Cannon Lake support HEVC low power encoding, do you have this platform? https:/ > /github.com/intel/media-driver/ should support this feature but it isn't ready > in https://github.com/intel/intel-vaapi-driver. I do not have any board with this - where can I get one? >> I don't like how this code is being copied around, but I guess it would work >> for now. I'll look into doing something more sensible here. >> > > I also dislike duplicated code. I wanted to add some common options for VAAPI > encoder in FFmpeg, such as QP. However low power encoding is not available for > some codecs, such as VP8, so I copied&pasted the code here. > > Another thing is a driver may not support both VAEntrypointEncSliceLP and > VAEntrypointEncSlice, so I want to change the value range of low_power to -1, 0, > 1. -1 means auto mode, FFmpeg-vaapi will select the first applicable entrypoint, > what is your comment for this proposal? Under what circumstances would a driver a provide low-power entrypoint but not a standard one? If some hardware only supports one encoder I would expect it to regard that encoder as the standard one. - Mark
>-----Original Message----- >From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of >Mark Thompson >Sent: Wednesday, February 14, 2018 2:58 AM >To: ffmpeg-devel@ffmpeg.org >Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h265: add support >for low-power encoding > >On 09/02/18 01:23, Xiang, Haihao wrote: >> >>> On 06/02/18 08:17, Haihao Xiang wrote: >>>> Although VAEntrypointEncSliceLP was added in old version of VAAPI, >>>> we never implemented it for VAAPI H265 encoder before. so it is >>>> reasonable to require VAAPI 1.0 >>>> >>>> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> >>>> --- >>>> libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- >>>> 1 file changed, 17 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/vaapi_encode_h265.c >>>> b/libavcodec/vaapi_encode_h265.c index f3b4f6c7e26..efe1148127f >>>> 100644 >>>> --- a/libavcodec/vaapi_encode_h265.c >>>> +++ b/libavcodec/vaapi_encode_h265.c >>>> @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { >>>> int aud; >>>> int profile; >>>> int level; >>>> + int low_power; >>>> } VAAPIEncodeH265Options; >>>> >>>> >>>> @@ -914,7 +915,18 @@ static av_cold int >>>> vaapi_encode_h265_init(AVCodecContext *avctx) >>>> avctx->profile); >>>> return AVERROR(EINVAL); >>>> } >>>> - ctx->va_entrypoint = VAEntrypointEncSlice; >>>> + >>>> + if (opt->low_power) { >>>> +#if VA_CHECK_VERSION(1, 0, 0) >>>> + ctx->va_entrypoint = VAEntrypointEncSliceLP; #else >>>> + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " >>>> + "supported with this VAAPI version.\n"); >>>> + return AVERROR(EINVAL); >>>> +#endif >>>> + } else { >>>> + ctx->va_entrypoint = VAEntrypointEncSlice; >>>> + } >>>> >>>> if (avctx->bit_rate > 0) { >>>> if (avctx->rc_max_rate == avctx->bit_rate) @@ -986,6 >>>> +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { >>>> { LEVEL("6.2", 186) }, >>>> #undef LEVEL >>>> >>>> + { "low_power", "Use low-power encoding mode (experimental: only >>>> supported " >>> >>> Do you want to copy this comment? >>> >> >> Actually I want to delete the comments in (), but the limitation for >> HEVC low power encoding still exist. It would be better to add some >> VAAPI attributes to query the capability/limitation etc. >> >> >>>> + "on some platforms, does not support all features)", >>>> + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS >>>> + }, >>>> + >>>> { NULL }, >>>> }; >>>> >>>> >>> >>> How can I test this (and the following patch)? It doesn't appear to >>> exist on any current platform/driver. >>> >> >> Cannon Lake support HEVC low power encoding, do you have this >> platform? https:/ /github.com/intel/media-driver/ should support this >> feature but it isn't ready in https://github.com/intel/intel-vaapi-driver. > >I do not have any board with this - where can I get one? > >>> I don't like how this code is being copied around, but I guess it >>> would work for now. I'll look into doing something more sensible here. >>> >> >> I also dislike duplicated code. I wanted to add some common options >> for VAAPI encoder in FFmpeg, such as QP. However low power encoding is >> not available for some codecs, such as VP8, so I copied&pasted the code >here. >> >> Another thing is a driver may not support both VAEntrypointEncSliceLP >> and VAEntrypointEncSlice, so I want to change the value range of >> low_power to -1, 0, 1. -1 means auto mode, FFmpeg-vaapi will select >> the first applicable entrypoint, what is your comment for this proposal? > >Under what circumstances would a driver a provide low-power entrypoint but >not a standard one? If some hardware only supports one encoder I would >expect it to regard that encoder as the standard one. > It is up to the driver, HW supports VAEntrypointEncSlice but the driver may not implement it. Because low-power encoding has some limitations, it will confuse users if regarding that encoder as the standard one. >- Mark >_______________________________________________ >ffmpeg-devel mailing list >ffmpeg-devel@ffmpeg.org >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c index f3b4f6c7e26..efe1148127f 100644 --- a/libavcodec/vaapi_encode_h265.c +++ b/libavcodec/vaapi_encode_h265.c @@ -65,6 +65,7 @@ typedef struct VAAPIEncodeH265Options { int aud; int profile; int level; + int low_power; } VAAPIEncodeH265Options; @@ -914,7 +915,18 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext *avctx) avctx->profile); return AVERROR(EINVAL); } - ctx->va_entrypoint = VAEntrypointEncSlice; + + if (opt->low_power) { +#if VA_CHECK_VERSION(1, 0, 0) + ctx->va_entrypoint = VAEntrypointEncSliceLP; +#else + av_log(avctx, AV_LOG_ERROR, "Low-power encoding is not " + "supported with this VAAPI version.\n"); + return AVERROR(EINVAL); +#endif + } else { + ctx->va_entrypoint = VAEntrypointEncSlice; + } if (avctx->bit_rate > 0) { if (avctx->rc_max_rate == avctx->bit_rate) @@ -986,6 +998,10 @@ static const AVOption vaapi_encode_h265_options[] = { { LEVEL("6.2", 186) }, #undef LEVEL + { "low_power", "Use low-power encoding mode (experimental: only supported " + "on some platforms, does not support all features)", + OFFSET(low_power), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, FLAGS }, + { NULL }, };
Although VAEntrypointEncSliceLP was added in old version of VAAPI, we never implemented it for VAAPI H265 encoder before. so it is reasonable to require VAAPI 1.0 Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> --- libavcodec/vaapi_encode_h265.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)