rtc: Clamp ROI segment q against base_qindex

av1_apply_roi_map() checked each segment's delta q against the qindex
of the last segment that had a delta, not against base_qindex. So a
segment after a raised one could skip the clamp, or get a clamp value
from the wrong qindex, and end up at qindex 0. With base_qindex 0, a
segment without a delta stayed at qindex 0 once an earlier segment had
been moved to 1. nonrd can't code a lossless segment. It hits the
lossless tx size assert in av1_nonrd_pick_intra_mode(), and without
asserts the stream is broken.

Clamp every segment against base_qindex. The ROI rdmult still follows
the last segment with a requested delta, so moving a segment off
qindex 0 does not change it.

Repro: ROIMapTest.ROIDeltaQMixed sets segment 1 to +10 and segment 2
to -63, which makes segment 2 lossless on every frame.

Change-Id: I5a7d43356f5620edaf3587b4dbd6af31a7396c4b

av1/encoder/encoder_utils.c

diff --git a/av1/encoder/encoder_utils.c b/av1/encoder/encoder_utils.c
index 3d995ba..7a478da 100644
--- a/av1/encoder/encoder_utils.c
+++ b/av1/encoder/encoder_utils.c
@@ -436,7 +436,10 @@
   int ref_frame[8];
   int internal_delta_q[MAX_SEGMENTS];
   int skip_enabled = 0;
-  int qindex = cm->quant_params.base_qindex;
+  const int base_qindex = cm->quant_params.base_qindex;
+  // Qindex of the last segment with a requested delta q, used for the ROI
+  // rdmult.
+  int roi_qindex = base_qindex;
 
   // Force disable of ROI if active_map is enabled. ROI for now
   // only supported/tested for realtime mode with speed >= 7.
@@ -485,17 +488,16 @@
     if (delta_q[i] < 0) internal_delta_q[i] = -internal_delta_q[i];
     // Clamp to allowed best/worst quality. For best quality clamp to 1
     // to avoid qindex = 0 case (lossless segment) which can happen for
-    // best_quality = 0.
-    if (qindex + internal_delta_q[i] <= cpi->rc.best_quality)
-      internal_delta_q[i] = AOMMAX(1, cpi->rc.best_quality) - qindex;
-    if (qindex + internal_delta_q[i] > cpi->rc.worst_quality)
-      internal_delta_q[i] =
-          cpi->rc.worst_quality - cm->quant_params.base_qindex;
+    // best_quality = 0. The segment qindex is base_qindex plus its delta.
+    if (base_qindex + internal_delta_q[i] <= cpi->rc.best_quality)
+      internal_delta_q[i] = AOMMAX(1, cpi->rc.best_quality) - base_qindex;
+    if (base_qindex + internal_delta_q[i] > cpi->rc.worst_quality)
+      internal_delta_q[i] = cpi->rc.worst_quality - base_qindex;
     if (internal_delta_q[i] != 0) {
       av1_enable_segfeature(seg, i, SEG_LVL_ALT_Q);
       av1_set_segdata(seg, i, SEG_LVL_ALT_Q, internal_delta_q[i]);
       roi->delta_qp_enabled = 1;
-      qindex = cm->quant_params.base_qindex + internal_delta_q[i];
+      if (delta_q[i] != 0) roi_qindex = base_qindex + internal_delta_q[i];
     }
     if (delta_lf[i] != 0) {
       // Disable loopfilter delta from ROI, as it requires
@@ -536,7 +538,7 @@
     roi->enabled = 1;
     if (roi->delta_qp_enabled) {
       roi->rdmult_delta_qp = av1_compute_rd_mult(
-          qindex, cm->seq_params->bit_depth,
+          roi_qindex, cm->seq_params->bit_depth,
           cpi->ppi->gf_group.update_type[cpi->gf_frame_index], 0, 15,
           INTER_FRAME, cpi->oxcf.q_cfg.use_fixed_qp_offsets,
           is_stat_consumption_stage(cpi), cpi->oxcf.tune_cfg.tuning,

test/roi_map_test.cc

diff --git a/test/roi_map_test.cc b/test/roi_map_test.cc
index 4da3c95..627076f 100644
--- a/test/roi_map_test.cc
+++ b/test/roi_map_test.cc
@@ -27,7 +27,7 @@
 
 namespace {
 
-enum { kSkip = 0, kDeltaQ = 1, kDeltaLF = 2, kReference = 3 };
+enum { kSkip = 0, kDeltaQ = 1, kDeltaLF = 2, kReference = 3, kDeltaQMixed = 4 };
 
 // Params: test mode, speed, aq_mode and screen_content mode.
 class ROIMapTest
@@ -80,6 +80,8 @@
           if (i > roi.rows / 4 && i < (3 * roi.rows) / 4 && j > roi.cols / 4 &&
               j < (3 * roi.cols) / 4)
             roi.roi_map[idx] = 1;
+          else if (roi_feature_ == kDeltaQMixed && i < roi.rows / 4)
+            roi.roi_map[idx] = 2;
           else
             roi.roi_map[idx] = 0;
         }
@@ -91,6 +93,11 @@
         roi.delta_q[1] = -40;
       else if (roi_feature_ == kReference)
         roi.ref_frame[1] = 4;  // GOLDEN_FRAME;
+      else if (roi_feature_ == kDeltaQMixed) {
+        // Segment 2 must not end up lossless after segment 1 raised the q.
+        roi.delta_q[1] = 10;
+        roi.delta_q[2] = -63;
+      }
       encoder->Control(AOME_SET_ROI_MAP, &roi);
       free(roi.roi_map);
     }
@@ -128,6 +135,22 @@
     ASSERT_NO_FATAL_FAILURE(RunLoop(&video));
   }
 
+  void ROIDeltaQMixedTest() {
+    cfg_.g_lag_in_frames = 0;
+    cfg_.rc_target_bitrate = 400;
+    cfg_.rc_resize_mode = 0;
+    cfg_.g_pass = AOM_RC_ONE_PASS;
+    cfg_.rc_end_usage = AOM_CBR;
+    cfg_.kf_max_dist = 90000;
+    cfg_.rc_min_quantizer = 0;
+    cfg_.rc_max_quantizer = 40;
+    roi_feature_ = kDeltaQMixed;
+    ::libaom_test::I420VideoSource video("niklas_640_480_30.yuv", 640, 480, 30,
+                                         1, 0, 30);
+
+    ASSERT_NO_FATAL_FAILURE(RunLoop(&video));
+  }
+
   void ROIReferenceTest() {
     cfg_.g_lag_in_frames = 0;
     cfg_.rc_target_bitrate = 400;
@@ -158,6 +181,8 @@
 
 TEST_P(ROIMapTest, ROIReference) { ROIReferenceTest(); }
 
+TEST_P(ROIMapTest, ROIDeltaQMixed) { ROIDeltaQMixedTest(); }
+
 AV1_INSTANTIATE_TEST_SUITE(ROIMapTest,
                            ::testing::Values(::libaom_test::kRealTime),
                            ::testing::Range(7, 12), ::testing::Values(0, 1),