Fix potential incorrect sync flags used with screenshare TL stream.

BUG=webrtc:7980

Review-Url: https://codereview.webrtc.org/2978743002
Cr-Commit-Position: refs/heads/master@{#18999}

webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc

@@ -25,7 +25,8 @@
 
 namespace webrtc {
 
-TemporalLayers::FrameConfig::FrameConfig() {}
+TemporalLayers::FrameConfig::FrameConfig()
+    : FrameConfig(kNone, kNone, kNone, false) {}
 
 TemporalLayers::FrameConfig::FrameConfig(TemporalLayers::BufferFlags last,
                                          TemporalLayers::BufferFlags golden,
@@ -48,7 +49,9 @@ TemporalLayers::FrameConfig::FrameConfig(TemporalLayers::BufferFlags last,
       last_buffer_flags(last),
       golden_buffer_flags(golden),
       arf_buffer_flags(arf),
-      freeze_entropy(freeze_entropy) {}
+      layer_sync(false),
+      freeze_entropy(freeze_entropy),
+      pattern_idx(0) {}
 
 namespace {
 

webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc

@@ -152,12 +152,18 @@ TemporalLayers::FrameConfig ScreenshareLayers::UpdateLayerConfig(
       last_emitted_tl0_timestamp_ = unwrapped_timestamp;
       break;
     case 1:
+      if (layers_[1].state != TemporalLayer::State::kDropped) {
         if (TimeToSync(unwrapped_timestamp)) {
           last_sync_timestamp_ = unwrapped_timestamp;
           layer_state = TemporalLayerState::kTl1Sync;
         } else {
           layer_state = TemporalLayerState::kTl1;
         }
+      } else {
+        layer_state = last_sync_timestamp_ == unwrapped_timestamp
+                          ? TemporalLayerState::kTl1Sync
+                          : TemporalLayerState::kTl1;
+      }
       break;
     case -1:
       layer_state = TemporalLayerState::kDrop;

webrtc/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc

@@ -126,18 +126,29 @@ class ScreenshareLayerTest : public ::testing::Test {
 
   // Adds frames until we get one in the specified temporal layer. The last
   // FrameEncoded() call will be omitted and needs to be done by the caller.
-  void SkipUntilTl(int layer) {
+  // Returns the flags for the last frame.
-    for (int i = 0; i < 5; ++i) {
+  int SkipUntilTl(int layer) {
-      ConfigureFrame(false);
+    return SkipUntilTlAndSync(layer, rtc::Optional<bool>());
+  }
+
+  // Same as SkipUntilTl, but also waits until the sync bit condition is met.
+  int SkipUntilTlAndSync(int layer, rtc::Optional<bool> sync) {
+    int flags = 0;
+    const int kMaxFramesToSkip =
+        1 + (sync.value_or(false) ? kMaxSyncPeriodSeconds : 1) * kFrameRate;
+    for (int i = 0; i < kMaxFramesToSkip; ++i) {
+      flags = ConfigureFrame(false);
       timestamp_ += kTimestampDelta5Fps;
-      if (vp8_info_.temporalIdx != layer) {
+      if (vp8_info_.temporalIdx != layer ||
+          (sync && *sync != vp8_info_.layerSync)) {
         layers_->FrameEncoded(frame_size_, kDefaultQp);
       } else {
-        // Found frame form sought layer.
+        // Found frame from sought after layer.
-        return;
+        return flags;
       }
     }
     ADD_FAILURE() << "Did not get a frame of TL" << layer << " in time.";
+    return -1;
   }
 
   int min_qp_;
@@ -562,4 +573,31 @@ TEST_F(ScreenshareLayerTest, RespectsConfiguredFramerate) {
   // Allow for some rounding errors in the measurements.
   EXPECT_NEAR(num_discarded_frames, num_input_frames / 2, 2);
 }
+
+TEST_F(ScreenshareLayerTest, 2LayersSyncAtOvershootDrop) {
+  // Run grace period so we have existing frames in both TL0 and Tl1.
+  EXPECT_TRUE(RunGracePeriod());
+
+  // Move ahead until we have a sync frame in TL1.
+  EXPECT_EQ(kTl1SyncFlags, SkipUntilTlAndSync(1, rtc::Optional<bool>(true)));
+  ASSERT_TRUE(vp8_info_.layerSync);
+
+  // Simulate overshoot of this frame.
+  layers_->FrameEncoded(0, -1);
+
+  // Reencode, frame config, flags and codec specific info should remain the
+  // same as for the dropped frame.
+  timestamp_ -= kTimestampDelta5Fps;  // Undo last timestamp increment.
+  TemporalLayers::FrameConfig new_tl_config =
+      layers_->UpdateLayerConfig(timestamp_);
+  EXPECT_EQ(tl_config_, new_tl_config);
+
+  config_updated_ = layers_->UpdateConfiguration(&cfg_);
+  EXPECT_EQ(kTl1SyncFlags, VP8EncoderImpl::EncodeFlags(tl_config_));
+
+  CodecSpecificInfoVP8 new_vp8_info;
+  layers_->PopulateCodecSpecific(false, tl_config_, &new_vp8_info, timestamp_);
+  EXPECT_TRUE(new_vp8_info.layerSync);
+}
+
 }  // namespace webrtc

webrtc/modules/video_coding/codecs/vp8/temporal_layers.h

@@ -55,6 +55,16 @@ class TemporalLayers {
 
     int pattern_idx;
 
+    bool operator==(const FrameConfig& o) const {
+      return drop_frame == o.drop_frame &&
+             last_buffer_flags == o.last_buffer_flags &&
+             golden_buffer_flags == o.golden_buffer_flags &&
+             arf_buffer_flags == o.arf_buffer_flags &&
+             layer_sync == o.layer_sync && freeze_entropy == o.freeze_entropy &&
+             pattern_idx == o.pattern_idx;
+    }
+    bool operator!=(const FrameConfig& o) const { return !(*this == o); }
+
    private:
     FrameConfig(BufferFlags last,
                 BufferFlags golden,