Fix FrameConfigs used for VP8 with four temporal layers.

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

Index: webrtc/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc
diff --git a/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc
index fd4bf527b4a1327a5697f04bc88099e224723035..0075fd51b14d86ff64a0a0009342a3c1c125f653 100644
--- a/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc
@@ -13,14 +13,15 @@
 #include "vpx/vpx_encoder.h"
 #include "webrtc/modules/video_coding/codecs/vp8/vp8_impl.h"
 #include "webrtc/modules/video_coding/include/video_codec_interface.h"
+#include "webrtc/test/field_trial.h"
 #include "webrtc/test/gtest.h"
 
 namespace webrtc {
+namespace test {
 
 enum {
   kTemporalUpdateLast = VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_UPD_ARF |
-                        VP8_EFLAG_NO_REF_GF |
-                        VP8_EFLAG_NO_REF_ARF,
+                        VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_REF_ARF,
   kTemporalUpdateGoldenWithoutDependency =
       VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_REF_ARF | VP8_EFLAG_NO_UPD_ARF |
       VP8_EFLAG_NO_UPD_LAST,
@@ -31,16 +32,16 @@ enum {
       VP8_EFLAG_NO_UPD_LAST,
   kTemporalUpdateAltref = VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_UPD_LAST,
   kTemporalUpdateNone = VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_UPD_ARF |
-                        VP8_EFLAG_NO_UPD_LAST |
-                        VP8_EFLAG_NO_UPD_ENTROPY,
-  kTemporalUpdateNoneNoRefAltRef = VP8_EFLAG_NO_REF_ARF | VP8_EFLAG_NO_UPD_GF |
-                                   VP8_EFLAG_NO_UPD_ARF |
-                                   VP8_EFLAG_NO_UPD_LAST |
-                                   VP8_EFLAG_NO_UPD_ENTROPY,
-  kTemporalUpdateNoneNoRefGolden = VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_UPD_GF |
-                                   VP8_EFLAG_NO_UPD_ARF |
-                                   VP8_EFLAG_NO_UPD_LAST |
-                                   VP8_EFLAG_NO_UPD_ENTROPY,
+                        VP8_EFLAG_NO_UPD_LAST | VP8_EFLAG_NO_UPD_ENTROPY,
+  kTemporalUpdateNoneNoRefAltRef =
+      VP8_EFLAG_NO_REF_ARF | VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_UPD_ARF |
+      VP8_EFLAG_NO_UPD_LAST | VP8_EFLAG_NO_UPD_ENTROPY,
+  kTemporalUpdateNoneNoRefGolden =
+      VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_UPD_ARF |
+      VP8_EFLAG_NO_UPD_LAST | VP8_EFLAG_NO_UPD_ENTROPY,
+  kTemporalUpdateNoneNoRefGoldenAltRef =
+      VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_UPD_GF | VP8_EFLAG_NO_REF_ARF |
+      VP8_EFLAG_NO_UPD_ARF | VP8_EFLAG_NO_UPD_LAST | VP8_EFLAG_NO_UPD_ENTROPY,
   kTemporalUpdateGoldenWithoutDependencyRefAltRef =
       VP8_EFLAG_NO_REF_GF | VP8_EFLAG_NO_UPD_ARF | VP8_EFLAG_NO_UPD_LAST,
   kTemporalUpdateGoldenRefAltRef = VP8_EFLAG_NO_UPD_ARF | VP8_EFLAG_NO_UPD_LAST,
@@ -58,7 +59,7 @@ TEST(TemporalLayersTest, 2Layers) {
   tl.UpdateConfiguration(&cfg);
 
   int expected_flags[16] = {
-      kTemporalUpdateLastAndGoldenRefAltRef,
+      kTemporalUpdateLastRefAltRef,
       kTemporalUpdateGoldenWithoutDependencyRefAltRef,
       kTemporalUpdateLastRefAltRef,
       kTemporalUpdateGoldenRefAltRef,
@@ -66,7 +67,7 @@ TEST(TemporalLayersTest, 2Layers) {
       kTemporalUpdateGoldenRefAltRef,
       kTemporalUpdateLastRefAltRef,
       kTemporalUpdateNone,
-      kTemporalUpdateLastAndGoldenRefAltRef,
+      kTemporalUpdateLastRefAltRef,
       kTemporalUpdateGoldenWithoutDependencyRefAltRef,
       kTemporalUpdateLastRefAltRef,
       kTemporalUpdateGoldenRefAltRef,
@@ -85,7 +86,7 @@ TEST(TemporalLayersTest, 2Layers) {
   uint32_t timestamp = 0;
   for (int i = 0; i < 16; ++i) {
     TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp);
-    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config));
+    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config)) << i;
     tl.PopulateCodecSpecific(false, tl_config, &vp8_info, 0);
     EXPECT_EQ(expected_temporal_idx[i], vp8_info.temporalIdx);
     EXPECT_EQ(expected_temporal_idx[i], tl_config.packetizer_temporal_idx);
@@ -104,7 +105,7 @@ TEST(TemporalLayersTest, 3Layers) {
   tl.UpdateConfiguration(&cfg);
 
   int expected_flags[16] = {
-      kTemporalUpdateLastAndGoldenRefAltRef,
+      kTemporalUpdateLastRefAltRef,
       kTemporalUpdateNoneNoRefGolden,
       kTemporalUpdateGoldenWithoutDependencyRefAltRef,
       kTemporalUpdateNone,
@@ -112,7 +113,7 @@ TEST(TemporalLayersTest, 3Layers) {
       kTemporalUpdateNone,
       kTemporalUpdateGoldenRefAltRef,
       kTemporalUpdateNone,
-      kTemporalUpdateLastAndGoldenRefAltRef,
+      kTemporalUpdateLastRefAltRef,
       kTemporalUpdateNoneNoRefGolden,
       kTemporalUpdateGoldenWithoutDependencyRefAltRef,
       kTemporalUpdateNone,
@@ -131,7 +132,42 @@ TEST(TemporalLayersTest, 3Layers) {
   unsigned int timestamp = 0;
   for (int i = 0; i < 16; ++i) {
     TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp);
-    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config));
+    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config)) << i;
+    tl.PopulateCodecSpecific(false, tl_config, &vp8_info, 0);
+    EXPECT_EQ(expected_temporal_idx[i], vp8_info.temporalIdx);
+    EXPECT_EQ(expected_temporal_idx[i], tl_config.packetizer_temporal_idx);
+    EXPECT_EQ(expected_temporal_idx[i], tl_config.encoder_layer_id);
+    EXPECT_EQ(expected_layer_sync[i], vp8_info.layerSync);
+    EXPECT_EQ(expected_layer_sync[i], tl_config.layer_sync);
+    timestamp += 3000;
+  }
+}
+
+TEST(TemporalLayersTest, Alternative3Layers) {
+  ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/");
+  DefaultTemporalLayers tl(3, 0);
+  vpx_codec_enc_cfg_t cfg;
+  CodecSpecificInfoVP8 vp8_info;
+  tl.OnRatesUpdated(500, 500, 30);
+  tl.UpdateConfiguration(&cfg);
+
+  int expected_flags[8] = {kTemporalUpdateLast,
+                           kTemporalUpdateAltrefWithoutDependency,
+                           kTemporalUpdateGoldenWithoutDependency,
+                           kTemporalUpdateNone,
+                           kTemporalUpdateLast,
+                           kTemporalUpdateAltrefWithoutDependency,
+                           kTemporalUpdateGoldenWithoutDependency,
+                           kTemporalUpdateNone};
+  int expected_temporal_idx[8] = {0, 2, 1, 2, 0, 2, 1, 2};
+
+  bool expected_layer_sync[8] = {false, true, true, false,
+                                 false, true, true, false};
+
+  unsigned int timestamp = 0;
+  for (int i = 0; i < 8; ++i) {
+    TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp);
+    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config)) << i;
     tl.PopulateCodecSpecific(false, tl_config, &vp8_info, 0);
     EXPECT_EQ(expected_temporal_idx[i], vp8_info.temporalIdx);
     EXPECT_EQ(expected_temporal_idx[i], tl_config.packetizer_temporal_idx);
@@ -150,9 +186,9 @@ TEST(TemporalLayersTest, 4Layers) {
   tl.UpdateConfiguration(&cfg);
   int expected_flags[16] = {
       kTemporalUpdateLast,
-      kTemporalUpdateNone,
+      kTemporalUpdateNoneNoRefGoldenAltRef,
       kTemporalUpdateAltrefWithoutDependency,
-      kTemporalUpdateNone,
+      kTemporalUpdateNoneNoRefGolden,
       kTemporalUpdateGoldenWithoutDependency,
       kTemporalUpdateNone,
       kTemporalUpdateAltref,
@@ -169,14 +205,14 @@ TEST(TemporalLayersTest, 4Layers) {
   int expected_temporal_idx[16] = {0, 3, 2, 3, 1, 3, 2, 3,
                                    0, 3, 2, 3, 1, 3, 2, 3};
 
-  bool expected_layer_sync[16] = {false, true, true,  true, true,  true,
-                                  false, true, false, true, false, true,
-                                  false, true, false, true};
+  bool expected_layer_sync[16] = {false, true,  true,  false, true,  false,
+                                  false, false, false, false, false, false,
+                                  false, false, false, false};
 
   uint32_t timestamp = 0;
   for (int i = 0; i < 16; ++i) {
     TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp);
-    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config));
+    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config)) << i;
     tl.PopulateCodecSpecific(false, tl_config, &vp8_info, 0);
     EXPECT_EQ(expected_temporal_idx[i], vp8_info.temporalIdx);
     EXPECT_EQ(expected_temporal_idx[i], tl_config.packetizer_temporal_idx);
