Fixes part of merge regression from adding arf parameters.

From Change  Ibf0c30b72074b3f71918ab278ccccc02a95a70a0
There is still an issue relating to one animated test clip with repeat
patterns where this change effectively increase the default  maximum
arf interval by +1. This can be examined seperately.

Change-Id: Idd01d5480fc45202d8a059a0c3afc0997cc5bdd1

4 files changed, 32 insertions, 28 deletions

diff --git a/test/vp9_arf_freq_test.cc b/test/vp9_arf_freq_test.cc
index 92c236f5b..07968bcd6 100644
--- a/test/vp9_arf_freq_test.cc
+++ b/test/vp9_arf_freq_test.cc
@@ -21,8 +21,8 @@ namespace {
 const unsigned int kFrames = 100;
 const int kBitrate = 500;
 
-#define ARF_NOT_SEEN   1000001
-#define ARF_SEEN_ONCE  1000000
+#define ARF_NOT_SEEN               1000001
+#define ARF_SEEN_ONCE              1000000
 
 typedef struct {
   const char *filename;
@@ -108,7 +108,7 @@ class ArfFreqTest
   }
 
   virtual void BeginPassHook(unsigned int) {
-    min_arf_ = ARF_NOT_SEEN;
+    min_run_ = ARF_NOT_SEEN;
     run_of_visible_frames_ = 0;
   }
 
@@ -137,15 +137,15 @@ class ArfFreqTest
     if (frames == 1) {
       run_of_visible_frames_++;
     } else if (frames == 2) {
-      if (min_arf_ == ARF_NOT_SEEN) {
-        min_arf_ = ARF_SEEN_ONCE;
-      } else if (min_arf_ == ARF_SEEN_ONCE ||
-                 run_of_visible_frames_ < min_arf_) {
-        min_arf_ = run_of_visible_frames_;
+      if (min_run_ == ARF_NOT_SEEN) {
+        min_run_ = ARF_SEEN_ONCE;
+      } else if (min_run_ == ARF_SEEN_ONCE ||
+                 run_of_visible_frames_ < min_run_) {
+        min_run_ = run_of_visible_frames_;
       }
       run_of_visible_frames_ = 1;
     } else {
-      min_arf_ = 0;
+      min_run_ = 0;
       run_of_visible_frames_ = 1;
     }
   }
@@ -166,8 +166,8 @@ class ArfFreqTest
     }
   }
 
