Revert "Merge "Adapt GOP size threshold to the allowed layer depth""
authorPaul Wilkins <paulwilkins@google.com>
Fri, 28 Sep 2018 12:17:31 +0000 (13:17 +0100)
committerPaul Wilkins <paulwilkins@google.com>
Fri, 28 Sep 2018 12:17:31 +0000 (13:17 +0100)
This reverts commit 5efde3914f1a85061e51bc8d96984216e976e90c, reversing
changes made to 3a291593726a3f0eff5e59983e84de20d0bf294c.

This is badly broken and may help somewhat for multi-layer but is hurting
massively in single layer encodes.

I ran this through this morning and while it often helps in SSIM it is badly down
for global PSNR and PSNR-HVS with some clips down by 35-40%. This is in line
with previous experiments where I have found that a bigger boost helps SSIM
but hurts PSNR and PSNR HVS.

I was also working on changes to the I factor that gave some improvements
in single layer though these were based upon the active Q mostly. I also have
looked at a bug for the first group where int_lbq is not properly defined and
will submit an interim patch for this while I look for a better solution.

In the meantime I think we should revert this.

The (Global PSNR, SSIM, PSNR-HVS) for the patch as is in my runs for
single layer vs a couple of days ago seem to be (-ve is better).

Low res 0.346, -1.475, 0.239
mid res  1.581, -1.300, 1.731 (worst result down by 30-40% in psnr)
hdres 0.665, -0.712, 1.043 (worst result down by 17-19% in psnr)
NF2k 0.927, 0.111, 1.3220 (Worst result down by 5-7% in psnr)

Change-Id: I55952b71b8cfc5a84484b3b659c5f8a530f3a755

vp9/encoder/vp9_firstpass.c

index b311a47..985c2fa 100644 (file)
@@ -2482,8 +2482,6 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
   const int is_key_frame = frame_is_intra_only(cm);
   const int arf_active_or_kf = is_key_frame || rc->source_alt_ref_active;
 
-  int gop_intra_factor = 0;
-
   // Reset the GF group data structures unless this is a key
   // frame in which case it will already have been done.
   if (is_key_frame == 0) {
@@ -2536,12 +2534,6 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
     // interval to spread the cost of the GF.
     active_max_gf_interval = 12 + arf_active_or_kf + VPXMIN(4, (int_lbq / 6));
 
-    // TODO(jingning, paulwilkins): Temporary solution to work around
-    // multi-layer ARF rate control issues at extremely low bit-rate cases.
-    // We would deprecate this soon.
-    if (cpi->multi_layer_arf && is_key_frame)
-      active_max_gf_interval = 7 + arf_active_or_kf + VPXMIN(4, (int_lbq / 6));
-
     // We have: active_min_gf_interval <=
     // rc->max_gf_interval + arf_active_or_kf.
     if (active_max_gf_interval < active_min_gf_interval) {
@@ -2557,11 +2549,6 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
       active_max_gf_interval = rc->frames_to_key / 2;
   }
 
-  // Adapt the intra_error factor to active_max_gf_interval limit.
-  for (i = active_max_gf_interval; i > 0; i >>= 1) ++gop_intra_factor;
-
-  gop_intra_factor = VPXMIN(MAX_ARF_LAYERS, gop_intra_factor);
-
   i = 0;
   while (i < rc->static_scene_max_gf_interval && i < rc->frames_to_key) {
     ++i;
@@ -2638,7 +2625,7 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
             (!flash_detected) &&
             ((mv_ratio_accumulator > mv_ratio_accumulator_thresh) ||
              (abs_mv_in_out_accumulator > abs_mv_in_out_thresh) ||
-             (sr_accumulator > gop_intra_factor * next_frame.intra_error)))) {
+             (sr_accumulator > next_frame.intra_error)))) {
       break;
     }