Re: [libav-devel] [PATCH] vaapi_encode: Move quality option in common code

2017-08-06 Thread Mark Thompson
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

2017-05-12 Thread Anton Khirnov
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