-  int GetMinArfDistance() const {
-    return min_arf_;
+  int GetMinVisibleRun() const {
+    return min_run_;
   }
 
   int GetMinArfDistanceRequested() const {
@@ -185,7 +185,7 @@ class ArfFreqTest
 
  private:
   int min_arf_requested_;
-  int min_arf_;
+  int min_run_;
   int run_of_visible_frames_;
 };
 
@@ -214,9 +214,10 @@ TEST_P(ArfFreqTest, MinArfFreqTest) {
   }
 
   ASSERT_NO_FATAL_FAILURE(RunLoop(video));
-  const int min_arf_dist = GetMinArfDistance();
+  const int min_run = GetMinVisibleRun();
   const int min_arf_dist_requested = GetMinArfDistanceRequested();
-  if (min_arf_dist != ARF_NOT_SEEN && min_arf_dist != ARF_SEEN_ONCE) {
+  if (min_run != ARF_NOT_SEEN && min_run != ARF_SEEN_ONCE) {
+    const int min_arf_dist = min_run + 1;
     EXPECT_GE(min_arf_dist, min_arf_dist_requested);
   }
   delete(video);
diff --git a/vp9/encoder/vp9_firstpass.c b/vp9/encoder/vp9_firstpass.c
index 61279f872..5caf2cbd8 100644
--- a/vp9/encoder/vp9_firstpass.c
+++ b/vp9/encoder/vp9_firstpass.c
@@ -1883,7 +1883,7 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
   double gf_group_error_left;
   int gf_arf_bits;
   const int is_key_frame = frame_is_intra_only(cm);
-  const int kf_or_arf_active = is_key_frame || rc->source_alt_ref_active;
+  const int arf_active_or_kf = is_key_frame || rc->source_alt_ref_active;
 
   // Reset the GF group data structures unless this is a key
   // frame in which case it will already have been done.
@@ -1903,7 +1903,7 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
 
   // If this is a key frame or the overlay from a previous arf then
   // the error score / cost of this frame has already been accounted for.
-  if (is_key_frame || rc->source_alt_ref_active) {
+  if (arf_active_or_kf) {
     gf_group_err -= gf_first_frame_err;
 #if GROUP_ADAPTIVE_MAXQ
     gf_group_raw_error -= this_frame->coded_error;
@@ -1936,7 +1936,7 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
       // bits to spare and are better with a smaller interval and smaller boost.
       // At high Q when there are few bits to spare we are better with a longer
       // interval to spread the cost of the GF.
-      active_max_gf_interval = rc->max_gf_interval - 4 + MIN(4, (int_lbq / 6));
+      active_max_gf_interval = 12 + MIN(4, (int_lbq / 6));
       if (active_max_gf_interval < active_min_gf_interval)
         active_max_gf_interval = active_min_gf_interval;
 
@@ -2001,11 +2001,11 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
     // Break out conditions.
     if (
       // Break at active_max_gf_interval unless almost totally static.
-      ((i >= active_max_gf_interval + kf_or_arf_active) &&
-       (zero_motion_accumulator < 0.995)) ||
+      (i >= (active_max_gf_interval + arf_active_or_kf) &&
+            zero_motion_accumulator < 0.995) ||
       (
         // Don't break out with a very short interval.
-        (i >= active_min_gf_interval + kf_or_arf_active) &&
+        (i >= active_min_gf_interval + arf_active_or_kf) &&
         (!flash_detected) &&
         ((mv_ratio_accumulator > mv_ratio_accumulator_thresh) ||
          (abs_mv_in_out_accumulator > 3.0) ||
@@ -2043,10 +2043,7 @@ static void define_gf_group(VP9_COMP *cpi, FIRSTPASS_STATS *this_frame) {
   }
 
   // Set the interval until the next gf.
-  if (is_key_frame || rc->source_alt_ref_pending)
-    rc->baseline_gf_interval = i - 1;
-  else
-    rc->baseline_gf_interval = i;
+  rc->baseline_gf_interval = i - (is_key_frame || rc->source_alt_ref_pending);
 
   // Only encode alt reference frame in temporal base layer. So
   // baseline_gf_interval should be multiple of a temporal layer group
diff --git a/vp9/encoder/vp9_ratectrl.c b/vp9/encoder/vp9_ratectrl.c
index be09bca0c..926afe1c5 100644
--- a/vp9/encoder/vp9_ratectrl.c
+++ b/vp9/encoder/vp9_ratectrl.c
@@ -281,11 +281,14 @@ int vp9_rc_get_default_min_gf_interval(
   // Assume we do not need any constraint lower than 4K 20 fps
   static const double factor_safe = 3840 * 2160 * 20.0;
   const double factor = width * height * framerate;
+  const double default_interval =
+      MIN(MAX_GF_INTERVAL, MAX(MIN_GF_INTERVAL, (int)(framerate * 0.125)));
 
   if (factor <= factor_safe)
-    return MIN_GF_INTERVAL;
+    return (int)default_interval;
   else
-    return (int)(MIN_GF_INTERVAL * factor / factor_safe + 0.5);
+    return (int)MAX(default_interval,
+                    (int)(MIN_GF_INTERVAL * factor / factor_safe + 0.5));
   // Note this logic makes:
   // 4K24: 5
   // 4K30: 6
@@ -294,6 +297,7 @@ int vp9_rc_get_default_min_gf_interval(
 
 int vp9_rc_get_default_max_gf_interval(double framerate, int min_gf_interval) {
   int interval = MIN(MAX_GF_INTERVAL, (int)(framerate * 0.75));
+  interval += (interval & 0x01);  // Round to even value
   return MAX(interval, min_gf_interval);
 }
 
@@ -1693,7 +1697,6 @@ void vp9_rc_set_gf_interval_range(const VP9_COMP *const cpi,
   if (rc->max_gf_interval == 0)
     rc->max_gf_interval = vp9_rc_get_default_max_gf_interval(
         cpi->framerate, rc->min_gf_interval);
-  rc->max_gf_interval += (rc->max_gf_interval & 0x01);
 
   // Extended interval for genuinely static scenes
   rc->static_scene_max_gf_interval = MAX_LAG_BUFFERS * 2;
diff --git a/vp9/vp9_cx_iface.c b/vp9/vp9_cx_iface.c
index d2d9288ee..f155b9aef 100644
--- a/vp9/vp9_cx_iface.c
+++ b/vp9/vp9_cx_iface.c
@@ -173,9 +173,12 @@ static vpx_codec_err_t validate_config(vpx_codec_alg_priv_t *ctx,
   RANGE_CHECK(cfg,        g_pass,         VPX_RC_ONE_PASS, VPX_RC_LAST_PASS);
   RANGE_CHECK(extra_cfg, min_gf_interval, 0, (MAX_LAG_BUFFERS - 1));
   RANGE_CHECK(extra_cfg, max_gf_interval, 0, (MAX_LAG_BUFFERS - 1));
+  if (extra_cfg->max_gf_interval > 0) {
+    RANGE_CHECK(extra_cfg, max_gf_interval, 2, (MAX_LAG_BUFFERS - 1));
+  }
   if (extra_cfg->min_gf_interval > 0 && extra_cfg->max_gf_interval > 0) {
     RANGE_CHECK(extra_cfg, max_gf_interval, extra_cfg->min_gf_interval,
-                (MAX_LAG_BUFFERS - 1));
+      (MAX_LAG_BUFFERS - 1));
   }
 
   if (cfg->rc_resize_allowed == 1) {