diff mbox

[FFmpeg-devel,v2,06/11] vaapi_encode_vp9: Enable support for more RC modes

Message ID 20190127234707.27689-6-sw@jkqxz.net
State Superseded
Headers show

Commit Message

Mark Thompson Jan. 27, 2019, 11:47 p.m. UTC 
---
 libavcodec/vaapi_encode_vp9.c | 41 +++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 16 deletions(-)

Comments

Carl Eugen Hoyos Feb. 5, 2019, 1:25 p.m. UTC | #1 
2019-01-28 0:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> ---
>  libavcodec/vaapi_encode_vp9.c | 41 +++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
> index 97142dcc49..f89fd0d07a 100644
> --- a/libavcodec/vaapi_encode_vp9.c
> +++ b/libavcodec/vaapi_encode_vp9.c
> @@ -178,23 +178,29 @@ static int
> vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
>
>  static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
>  {
> +    VAAPIEncodeContext     *ctx = avctx->priv_data;
>      VAAPIEncodeVP9Context *priv = avctx->priv_data;
>
> -    priv->q_idx_p = av_clip(avctx->global_quality, 0, VP9_MAX_QUANT);
> -    if (avctx->i_quant_factor > 0.0)
> -        priv->q_idx_idr = av_clip((avctx->global_quality *
> -                                   avctx->i_quant_factor +
> -                                   avctx->i_quant_offset) + 0.5,
> -                                  0, VP9_MAX_QUANT);
> -    else
> -        priv->q_idx_idr = priv->q_idx_p;
> -    if (avctx->b_quant_factor > 0.0)
> -        priv->q_idx_b = av_clip((avctx->global_quality *
> -                                 avctx->b_quant_factor +
> -                                 avctx->b_quant_offset) + 0.5,
> -                                0, VP9_MAX_QUANT);
> -    else
> -        priv->q_idx_b = priv->q_idx_p;
> +    if (ctx->rc_mode->quality) {
> +        priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
> +        if (avctx->i_quant_factor > 0.0)
> +            priv->q_idx_idr =
> +                av_clip((avctx->i_quant_factor * priv->q_idx_p  +
> +                         avctx->i_quant_offset) + 0.5,
> +                        0, VP9_MAX_QUANT);
> +        else
> +            priv->q_idx_idr = priv->q_idx_p;
> +        if (avctx->b_quant_factor > 0.0)
> +            priv->q_idx_b =
> +                av_clip((avctx->b_quant_factor * priv->q_idx_p  +
> +                         avctx->b_quant_offset) + 0.5,
> +                        0, VP9_MAX_QUANT);
> +        else
> +            priv->q_idx_b = priv->q_idx_p;

I will not work on this code, so I shouldn't care but this
is an exceptional example for an unreadable patch.

Carl Eugen
Mark Thompson Feb. 10, 2019, 6:13 p.m. UTC | #2 
On 05/02/2019 13:25, Carl Eugen Hoyos wrote:
> 2019-01-28 0:47 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>> ---
>>  libavcodec/vaapi_encode_vp9.c | 41 +++++++++++++++++++++--------------
>>  1 file changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
>> index 97142dcc49..f89fd0d07a 100644
>> --- a/libavcodec/vaapi_encode_vp9.c
>> +++ b/libavcodec/vaapi_encode_vp9.c
>> @@ -178,23 +178,29 @@ static int
>> vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
>>
>>  static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
>>  {
>> +    VAAPIEncodeContext     *ctx = avctx->priv_data;
>>      VAAPIEncodeVP9Context *priv = avctx->priv_data;
>>
>> -    priv->q_idx_p = av_clip(avctx->global_quality, 0, VP9_MAX_QUANT);
>> -    if (avctx->i_quant_factor > 0.0)
>> -        priv->q_idx_idr = av_clip((avctx->global_quality *
>> -                                   avctx->i_quant_factor +
>> -                                   avctx->i_quant_offset) + 0.5,
>> -                                  0, VP9_MAX_QUANT);
>> -    else
>> -        priv->q_idx_idr = priv->q_idx_p;
>> -    if (avctx->b_quant_factor > 0.0)
>> -        priv->q_idx_b = av_clip((avctx->global_quality *
>> -                                 avctx->b_quant_factor +
>> -                                 avctx->b_quant_offset) + 0.5,
>> -                                0, VP9_MAX_QUANT);
>> -    else
>> -        priv->q_idx_b = priv->q_idx_p;
>> +    if (ctx->rc_mode->quality) {
>> +        priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
>> +        if (avctx->i_quant_factor > 0.0)
>> +            priv->q_idx_idr =
>> +                av_clip((avctx->i_quant_factor * priv->q_idx_p  +
>> +                         avctx->i_quant_offset) + 0.5,
>> +                        0, VP9_MAX_QUANT);
>> +        else
>> +            priv->q_idx_idr = priv->q_idx_p;
>> +        if (avctx->b_quant_factor > 0.0)
>> +            priv->q_idx_b =
>> +                av_clip((avctx->b_quant_factor * priv->q_idx_p  +
>> +                         avctx->b_quant_offset) + 0.5,
>> +                        0, VP9_MAX_QUANT);
>> +        else
>> +            priv->q_idx_b = priv->q_idx_p;
> 
> I will not work on this code, so I shouldn't care but this
> is an exceptional example for an unreadable patch.

Yeah, that's probably fair.  I've split this into functional and cosmetic parts in a new version.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index 97142dcc49..f89fd0d07a 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -178,23 +178,29 @@  static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
 
 static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
 {
+    VAAPIEncodeContext     *ctx = avctx->priv_data;
     VAAPIEncodeVP9Context *priv = avctx->priv_data;
 
-    priv->q_idx_p = av_clip(avctx->global_quality, 0, VP9_MAX_QUANT);
-    if (avctx->i_quant_factor > 0.0)
-        priv->q_idx_idr = av_clip((avctx->global_quality *
-                                   avctx->i_quant_factor +
-                                   avctx->i_quant_offset) + 0.5,
-                                  0, VP9_MAX_QUANT);
-    else
-        priv->q_idx_idr = priv->q_idx_p;
-    if (avctx->b_quant_factor > 0.0)
-        priv->q_idx_b = av_clip((avctx->global_quality *
-                                 avctx->b_quant_factor +
-                                 avctx->b_quant_offset) + 0.5,
-                                0, VP9_MAX_QUANT);
-    else
-        priv->q_idx_b = priv->q_idx_p;
+    if (ctx->rc_mode->quality) {
+        priv->q_idx_p = av_clip(ctx->rc_quality, 0, VP9_MAX_QUANT);
+        if (avctx->i_quant_factor > 0.0)
+            priv->q_idx_idr =
+                av_clip((avctx->i_quant_factor * priv->q_idx_p  +
+                         avctx->i_quant_offset) + 0.5,
+                        0, VP9_MAX_QUANT);
+        else
+            priv->q_idx_idr = priv->q_idx_p;
+        if (avctx->b_quant_factor > 0.0)
+            priv->q_idx_b =
+                av_clip((avctx->b_quant_factor * priv->q_idx_p  +
+                         avctx->b_quant_offset) + 0.5,
+                        0, VP9_MAX_QUANT);
+        else
+            priv->q_idx_b = priv->q_idx_p;
+    } else {
+        // Arbitrary value.
+        priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100;
+    }
 
     return 0;
 }
@@ -211,6 +217,8 @@  static const VAAPIEncodeType vaapi_encode_type_vp9 = {
     .flags                 = FLAG_B_PICTURES |
                              FLAG_B_PICTURE_REFERENCES,
 
+    .default_quality       = 100,
+
     .picture_priv_data_size = sizeof(VAAPIEncodeVP9Picture),
 
     .configure             = &vaapi_encode_vp9_configure,
@@ -244,6 +252,8 @@  static av_cold int vaapi_encode_vp9_init(AVCodecContext *avctx)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
 static const AVOption vaapi_encode_vp9_options[] = {
     VAAPI_ENCODE_COMMON_OPTIONS,
+    VAAPI_ENCODE_RC_OPTIONS,
+
     { "loop_filter_level", "Loop filter level",
       OFFSET(loop_filter_level), AV_OPT_TYPE_INT, { .i64 = 16 }, 0, 63, FLAGS },
     { "loop_filter_sharpness", "Loop filter sharpness",
@@ -255,7 +265,6 @@  static const AVCodecDefault vaapi_encode_vp9_defaults[] = {
     { "b",              "0"   },
     { "bf",             "0"   },
     { "g",              "250" },
-    { "global_quality", "100" },
     { "qmin",           "-1"  },
     { "qmax",           "-1"  },
     { NULL },