@@ -195,7 +231,7 @@ TEST(TemporalLayersTest, KeyFrame) {
   tl.UpdateConfiguration(&cfg);
 
   int expected_flags[8] = {
-      kTemporalUpdateLastAndGoldenRefAltRef,
+      kTemporalUpdateLastRefAltRef,
       kTemporalUpdateNoneNoRefGolden,
       kTemporalUpdateGoldenWithoutDependencyRefAltRef,
       kTemporalUpdateNone,
@@ -211,7 +247,7 @@ TEST(TemporalLayersTest, KeyFrame) {
   uint32_t timestamp = 0;
   for (int i = 0; i < 7; ++i) {
     TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp);
-    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config));
+    EXPECT_EQ(expected_flags[i], VP8EncoderImpl::EncodeFlags(tl_config)) << i;
     tl.PopulateCodecSpecific(true, tl_config, &vp8_info, 0);
     EXPECT_EQ(expected_temporal_idx[i], tl_config.packetizer_temporal_idx);
     EXPECT_EQ(expected_temporal_idx[i], tl_config.encoder_layer_id);
@@ -235,4 +271,128 @@ TEST(TemporalLayersTest, KeyFrame) {
                                      "marked layer sync since it only depends "
                                      "on the base layer.";
 }
+
+class TemporalLayersReferenceTest : public ::testing::TestWithParam<int> {
+ public:
+  TemporalLayersReferenceTest()
+      : timestamp_(1),
+        last_sync_timestamp_(timestamp_),
+        tl0_reference_(nullptr) {}
+  virtual ~TemporalLayersReferenceTest() {}
+
+ protected:
+  static const int kMaxPatternLength = 32;
+
+  struct BufferState {
+    BufferState() : BufferState(-1, 0, false) {}
+    BufferState(int temporal_idx, uint32_t timestamp, bool sync)
+        : temporal_idx(temporal_idx), timestamp(timestamp), sync(sync) {}
+    int temporal_idx;
+    uint32_t timestamp;
+    bool sync;
+  };
+
+  bool UpdateSyncRefState(const TemporalLayers::BufferFlags& flags,
+                          BufferState* buffer_state) {
+    if (flags & TemporalLayers::kReference) {
+      if (buffer_state->temporal_idx == -1)
+        return true;  // References key-frame.
+      if (buffer_state->temporal_idx == 0) {
+        // No more than one reference to TL0 frame.
+        EXPECT_EQ(nullptr, tl0_reference_);
+        tl0_reference_ = buffer_state;
+        return true;
+      }
+      return false;  // References higher layer.
+    }
+    return true;  // No reference, does not affect sync frame status.
+  }
+
+  void ValidateReference(const TemporalLayers::BufferFlags& flags,
+                         const BufferState& buffer_state,
+                         int temporal_layer) {
+    if (flags & TemporalLayers::kReference) {
+      if (temporal_layer > 0 && buffer_state.timestamp > 0) {
+        // Check that high layer reference does not go past last sync frame.
+        EXPECT_GE(buffer_state.timestamp, last_sync_timestamp_);
+      }
+      // No reference to buffer in higher layer.
+      EXPECT_LE(buffer_state.temporal_idx, temporal_layer);
+    }
+  }
+
+  uint32_t timestamp_ = 1;
+  uint32_t last_sync_timestamp_ = timestamp_;
+  BufferState* tl0_reference_;
+
+  BufferState last_state;
+  BufferState golden_state;
+  BufferState altref_state;
+};
+
+INSTANTIATE_TEST_CASE_P(DefaultTemporalLayersTest,
+                        TemporalLayersReferenceTest,
+                        ::testing::Range(1, kMaxTemporalStreams + 1));
+
+TEST_P(TemporalLayersReferenceTest, ValidFrameConfigs) {
+  const int num_layers = GetParam();
+  DefaultTemporalLayers tl(num_layers, 0);
+  vpx_codec_enc_cfg_t cfg;
+  tl.OnRatesUpdated(500, 500, 30);
+  tl.UpdateConfiguration(&cfg);
+
+  // Run through the pattern and store the frame dependencies, plus keep track
+  // of the buffer state; which buffers references which temporal layers (if
+  // (any). If a given buffer is never updated, it is legal to reference it
+  // even for sync frames. In order to be general, don't assume TL0 always
+  // updates |last|.
+  std::vector<TemporalLayers::FrameConfig> tl_configs(kMaxPatternLength);
+  for (int i = 0; i < kMaxPatternLength; ++i) {
+    TemporalLayers::FrameConfig tl_config = tl.UpdateLayerConfig(timestamp_++);
+    EXPECT_FALSE(tl_config.drop_frame);
+    tl_configs.push_back(tl_config);
+    int temporal_idx = tl_config.encoder_layer_id;
+    // For the default layers, always keep encoder and rtp layers in sync.
+    EXPECT_EQ(tl_config.packetizer_temporal_idx, temporal_idx);
+
+    // Determine if this frame is in a higher layer but references only TL0
+    // or untouched buffers, if so verify it is marked as a layer sync.
+    bool is_sync_frame = true;
+    tl0_reference_ = nullptr;
+    if (temporal_idx <= 0) {
+      is_sync_frame = false;  // TL0 by definition not a sync frame.
+    } else if (!UpdateSyncRefState(tl_config.last_buffer_flags, &last_state)) {
+      is_sync_frame = false;
+    } else if (!UpdateSyncRefState(tl_config.golden_buffer_flags,
+                                   &golden_state)) {
+      is_sync_frame = false;
+    } else if (!UpdateSyncRefState(tl_config.arf_buffer_flags, &altref_state)) {
+      is_sync_frame = false;
+    }
+    if (is_sync_frame) {
+      // Cache timestamp for last found sync frame, so that we can verify no
+      // references back past this frame.
+      ASSERT_TRUE(tl0_reference_);
+      last_sync_timestamp_ = tl0_reference_->timestamp;
+    }
+    EXPECT_EQ(tl_config.layer_sync, is_sync_frame);
+
+    // Validate no reference from lower to high temporal layer, or backwards
+    // past last reference frame.
+    ValidateReference(tl_config.last_buffer_flags, last_state, temporal_idx);
+    ValidateReference(tl_config.golden_buffer_flags, golden_state,
+                      temporal_idx);
+    ValidateReference(tl_config.arf_buffer_flags, altref_state, temporal_idx);
+
+    // Update the current layer state.
+    BufferState state = {temporal_idx, timestamp_, is_sync_frame};
+    if (tl_config.last_buffer_flags & TemporalLayers::kUpdate)
+      last_state = state;
+    if (tl_config.golden_buffer_flags & TemporalLayers::kUpdate)
+      golden_state = state;
+    if (tl_config.arf_buffer_flags & TemporalLayers::kUpdate)
+      altref_state = state;
+  }
+}
+}  // namespace test
 }  // namespace webrtc