diff mbox

[FFmpeg-devel,V3] lavc/vaapi_encode_h264: Enable MB rate control.

Message ID ab046eb4-81ec-ce1d-5c58-bec1b3092710@gmail.com
State New
Headers show

Commit Message

Jun Zhao May 14, 2017, 4:26 a.m. UTC
V3: - Fix build error with old VAAPI version.
V2: - Refine the name/value type to mb_rate_control/bool.
    - Only supported GEN9+ (SKL/APL/KBL/...)
    - i965 driver default use frame-level rate control algorithm (generate the QP for each frame), 
      when enable mb_rate_control, it's will enable the MB-level RC algorithm (generate the QP for each MB). 
    - enables MB-level bitrate control that generally improves subjective visual quality, 
      but have negative impact on performance and objective visual quality metric.
From 219b06cec04a6c3f719ce80084a98b19456498e0 Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>
Date: Tue, 9 May 2017 08:19:16 +0800
Subject: [PATCH V3] lavc/vaapi_encode_h264: Enable MB rate control.

Enables macroblock-level bitrate control that generally improves
subjective visual quality. It may have a negative impact on
performance and objective visual quality metrics. Default is off
and can't compatible with Constant QP.

Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavcodec/vaapi_encode_h264.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Jun Zhao May 18, 2017, 11:43 p.m. UTC | #1
On 2017/5/14 12:26, Jun Zhao wrote:
> V3: - Fix build error with old VAAPI version.
> V2: - Refine the name/value type to mb_rate_control/bool.
>     - Only supported GEN9+ (SKL/APL/KBL/...)
>     - i965 driver default use frame-level rate control algorithm (generate the QP for each frame), 
>       when enable mb_rate_control, it's will enable the MB-level RC algorithm (generate the QP for each MB). 
>     - enables MB-level bitrate control that generally improves subjective visual quality, 
>       but have negative impact on performance and objective visual quality metric. 
> 

Ping ?
Mark Thompson May 19, 2017, 1:10 p.m. UTC | #2
On 19/05/17 00:43, Jun Zhao wrote:
> 
> On 2017/5/14 12:26, Jun Zhao wrote:
>> V3: - Fix build error with old VAAPI version.
>> V2: - Refine the name/value type to mb_rate_control/bool.
>>     - Only supported GEN9+ (SKL/APL/KBL/...)
>>     - i965 driver default use frame-level rate control algorithm (generate the QP for each frame), 
>>       when enable mb_rate_control, it's will enable the MB-level RC algorithm (generate the QP for each MB). 
>>     - enables MB-level bitrate control that generally improves subjective visual quality, 
>>       but have negative impact on performance and objective visual quality metric. 
>>
> 
> Ping ?

I fixed the default/enable/disable inconsistency in <https://github.com/01org/intel-vaapi-driver/commit/0dd6ba4e26d3abaa7e66265284ef81c746c13f20>.  Using 0/1 as default/on is probably fine, but it doesn't match the statement that it's definitely disabled if the option is zero.  (I don't mind which way around that gets fixed.)

I tested with this patch and it works fine on a Skylake GT3 (6260U), but it killed the whole machine when enabled on a Skylake GT2 (6300).  I haven't yet had chance to investigate that further (rebooting repeatedly is kindof inconvenient), but I will try to do so soon.

What platforms have you tested on?

- Mark
Mark Thompson May 20, 2017, 12:32 p.m. UTC | #3
On 19/05/17 14:10, Mark Thompson wrote:
> On 19/05/17 00:43, Jun Zhao wrote:
>>
>> On 2017/5/14 12:26, Jun Zhao wrote:
>>> V3: - Fix build error with old VAAPI version.
>>> V2: - Refine the name/value type to mb_rate_control/bool.
>>>     - Only supported GEN9+ (SKL/APL/KBL/...)
>>>     - i965 driver default use frame-level rate control algorithm (generate the QP for each frame), 
>>>       when enable mb_rate_control, it's will enable the MB-level RC algorithm (generate the QP for each MB). 
>>>     - enables MB-level bitrate control that generally improves subjective visual quality, 
>>>       but have negative impact on performance and objective visual quality metric. 
>>>
>>
>> Ping ?
> 
> I fixed the default/enable/disable inconsistency in <https://github.com/01org/intel-vaapi-driver/commit/0dd6ba4e26d3abaa7e66265284ef81c746c13f20>.  Using 0/1 as default/on is probably fine, but it doesn't match the statement that it's definitely disabled if the option is zero.  (I don't mind which way around that gets fixed.)
> 
> I tested with this patch and it works fine on a Skylake GT3 (6260U), but it killed the whole machine when enabled on a Skylake GT2 (6300).  I haven't yet had chance to investigate that further (rebooting repeatedly is kindof inconvenient), but I will try to do so soon.

