Re: [libav-devel] [PATCH] vaapi_encode: Move quality option in common code
On 12/05/17 16:59, Anton Khirnov wrote: > Quoting Mark Thompson (2017-04-30 20:43:34) >> Use AVCodecContext.compression_level rather than a private option, >> replacing the H.264-specific quality option (which stays only for >> compatibility). >> >> This now works with the H.265 encoder in the i965 driver, as well as >> the existing cases with the H.264 encoder. >> --- >> doc/encoders.texi | 9 ++--- >> libavcodec/vaapi_encode.c | 33 + >> libavcodec/vaapi_encode.h | 6 ++ >> libavcodec/vaapi_encode_h264.c | 25 ++--- >> 4 files changed, 47 insertions(+), 26 deletions(-) >> >> diff --git a/doc/encoders.texi b/doc/encoders.texi >> index 7cebe39c1..c369e03bf 100644 >> --- a/doc/encoders.texi >> +++ b/doc/encoders.texi >> @@ -948,7 +948,13 @@ The following standard libavcodec options are used: >> @item >> @option{rc_init_occupancy} / @option{rc_initial_buffer_occupancy} >> @item >> +@option{compression_level} >> + >> +Speed / quality tradeoff: higher values are faster / worse quality. >> +@item >> @option{q} / @option{global_quality} >> + >> +Size / quality tradeoff: higher values are smaller / worse quality. >> @item >> @option{qmin} >> (only: @option{qmax} is not supported) >> @@ -969,9 +975,6 @@ The following standard libavcodec options are used: >> @option{level} sets the value of @emph{level_idc}. >> >> @table @option >> -@item quality >> -Set the local encoding quality/speed tradeoff (range 1-8, higher values are >> faster; not all >> -systems implement all levels). >> @item low_power >> Use low-power encoding mode. >> @end table >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c >> index 620518419..5e5177a5d 100644 >> --- a/libavcodec/vaapi_encode.c >> +++ b/libavcodec/vaapi_encode.c >> @@ -1423,6 +1423,39 @@ av_cold int ff_vaapi_encode_init(AVCodecContext >> *avctx) >> goto fail; >> } >> >> +if (avctx->compression_level >= 0) { >> +#if VA_CHECK_VERSION(0, 36, 0) >> +VAConfigAttrib attr = { VAConfigAttribEncQualityRange }; >> + >> +vas = vaGetConfigAttributes(ctx->hwctx->display, >> +ctx->va_profile, >> +ctx->va_entrypoint, >> +, 1); >> +if (vas != VA_STATUS_SUCCESS) { >> +av_log(avctx, AV_LOG_WARNING, "Failed to query quality " >> + "attribute: will use default compression level.\n"); >> +} else if (avctx->compression_level > attr.value) { >> +av_log(avctx, AV_LOG_ERROR, "Invalid compression level: " >> + "valid range is 0-%d.\n", attr.value); >> +err = AVERROR(EINVAL); > > It looks a bit strange to me that this branch fails, while the case > where vaapi doesn't support this option continues. Yeah; changed to warn and bound above in this case. > Otherwise looks fine. And pushed (somewhat belatedly). Thanks, - Mark ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] vaapi_encode: Move quality option in common code
Quoting Mark Thompson (2017-04-30 20:43:34) > Use AVCodecContext.compression_level rather than a private option, > replacing the H.264-specific quality option (which stays only for > compatibility). > > This now works with the H.265 encoder in the i965 driver, as well as > the existing cases with the H.264 encoder. > --- > doc/encoders.texi | 9 ++--- > libavcodec/vaapi_encode.c | 33 + > libavcodec/vaapi_encode.h | 6 ++ > libavcodec/vaapi_encode_h264.c | 25 ++--- > 4 files changed, 47 insertions(+), 26 deletions(-) > > diff --git a/doc/encoders.texi b/doc/encoders.texi > index 7cebe39c1..c369e03bf 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -948,7 +948,13 @@ The following standard libavcodec options are used: > @item > @option{rc_init_occupancy} / @option{rc_initial_buffer_occupancy} > @item > +@option{compression_level} > + > +Speed / quality tradeoff: higher values are faster / worse quality. > +@item > @option{q} / @option{global_quality} > + > +Size / quality tradeoff: higher values are smaller / worse quality. > @item > @option{qmin} > (only: @option{qmax} is not supported) > @@ -969,9 +975,6 @@ The following standard libavcodec options are used: > @option{level} sets the value of @emph{level_idc}. > > @table @option > -@item quality > -Set the local encoding quality/speed tradeoff (range 1-8, higher values are > faster; not all > -systems implement all levels). > @item low_power > Use low-power encoding mode. > @end table > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 620518419..5e5177a5d 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -1423,6 +1423,39 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx) > goto fail; > } > > +if (avctx->compression_level >= 0) { > +#if VA_CHECK_VERSION(0, 36, 0) > +VAConfigAttrib attr = { VAConfigAttribEncQualityRange }; > + > +vas = vaGetConfigAttributes(ctx->hwctx->display, > +ctx->va_profile, > +ctx->va_entrypoint, > +, 1); > +if (vas != VA_STATUS_SUCCESS) { > +av_log(avctx, AV_LOG_WARNING, "Failed to query quality " > + "attribute: will use default compression level.\n"); > +} else if (avctx->compression_level > attr.value) { > +av_log(avctx, AV_LOG_ERROR, "Invalid compression level: " > + "valid range is 0-%d.\n", attr.value); > +err = AVERROR(EINVAL); It looks a bit strange to me that this branch fails, while the case where vaapi doesn't support this option continues. Otherwise looks fine. -- Anton Khirnov ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] vaapi_encode: Move quality option in common code
Use AVCodecContext.compression_level rather than a private option, replacing the H.264-specific quality option (which stays only for compatibility). This now works with the H.265 encoder in the i965 driver, as well as the existing cases with the H.264 encoder. --- doc/encoders.texi | 9 ++--- libavcodec/vaapi_encode.c | 33 + libavcodec/vaapi_encode.h | 6 ++ libavcodec/vaapi_encode_h264.c | 25 ++--- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/doc/encoders.texi b/doc/encoders.texi index 7cebe39c1..c369e03bf 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -948,7 +948,13 @@ The following standard libavcodec options are used: @item @option{rc_init_occupancy} / @option{rc_initial_buffer_occupancy} @item +@option{compression_level} + +Speed / quality tradeoff: higher values are faster / worse quality. +@item @option{q} / @option{global_quality} + +Size / quality tradeoff: higher values are smaller / worse quality. @item @option{qmin} (only: @option{qmax} is not supported) @@ -969,9 +975,6 @@ The following standard libavcodec options are used: @option{level} sets the value of @emph{level_idc}. @table @option -@item quality -Set the local encoding quality/speed tradeoff (range 1-8, higher values are faster; not all -systems implement all levels). @item low_power Use low-power encoding mode. @end table diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 620518419..5e5177a5d 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -1423,6 +1423,39 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx) goto fail; } +if (avctx->compression_level >= 0) { +#if VA_CHECK_VERSION(0, 36, 0) +VAConfigAttrib attr = { VAConfigAttribEncQualityRange }; + +vas = vaGetConfigAttributes(ctx->hwctx->display, +ctx->va_profile, +ctx->va_entrypoint, +, 1); +if (vas != VA_STATUS_SUCCESS) { +av_log(avctx, AV_LOG_WARNING, "Failed to query quality " + "attribute: will use default compression level.\n"); +} else if (avctx->compression_level > attr.value) { +av_log(avctx, AV_LOG_ERROR, "Invalid compression level: " + "valid range is 0-%d.\n", attr.value); +err = AVERROR(EINVAL); +goto fail; +} else { +ctx->quality_params.misc.type = +VAEncMiscParameterTypeQualityLevel; +ctx->quality_params.quality.quality_level = +avctx->compression_level; + +ctx->global_params[ctx->nb_global_params] = +>quality_params.misc; +ctx->global_params_size[ctx->nb_global_params++] = +sizeof(ctx->quality_params); +} +#else +av_log(avctx, AV_LOG_WARNING, "The encode compression level " + "option is not supported with this VAAPI version.\n"); +#endif +} + ctx->input_order = 0; ctx->output_delay = avctx->max_b_frames; ctx->decode_delay = 1; diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h index fc6236514..1b0fed80e 100644 --- a/libavcodec/vaapi_encode.h +++ b/libavcodec/vaapi_encode.h @@ -159,6 +159,12 @@ typedef struct VAAPIEncodeContext { VAEncMiscParameterBuffer misc; VAEncMiscParameterFrameRate fr; } fr_params; +#if VA_CHECK_VERSION(0, 36, 0) +struct { +VAEncMiscParameterBuffer misc; +VAEncMiscParameterBufferQualityLevel quality; +} quality_params; +#endif // Per-sequence parameter structure (VAEncSequenceParameterBuffer*). void *codec_sequence_params; diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index 0c3ac3441..9013207f9 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -154,14 +154,6 @@ typedef struct VAAPIEncodeH264Context { // Rate control configuration. int send_timing_sei; - -#if VA_CHECK_VERSION(0, 36, 0) -// Speed-quality tradeoff setting. -struct { -VAEncMiscParameterBuffer misc; -VAEncMiscParameterBufferQualityLevel quality; -} quality_params; -#endif } VAAPIEncodeH264Context; typedef struct VAAPIEncodeH264Options { @@ -1141,21 +1133,8 @@ static av_cold int vaapi_encode_h264_configure(AVCodecContext *avctx) av_assert0(0 && "Invalid RC mode."); } -if (opt->quality > 0) { -#if VA_CHECK_VERSION(0, 36, 0) -priv->quality_params.misc.type = -VAEncMiscParameterTypeQualityLevel; -priv->quality_params.quality.quality_level = opt->quality; - -ctx->global_params[ctx->nb_global_params] = ->quality_params.misc; -ctx->global_params_size[ctx->nb_global_params++] = -sizeof(priv->quality_params); -#else -