Fix FrameConfigs used for VP8 with four temporal layers.

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

Index: webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc
diff --git a/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc b/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc
index 979c64e24324bca7493001b16f37b94ed14b18d1..728960d23ec2e584271601d2a2795d0849bd4a5e 100644
--- a/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc
@@ -96,10 +96,14 @@ std::vector<bool> GetTemporalLayerSync(size_t num_layers) {
     case 2:
       return {false, true, false, false, false, false, false, false};
     case 3:
-      return {false, true, true, false, false, false, false, false};
+      if (field_trial::IsEnabled("WebRTC-AltTemporalLayers")) {

[pbos-webrtc, 2017/08/29 15:46:42]

Since this is a subset of the first, can you create the first and then resize it
to four?

[sprang_webrtc, 2017/08/31 11:56:18]

Done.

+        return {false, true, true, false};
+      } else {
+        return {false, true, true, false, false, false, false, false};
+      }
     case 4:
-      return {false, true, true,  true, true,  true, false, true,
-              false, true, false, true, false, true, false, true};
+      return {false, true,  true,  false, true,  false, false, false,
+              false, false, false, false, false, false, false, false};
     default:
       RTC_NOTREACHED();
       break;
@@ -134,7 +138,7 @@ std::vector<TemporalLayers::FrameConfig> GetTemporalPattern(size_t num_layers) {
       // TL0 also references and updates the 'last' buffer.
       // TL1 also references 'last' and references and updates 'golden'.
       return {TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
-                                          TemporalLayers::kUpdate,
+                                          TemporalLayers::kNone,
                                           TemporalLayers::kReference),
               TemporalLayers::FrameConfig(TemporalLayers::kReference,
                                           TemporalLayers::kUpdate,
@@ -158,35 +162,55 @@ std::vector<TemporalLayers::FrameConfig> GetTemporalPattern(size_t num_layers) {
                   TemporalLayers::kReference, TemporalLayers::kReference,
                   TemporalLayers::kReference, TemporalLayers::kFreezeEntropy)};
     case 3:
-      // All layers can reference but not update the 'alt' buffer, this means
-      // that the 'alt' buffer reference is effectively the last keyframe.
-      // TL0 also references and updates the 'last' buffer.
-      // TL1 also references 'last' and references and updates 'golden'.
-      // TL2 references both 'last' and 'golden' but updates no buffer.
-      return {TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
-                                          TemporalLayers::kUpdate,
-                                          TemporalLayers::kReference),
-              TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kNone,
-                  TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
-              TemporalLayers::FrameConfig(TemporalLayers::kReference,
-                                          TemporalLayers::kUpdate,
-                                          TemporalLayers::kReference),
-              TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kReference,
-                  TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
-              TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
-                                          TemporalLayers::kNone,
-                                          TemporalLayers::kReference),
-              TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kReference,
-                  TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
-              TemporalLayers::FrameConfig(TemporalLayers::kReference,
-                                          TemporalLayers::kReferenceAndUpdate,
-                                          TemporalLayers::kReference),
-              TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kReference,
-                  TemporalLayers::kReference, TemporalLayers::kFreezeEntropy)};
+      if (field_trial::IsEnabled("WebRTC-AltTemporalLayers")) {

[pbos-webrtc, 2017/08/29 15:46:42]

Can you add a test that covers this case? You can set field trials in tests.

[pbos-webrtc, 2017/08/29 15:46:42]

Since this is a subset of the first, can you create the vector first and then
resize it to four instead of returning two different-looking initializer lists?

[sprang_webrtc, 2017/08/31 11:56:18]

They are different. In this version we update altref with frames in TL2, instead
of just referencing TL0, TL1, and the previous key frame.

[sprang_webrtc, 2017/08/31 11:56:18]

Test added.

+        // TL0 references and updates the 'last' buffer.
+        // TL1  references 'last' and references and updates 'golden'.
+        // TL2 references both 'last' & 'golden' and references and updates
+        // 'arf'.
+        return {TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
+                                            TemporalLayers::kNone,
+                                            TemporalLayers::kNone),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kNone,
+                                            TemporalLayers::kUpdate),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kUpdate,
+                                            TemporalLayers::kNone),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kReference,
+                                            TemporalLayers::kReference)};
+      } else {
+        // All layers can reference but not update the 'alt' buffer, this means
+        // that the 'alt' buffer reference is effectively the last keyframe.
+        // TL0 also references and updates the 'last' buffer.
+        // TL1 also references 'last' and references and updates 'golden'.
+        // TL2 references both 'last' and 'golden' but updates no buffer.
+        return {TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
+                                            TemporalLayers::kNone,
+                                            TemporalLayers::kReference),
+                TemporalLayers::FrameConfig(
+                    TemporalLayers::kReference, TemporalLayers::kNone,
+                    TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kUpdate,
+                                            TemporalLayers::kReference),
+                TemporalLayers::FrameConfig(
+                    TemporalLayers::kReference, TemporalLayers::kReference,
+                    TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
+                TemporalLayers::FrameConfig(TemporalLayers::kReferenceAndUpdate,
+                                            TemporalLayers::kNone,
+                                            TemporalLayers::kReference),
+                TemporalLayers::FrameConfig(
+                    TemporalLayers::kReference, TemporalLayers::kReference,
+                    TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kReferenceAndUpdate,
+                                            TemporalLayers::kReference),
+                TemporalLayers::FrameConfig(TemporalLayers::kReference,
+                                            TemporalLayers::kReference,
+                                            TemporalLayers::kReference,
+                                            TemporalLayers::kFreezeEntropy)};
+      }
     case 4:
       // TL0 references and updates only the 'last' buffer.
       // TL1 references 'last' and updates and references 'golden'.
@@ -196,13 +220,13 @@ std::vector<TemporalLayers::FrameConfig> GetTemporalPattern(size_t num_layers) {
                                           TemporalLayers::kNone,
                                           TemporalLayers::kNone),
               TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kReference,
-                  TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
+                  TemporalLayers::kReference, TemporalLayers::kNone,
+                  TemporalLayers::kNone, TemporalLayers::kFreezeEntropy),
               TemporalLayers::FrameConfig(TemporalLayers::kReference,
                                           TemporalLayers::kNone,
                                           TemporalLayers::kUpdate),
               TemporalLayers::FrameConfig(
-                  TemporalLayers::kReference, TemporalLayers::kReference,
+                  TemporalLayers::kReference, TemporalLayers::kNone,
                   TemporalLayers::kReference, TemporalLayers::kFreezeEntropy),
               TemporalLayers::FrameConfig(TemporalLayers::kReference,
                                           TemporalLayers::kUpdate,