From 881c8ec816eb0790613dc674ee8f7a0e102e1835 Mon Sep 17 00:00:00 2001
From: Marco Paniconi <marpan@google.com>
Date: Wed, 14 Mar 2018 17:00:27 -0700
Subject: [PATCH] vp9-svc: Bugfix to dyanmic enabling/disabling of layers.

Fix a bug when middle and top spatial layer are skip encoded
(disabled) and then re-enabled again, during the sequence.

Issue is that pending_frame_count in the packing may
be incremented on middle layer, even though that layer is skipped
(not encoded and hence zero size). Fix is to add size check.

Modified existing unitest to reproduce the issue.

Change-Id: I86d806a112d468e06b04fbf7c46ae07db9e0ad93
---
 test/datarate_test.cc | 38 ++++++++++++++++++++++++++------------
 vp9/vp9_cx_iface.c    |  2 +-
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/test/datarate_test.cc b/test/datarate_test.cc
index 70be44ec4..4ab8aa382 100644
--- a/test/datarate_test.cc
+++ b/test/datarate_test.cc
@@ -1364,6 +1364,8 @@ class DatarateOnePassCbrSvc
     dynamic_drop_layer_ = false;
     change_bitrate_ = false;
     last_pts_ref_ = 0;
+    middle_bitrate_ = 0;
+    top_bitrate_ = 0;
   }
   virtual void BeginPassHook(unsigned int /*pass*/) {}
 
@@ -1497,16 +1499,26 @@ class DatarateOnePassCbrSvc
     }
 
     if (dynamic_drop_layer_) {
-      if (video->frame() == 100) {
-        // Change layer bitrates to set top layer to 0. This will trigger skip
-        // encoding/dropping of top spatial layer.
-        cfg_.rc_target_bitrate -= cfg_.layer_target_bitrate[2];
+      if (video->frame() == 50) {
+        // Change layer bitrates to set top layers to 0. This will trigger skip
+        // encoding/dropping of top two spatial layers.
+        cfg_.rc_target_bitrate -=
+            (cfg_.layer_target_bitrate[1] + cfg_.layer_target_bitrate[2]);
+        middle_bitrate_ = cfg_.layer_target_bitrate[1];
+        top_bitrate_ = cfg_.layer_target_bitrate[2];
+        cfg_.layer_target_bitrate[1] = 0;
         cfg_.layer_target_bitrate[2] = 0;
         encoder->Config(&cfg_);
-      } else if (video->frame() == 300) {
-        // Change layer bitrate on top layer to non-zero to start encoding it
-        // again.
-        cfg_.layer_target_bitrate[2] = 500;
+      } else if (video->frame() == 100) {
+        // Change layer bitrate on second layer to non-zero to start
+        // encoding it again.
+        cfg_.layer_target_bitrate[1] = middle_bitrate_;
+        cfg_.rc_target_bitrate += cfg_.layer_target_bitrate[1];
+        encoder->Config(&cfg_);
+      } else if (video->frame() == 200) {
+        // Change layer bitrate on top layer to non-zero to start
+        // encoding it again.
+        cfg_.layer_target_bitrate[2] = top_bitrate_;
         cfg_.rc_target_bitrate += cfg_.layer_target_bitrate[2];
         encoder->Config(&cfg_);
       }
@@ -1649,6 +1661,8 @@ class DatarateOnePassCbrSvc
   int update_pattern_;
   bool change_bitrate_;
   vpx_codec_pts_t last_pts_ref_;
+  int middle_bitrate_;
+  int top_bitrate_;
 };
 
 // Check basic rate targeting for 1 pass CBR SVC: 2 spatial layers and 1
@@ -2045,10 +2059,10 @@ TEST_P(DatarateOnePassCbrSvc, OnePassCbrSvc3SL2TLDynamicPatternChange) {
 }
 
 // Check basic rate targeting for 1 pass CBR SVC with 3 spatial layers and on
-// the fly switching to 2 spatial layers and then back to 3. This switch is done
-// by setting top spatial layer bitrate to 0, and then back to non-zero, during
-// the sequence.
-TEST_P(DatarateOnePassCbrSvc, OnePassCbrSvc3SL_to_2SL_dynamic) {
+// the fly switching to 1 and then 2 and back to 3 spatial layers. This switch
+// is done by setting spatial layer bitrates to 0, and then back to non-zero,
+// during the sequence.
+TEST_P(DatarateOnePassCbrSvc, OnePassCbrSvc3SL_dynamic) {
   cfg_.rc_buf_initial_sz = 500;
   cfg_.rc_buf_optimal_sz = 500;
   cfg_.rc_buf_sz = 1000;
diff --git a/vp9/vp9_cx_iface.c b/vp9/vp9_cx_iface.c
index 40f7ab531..510701d18 100644
--- a/vp9/vp9_cx_iface.c
+++ b/vp9/vp9_cx_iface.c
@@ -1229,7 +1229,7 @@ static vpx_codec_err_t encoder_encode(vpx_codec_alg_priv_t *ctx,
              cpi->svc.spatial_layer_id < cpi->svc.number_spatial_layers - 1)) {
           if (ctx->pending_cx_data == 0) ctx->pending_cx_data = cx_data;
           ctx->pending_cx_data_sz += size;
-          ctx->pending_frame_sizes[ctx->pending_frame_count++] = size;
+          if (size) ctx->pending_frame_sizes[ctx->pending_frame_count++] = size;
           ctx->pending_frame_magnitude |= size;
           cx_data += size;
           cx_data_sz -= size;
-- 
2.40.0