This failure is completely consistent.  Reported here: <https://github.com/01org/intel-vaapi-driver/issues/172>.

You will understand that I am reluctant to commit an option which nukes my machine, so lets hold off on this patch while the Intel driver people look at it.

Thanks,

- Mark
Jun Zhao May 22, 2017, 12:05 a.m. UTC | #4
On 2017/5/20 20:32, Mark Thompson wrote:
> On 19/05/17 14:10, Mark Thompson wrote:
>> On 19/05/17 00:43, Jun Zhao wrote:
>>>
>>> On 2017/5/14 12:26, Jun Zhao wrote:
>>>> V3: - Fix build error with old VAAPI version.
>>>> V2: - Refine the name/value type to mb_rate_control/bool.
>>>>     - Only supported GEN9+ (SKL/APL/KBL/...)
>>>>     - i965 driver default use frame-level rate control algorithm (generate the QP for each frame), 
>>>>       when enable mb_rate_control, it's will enable the MB-level RC algorithm (generate the QP for each MB). 
>>>>     - enables MB-level bitrate control that generally improves subjective visual quality, 
>>>>       but have negative impact on performance and objective visual quality metric. 
>>>>
>>>
>>> Ping ?
>>
>> I fixed the default/enable/disable inconsistency in <https://github.com/01org/intel-vaapi-driver/commit/0dd6ba4e26d3abaa7e66265284ef81c746c13f20>.  Using 0/1 as default/on is probably fine, but it doesn't match the statement that it's definitely disabled if the option is zero.  (I don't mind which way around that gets fixed.)
>>
>> I tested with this patch and it works fine on a Skylake GT3 (6260U), but it killed the whole machine when enabled on a Skylake GT2 (6300).  I haven't yet had chance to investigate that further (rebooting repeatedly is kindof inconvenient), but I will try to do so soon.
> 
> This failure is completely consistent.  Reported here: <https://github.com/01org/intel-vaapi-driver/issues/172>.
> 
> You will understand that I am reluctant to commit an option which nukes my machine, so lets hold off on this patch while the Intel driver people look at it.
> 
> Thanks,
> 

Got it,Thanks , Mark. BTW, I have enabled the multi slice encoding for VAAPI AVC/HEVC, maybe I can send a early version for code review.

> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 92e29554ed..44223ada55 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -168,6 +168,7 @@  typedef struct VAAPIEncodeH264Options {
     int qp;
     int quality;
     int low_power;
+    int mb_rate_control;
 } VAAPIEncodeH264Options;
 
 
@@ -1133,8 +1134,22 @@  static av_cold int vaapi_encode_h264_configure(AVCodecContext *avctx)
         priv->fixed_qp_p   = 26;
         priv->fixed_qp_b   = 26;
 
-        av_log(avctx, AV_LOG_DEBUG, "Using %s-bitrate = %"PRId64" bps.\n",
+        if (opt->mb_rate_control) {
+#if VA_CHECK_VERSION(0, 39, 2)
+            ctx->rc_params.rc.rc_flags.bits.mb_rate_control = opt->mb_rate_control;
+#else
+            av_log(avctx, AV_LOG_WARNING, "The MB rate control option is not "
+                   "supported with this VAAPI version.\n");
+#endif
+        }
+
+        av_log(avctx, AV_LOG_DEBUG, "Using %s-bitrate %s MB rate control = %"PRId64" bps.\n",
                ctx->va_rc_mode == VA_RC_CBR ? "constant" : "variable",
+#if VA_CHECK_VERSION(0, 39, 2)
+               opt->mb_rate_control ? "with" : "without",
+#else
+               "without",
+#endif
                avctx->bit_rate);
 
     } else {
@@ -1283,6 +1298,8 @@  static const AVOption vaapi_encode_h264_options[] = {
     { "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 },
+    { "mb_rate_control", "MB level bitrate control (only supported on GEN9+)",
+      OFFSET(mb_rate_control), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS, "mb_rate_control" },
     { NULL },
 };