Parsing of encoded duration for unencrypted opus streams.

media/formats/webm/webm_cluster_parser_unittest.cc

Index: media/formats/webm/webm_cluster_parser_unittest.cc
diff --git a/media/formats/webm/webm_cluster_parser_unittest.cc b/media/formats/webm/webm_cluster_parser_unittest.cc
index bf19bee1c14db41fde44ef47d6c47b4a7329fb75..4377ee2618c7dee3b6656dfb4c58a3d427740a1d 100644
--- a/media/formats/webm/webm_cluster_parser_unittest.cc
+++ b/media/formats/webm/webm_cluster_parser_unittest.cc
@@ -4,9 +4,11 @@
 
 #include <algorithm>
 #include <cstdlib>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "media/base/audio_decoder_config.h"
 #include "media/base/decrypt_config.h"
 #include "media/formats/webm/cluster_builder.h"
 #include "media/formats/webm/webm_cluster_parser.h"
@@ -49,20 +51,36 @@ struct BlockInfo {
   // this BlockGroup. The absolute value is used for parser verification.
   // For simple blocks, this value must be non-negative, and is used only for
   // parser verification.
-  int duration;
+  float duration;
+
   bool use_simple_block;
+
+  // Default data will be used if no data given.
+  const uint8* data;
+  int data_length;
 };
 
 static const BlockInfo kDefaultBlockInfo[] = {
-  { kAudioTrackNum, 0, 23, true },
-  { kAudioTrackNum, 23, 23, true },
-  { kVideoTrackNum, 33, 34, true },  // Assumes not using DefaultDuration
-  { kAudioTrackNum, 46, 23, true },
-  { kVideoTrackNum, 67, 33, false },
-  { kAudioTrackNum, 69, 23, false },
-  { kVideoTrackNum, 100, 33, false },
+    {kAudioTrackNum, 0, 23, true, NULL, 0},
+    {kAudioTrackNum, 23, 23, true, NULL, 0},
+    // Assumes not using DefaultDuration
+    {kVideoTrackNum, 33, 34, true, NULL, 0},
+    {kAudioTrackNum, 46, 23, true, NULL, 0},
+    {kVideoTrackNum, 67, 33, false, NULL, 0},
+    {kAudioTrackNum, 69, 23, false, NULL, 0},
+    {kVideoTrackNum, 100, 33, false, NULL, 0},
 };
 
+// Opus configuration. kCodecOpus is critical for OpusWebMClusterParserTest,
+// other arguments are just reasonable dummy data.
+const AudioDecoderConfig kOpusAudioConfig(kCodecOpus,
+                                          kSampleFormatF32,
+                                          CHANNEL_LAYOUT_STEREO,
+                                          48000,
+                                          NULL,
+                                          0,
+                                          false);
+
 static const uint8 kEncryptedFrame[] = {
   0x01,  // Block is encrypted
   0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08  // IV
@@ -74,27 +92,38 @@ static scoped_ptr<Cluster> CreateCluster(int timecode,
   ClusterBuilder cb;
   cb.SetClusterTimecode(0);
 
+  uint8 kDefaultBlockData[] = {0x00};
+
   for (int i = 0; i < block_count; i++) {
-    uint8 data[] = { 0x00 };
+    const uint8* data;
+    int data_length;
+    if (block_info[i].data != NULL) {
+      data = block_info[i].data;
+      data_length = block_info[i].data_length;
+    } else {
+      data = kDefaultBlockData;
+      data_length = sizeof(kDefaultBlockData);
+    }
+
     if (block_info[i].use_simple_block) {
       CHECK_GE(block_info[i].duration, 0);
       cb.AddSimpleBlock(block_info[i].track_num,
                         block_info[i].timestamp,
-                        0, data, sizeof(data));
+                        0, data, data_length);
       continue;
     }
 
     if (block_info[i].duration < 0) {
       cb.AddBlockGroupWithoutBlockDuration(block_info[i].track_num,
                                            block_info[i].timestamp,
-                                           0, data, sizeof(data));
+                                           0, data,data_length);
       continue;
     }
 
     cb.AddBlockGroup(block_info[i].track_num,
                      block_info[i].timestamp,
                      block_info[i].duration,
-                     0, data, sizeof(data));
+                     0, data, data_length);
   }
 
   return cb.Finish();
@@ -248,6 +277,7 @@ class WebMClusterParserTest : public testing::Test {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB())) {}
 
  protected:
@@ -270,6 +300,7 @@ class WebMClusterParserTest : public testing::Test {
                                         std::set<int64>(),
                                         std::string(),
                                         std::string(),
+                                        AudioDecoderConfig(),
                                         LogCB()));
   }
 
@@ -305,18 +336,19 @@ TEST_F(WebMClusterParserTest, HeldBackBufferHoldsBackAllTracks) {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kBlockInfo[] = {
-    { kVideoTrackNum, 0, 33, true },
-    { kAudioTrackNum, 0, 23, false },
-    { kTextTrackNum, 10, 42, false },
-    { kAudioTrackNum, 23, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 33, 33, true },
-    { kAudioTrackNum, 36, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 66, 33, true },
-    { kAudioTrackNum, 70, kTestAudioFrameDefaultDurationInMs, true },
-    { kAudioTrackNum, 83, kTestAudioFrameDefaultDurationInMs, true },
+      {kVideoTrackNum, 0, 33, true, NULL, 0},
+      {kAudioTrackNum, 0, 23, false, NULL, 0},
+      {kTextTrackNum, 10, 42, false, NULL, 0},
+      {kAudioTrackNum, 23, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 33, 33, true, NULL, 0},
+      {kAudioTrackNum, 36, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 66, 33, true, NULL, 0},
+      {kAudioTrackNum, 70, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kAudioTrackNum, 83, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
   };
 
   const int kExpectedBuffersOnPartialCluster[] = {
@@ -444,8 +476,8 @@ TEST_F(WebMClusterParserTest, ParseClusterWithMultipleCalls) {
 // one of these scenarios.
 TEST_F(WebMClusterParserTest, ParseBlockGroup) {
   const BlockInfo kBlockInfo[] = {
-    { kAudioTrackNum, 0, 23, false },
-    { kVideoTrackNum, 33, 34, false },
+      {kAudioTrackNum, 0, 23, false, NULL, 0},
+      {kVideoTrackNum, 33, 34, false, NULL, 0},
   };
   int block_count = arraysize(kBlockInfo);
 
@@ -470,11 +502,11 @@ TEST_F(WebMClusterParserTest, ParseBlockGroup) {
 
 TEST_F(WebMClusterParserTest, ParseSimpleBlockAndBlockGroupMixture) {
   const BlockInfo kBlockInfo[] = {
-    { kAudioTrackNum, 0, 23, true },
-    { kAudioTrackNum, 23, 23, false },
-    { kVideoTrackNum, 33, 34, true },
-    { kAudioTrackNum, 46, 23, false },
-    { kVideoTrackNum, 67, 33, false },
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 23, false, NULL, 0},
+      {kVideoTrackNum, 33, 34, true, NULL, 0},
+      {kAudioTrackNum, 46, 23, false, NULL, 0},
+      {kVideoTrackNum, 67, 33, false, NULL, 0},
   };
   int block_count = arraysize(kBlockInfo);
   scoped_ptr<Cluster> cluster(CreateCluster(0, kBlockInfo, block_count));
@@ -497,24 +529,25 @@ TEST_F(WebMClusterParserTest, IgnoredTracks) {
                                       ignored_tracks,
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kInputBlockInfo[] = {
-    { kAudioTrackNum, 0,  23, true },
-    { kAudioTrackNum, 23, 23, true },
-    { kVideoTrackNum, 33, 34, true },
-    { kTextTrackNum,  33, 99, true },
-    { kAudioTrackNum, 46, 23, true },
-    { kVideoTrackNum, 67, 34, true },
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 23, true, NULL, 0},
+      {kVideoTrackNum, 33, 34, true, NULL, 0},
+      {kTextTrackNum, 33, 99, true, NULL, 0},
+      {kAudioTrackNum, 46, 23, true, NULL, 0},
+      {kVideoTrackNum, 67, 34, true, NULL, 0},
   };
   int input_block_count = arraysize(kInputBlockInfo);
 
   const BlockInfo kOutputBlockInfo[] = {
-    { kAudioTrackNum, 0,  23, true },
-    { kAudioTrackNum, 23, 23, true },
-    { kVideoTrackNum, 33, 34, true },
-    { kAudioTrackNum, 46, 23, true },
-    { kVideoTrackNum, 67, 34, true },
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 23, true, NULL, 0},
+      {kVideoTrackNum, 33, 34, true, NULL, 0},
+      {kAudioTrackNum, 46, 23, true, NULL, 0},
+      {kVideoTrackNum, 67, 34, true, NULL, 0},
   };
   int output_block_count = arraysize(kOutputBlockInfo);
 
@@ -542,16 +575,17 @@ TEST_F(WebMClusterParserTest, ParseTextTracks) {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kInputBlockInfo[] = {
-    { kAudioTrackNum, 0,  23, true },
-    { kAudioTrackNum, 23, 23, true },
-    { kVideoTrackNum, 33, 34, true },
-    { kTextTrackNum,  33, 42, false },
-    { kAudioTrackNum, 46, 23, true },
-    { kTextTrackNum, 55, 44, false },
-    { kVideoTrackNum, 67, 34, true },
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 23, true, NULL, 0},
+      {kVideoTrackNum, 33, 34, true, NULL, 0},
+      {kTextTrackNum, 33, 42, false, NULL, 0},
+      {kAudioTrackNum, 46, 23, true, NULL, 0},
+      {kTextTrackNum, 55, 44, false, NULL, 0},
+      {kVideoTrackNum, 67, 34, true, NULL, 0},
   };
   int input_block_count = arraysize(kInputBlockInfo);
 
@@ -579,6 +613,7 @@ TEST_F(WebMClusterParserTest, TextTracksSimpleBlock) {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kInputBlockInfo[] = {
@@ -616,17 +651,18 @@ TEST_F(WebMClusterParserTest, ParseMultipleTextTracks) {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kInputBlockInfo[] = {
-    { kAudioTrackNum, 0,  23, true },
-    { kAudioTrackNum, 23, 23, true },
-    { kVideoTrackNum, 33, 34, true },
-    { kSubtitleTextTrackNum,  33, 42, false },
-    { kAudioTrackNum, 46, 23, true },
-    { kCaptionTextTrackNum, 55, 44, false },
-    { kVideoTrackNum, 67, 34, true },
-    { kSubtitleTextTrackNum,  67, 33, false },
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 23, true, NULL, 0},
+      {kVideoTrackNum, 33, 34, true, NULL, 0},
+      {kSubtitleTextTrackNum, 33, 42, false, NULL, 0},
+      {kAudioTrackNum, 46, 23, true, NULL, 0},
+      {kCaptionTextTrackNum, 55, 44, false, NULL, 0},
+      {kVideoTrackNum, 67, 34, true, NULL, 0},
+      {kSubtitleTextTrackNum, 67, 33, false, NULL, 0},
   };
   int input_block_count = arraysize(kInputBlockInfo);
 
@@ -662,6 +698,7 @@ TEST_F(WebMClusterParserTest, ParseEncryptedBlock) {
                                       std::set<int64>(),
                                       std::string(),
                                       "video_key_id",
+                                      AudioDecoderConfig(),
                                       LogCB()));
   int result = parser_->Parse(cluster->data(), cluster->size());
   EXPECT_EQ(cluster->size(), result);
@@ -683,6 +720,7 @@ TEST_F(WebMClusterParserTest, ParseBadEncryptedBlock) {
                                       std::set<int64>(),
                                       std::string(),
                                       "video_key_id",
+                                      AudioDecoderConfig(),
                                       LogCB()));
   int result = parser_->Parse(cluster->data(), cluster->size());
   EXPECT_EQ(-1, result);
@@ -722,6 +760,7 @@ TEST_F(WebMClusterParserTest, ParseInvalidTextBlockGroupWithoutDuration) {
                                       std::set<int64>(),
                                       std::string(),
                                       std::string(),
+                                      AudioDecoderConfig(),
                                       LogCB()));
 
   const BlockInfo kBlockInfo[] = {
@@ -741,13 +780,13 @@ TEST_F(WebMClusterParserTest, ParseWithDefaultDurationsSimpleBlocks) {
   EXPECT_LT(kTestVideoFrameDefaultDurationInMs, 33);
 
   const BlockInfo kBlockInfo[] = {
-    { kAudioTrackNum, 0, kTestAudioFrameDefaultDurationInMs, true },
-    { kAudioTrackNum, 23, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 33, kTestVideoFrameDefaultDurationInMs, true },
-    { kAudioTrackNum, 46, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 67, kTestVideoFrameDefaultDurationInMs, true },
-    { kAudioTrackNum, 69, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 100, kTestVideoFrameDefaultDurationInMs, true },
+      {kAudioTrackNum, 0, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kAudioTrackNum, 23, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 33, kTestVideoFrameDefaultDurationInMs, true, NULL, 0},
+      {kAudioTrackNum, 46, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 67, kTestVideoFrameDefaultDurationInMs, true, NULL, 0},
+      {kAudioTrackNum, 69, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 100, kTestVideoFrameDefaultDurationInMs, true, NULL, 0},
   };
 
   int block_count = arraysize(kBlockInfo);
@@ -778,13 +817,15 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsSimpleBlocks) {
   // as the lowest non-zero duration seen so far if the last buffer in the track
   // in the cluster (independently for each track in the cluster).
   const BlockInfo kBlockInfo1[] = {
-    { kAudioTrackNum, 0, 23, true },
-    { kAudioTrackNum, 23, 22, true },
-    { kVideoTrackNum, 33, 33, true },
-    { kAudioTrackNum, 45, 23, true },
-    { kVideoTrackNum, 66, 34, true },
-    { kAudioTrackNum, 68, 22, true },  // Estimated from minimum audio dur
-    { kVideoTrackNum, 100, 33, true },  // Estimated from minimum video dur
+      {kAudioTrackNum, 0, 23, true, NULL, 0},
+      {kAudioTrackNum, 23, 22, true, NULL, 0},
+      {kVideoTrackNum, 33, 33, true, NULL, 0},
+      {kAudioTrackNum, 45, 23, true, NULL, 0},
+      {kVideoTrackNum, 66, 34, true, NULL, 0},
+      // Estimated from minimum audio dur
+      {kAudioTrackNum, 68, 22, true, NULL, 0},
+      // Estimated from minimum video dur
+      {kVideoTrackNum, 100, 33, true, NULL, 0},
   };
 
   int block_count1 = arraysize(kBlockInfo1);
@@ -812,8 +853,10 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsSimpleBlocks) {
   // Verify that the estimated frame duration is tracked across clusters for
   // each track.
   const BlockInfo kBlockInfo2[] = {
-    { kAudioTrackNum, 200, 22, true },  // Estimate carries over across clusters
-    { kVideoTrackNum, 201, 33, true },  // Estimate carries over across clusters
+      // Estimate carries over across clusters
+      {kAudioTrackNum, 200, 22, true, NULL, 0},
+      // Estimate carries over across clusters
+      {kVideoTrackNum, 201, 33, true, NULL, 0},
   };
 
   int block_count2 = arraysize(kBlockInfo2);
@@ -831,14 +874,16 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsBlockGroups) {
   // cluster, and are estimated as the lowest non-zero duration seen so far if
   // the last buffer in the track in the cluster (independently for each track
   // in the cluster).
-    const BlockInfo kBlockInfo1[] = {
-    { kAudioTrackNum, 0, -23, false },
-    { kAudioTrackNum, 23, -22, false },
-    { kVideoTrackNum, 33, -33, false },
-    { kAudioTrackNum, 45, -23, false },
-    { kVideoTrackNum, 66, -34, false },
-    { kAudioTrackNum, 68, -22, false },  // Estimated from minimum audio dur
-    { kVideoTrackNum, 100, -33, false },  // Estimated from minimum video dur
+  const BlockInfo kBlockInfo1[] = {
+      {kAudioTrackNum, 0, -23, false, NULL, 0},
+      {kAudioTrackNum, 23, -22, false, NULL, 0},
+      {kVideoTrackNum, 33, -33, false, NULL, 0},
+      {kAudioTrackNum, 45, -23, false, NULL, 0},
+      {kVideoTrackNum, 66, -34, false, NULL, 0},
+      // Estimated from minimum audio dur
+      {kAudioTrackNum, 68, -22, false, NULL, 0},
+      // Estimated from minimum video dur
+      {kVideoTrackNum, 100, -33, false, NULL, 0},
   };
 
   int block_count1 = arraysize(kBlockInfo1);
@@ -866,8 +911,8 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsBlockGroups) {
   // Verify that the estimated frame duration is tracked across clusters for
   // each track.
   const BlockInfo kBlockInfo2[] = {
-    { kAudioTrackNum, 200, -22, false },
-    { kVideoTrackNum, 201, -33, false },
+      {kAudioTrackNum, 200, -22, false, NULL, 0},
+      {kVideoTrackNum, 201, -33, false, NULL, 0},
   };
 
   int block_count2 = arraysize(kBlockInfo2);
@@ -887,13 +932,18 @@ TEST_F(WebMClusterParserTest,
   EXPECT_LT(kTestVideoFrameDefaultDurationInMs, 33);
 
   const BlockInfo kBlockInfo[] = {
-    { kAudioTrackNum, 0, -kTestAudioFrameDefaultDurationInMs, false },
-    { kAudioTrackNum, 23, -kTestAudioFrameDefaultDurationInMs, false },
-    { kVideoTrackNum, 33, -kTestVideoFrameDefaultDurationInMs, false },
-    { kAudioTrackNum, 46, -kTestAudioFrameDefaultDurationInMs, false },
-    { kVideoTrackNum, 67, -kTestVideoFrameDefaultDurationInMs, false },
-    { kAudioTrackNum, 69, -kTestAudioFrameDefaultDurationInMs, false },
-    { kVideoTrackNum, 100, -kTestVideoFrameDefaultDurationInMs, false },
+      {kAudioTrackNum, 0, -kTestAudioFrameDefaultDurationInMs, false, NULL, 0},
+      {kAudioTrackNum, 23, -kTestAudioFrameDefaultDurationInMs, false, NULL, 0},
+      {kVideoTrackNum, 33, -kTestVideoFrameDefaultDurationInMs, false, NULL, 0},
+      {kAudioTrackNum, 46, -kTestAudioFrameDefaultDurationInMs, false, NULL, 0},
+      {kVideoTrackNum, 67, -kTestVideoFrameDefaultDurationInMs, false, NULL, 0},
+      {kAudioTrackNum, 69, -kTestAudioFrameDefaultDurationInMs, false, NULL, 0},
+      {kVideoTrackNum,
+       100,
+       -kTestVideoFrameDefaultDurationInMs,
+       false,
+       NULL,
+       0},
   };
 
   int block_count = arraysize(kBlockInfo);
@@ -943,8 +993,8 @@ TEST_F(WebMClusterParserTest,
   ResetParserToHaveDefaultDurations();
 
   const BlockInfo kBlockInfo[] = {
-    { kAudioTrackNum, 0, kTestAudioFrameDefaultDurationInMs, true },
-    { kVideoTrackNum, 0, kTestVideoFrameDefaultDurationInMs, true },
+      {kAudioTrackNum, 0, kTestAudioFrameDefaultDurationInMs, true, NULL, 0},
+      {kVideoTrackNum, 0, kTestVideoFrameDefaultDurationInMs, true, NULL, 0},
   };
 
   int block_count = arraysize(kBlockInfo);
@@ -954,4 +1004,141 @@ TEST_F(WebMClusterParserTest,
   ASSERT_TRUE(VerifyBuffers(parser_, kBlockInfo, block_count));
 }
 
+using OpusTestParam = std::tr1::tuple<int, int, bool>;
+class OpusWebMClusterParserTest

[wolenetz, 2015/01/30 20:46:12]

This looks like a good candidate for splitting out into a separate test file.

[chcunningham, 2015/02/03 20:34:43]

I split the Builder pieces into a separate file. With those gone, I feel like
the opus test functions are hopefully small enough to stay here.

If you'd prefer I can pull the tests out to a separate file as well, but it will
require a larger refactoring of this test file to put things like BlockInfo and
VerifyBuffers into common places.

+    : public WebMClusterParserTest,
+      public ::testing::WithParamInterface<OpusTestParam> {
+ protected:
+  void SetUp() override {
+    opus_config_ = std::tr1::get<0>(GetParam());
+    opus_frame_count_ = std::tr1::get<1>(GetParam());
+    variable_len_frames_ = std::tr1::get<2>(GetParam());
+
+    parser_.reset(new WebMClusterParser(
+        kTimecodeScale, kAudioTrackNum, kNoTimestamp(), kVideoTrackNum,
+        kNoTimestamp(), TextTracks(), std::set<int64>(), std::string(),
+        std::string(), kOpusAudioConfig, LogCB()));
+  }
+
+  // 5 highest bits of the Opus TOC byte.
+  int opus_config_;
+
+  // Number of frames in the opus packet.
+  int opus_frame_count_;
+
+  // Indicates compressed frames may differ in length. Only considered when
+  // opus_frame_count_ >= 2.
+  bool variable_len_frames_;
+};
+
+std::vector<uint8> BuildOpusPacket(int config,

[chcunningham, 2015/01/29 22:14:20]

I'm expecting this return to be RVO'd. Am I right? Is this good with
chromium/team style?

[wolenetz, 2015/01/30 20:46:12]

This is fine with me for these tests. For production code, we'd usually
encapsulate within something like ClusterBuilder, with an accessor that returns
a reference or copy of the constructed blob of data.

[chcunningham, 2015/02/03 20:34:43]

Acknowledged.

+                                   int frame_count,
+                                   bool variable_len) {
+  std::vector<uint8> opus_bytes;
+  int frame_count_code;
+  uint8 frame_count_byte;
+
+  if (frame_count == 1) {
+    frame_count_code = 0;
+  } else if (frame_count == 2) {
+    frame_count_code = variable_len ? 2 : 1;
+  } else {
+    frame_count_code = 3;
+    frame_count_byte = (variable_len ? 1 << 7 : 0) | frame_count;
+  }
+
+  // All opus packets must have TOC byte.
+  uint8 opus_toc_byte = (config << 3) | frame_count_code;
+  opus_bytes.push_back(opus_toc_byte);
+
+  // For code 3 packets, the number of frames is signalled in the "frame
+  // count byte".
+  if (frame_count_code == 3) {
+    opus_bytes.push_back(frame_count_byte);
+  }
+
+  // Simulating encoded frame data.
+  opus_bytes.push_back(static_cast<uint8>(0));
+
+  return opus_bytes;
+}
+
+TEST_P(OpusWebMClusterParserTest, ReadOpusDurationSimpleBlockAtEndOfCluster) {
+  std::vector<uint8> opus_bytes =
+      BuildOpusPacket(opus_config_, opus_frame_count_, variable_len_frames_);
+
+  int expected_duration_ms =
+      opus_frame_count_ *
+      (parser_->kOpusFrameDurationsMu[opus_config_] / static_cast<float>(1000));
+
+  const BlockInfo kBlockInfo[] = {{kAudioTrackNum,
+                                   0,
+                                   expected_duration_ms,
+                                   true,  // Make it a SimpleBlock.
+                                   &opus_bytes[0],
+                                   opus_bytes.size()}};
+
+  int block_count = arraysize(kBlockInfo);
+  scoped_ptr<Cluster> cluster(CreateCluster(0, kBlockInfo, block_count));
+  int result = parser_->Parse(cluster->data(), cluster->size());
+  EXPECT_EQ(cluster->size(), result);
+  ASSERT_TRUE(VerifyBuffers(parser_, kBlockInfo, block_count));
+}
+
+TEST_P(OpusWebMClusterParserTest, PreferOpusDurationsOverBlockDurations) {
+  std::vector<uint8> opus_bytes =
+      BuildOpusPacket(opus_config_, opus_frame_count_, variable_len_frames_);
+
+  int expected_duration_ms =
+      opus_frame_count_ *
+      (parser_->kOpusFrameDurationsMu[opus_config_] / static_cast<float>(1000));
+
+  // Setting BlockDuration != Opus duration to see which one the parser uses.
+  int block_duration_ms = expected_duration_ms + 10;
+
+  BlockInfo block_infos[] = {{kAudioTrackNum,
+                              0,
+                              block_duration_ms,
+                              false,  // Not a SimpleBlock.
+                              &opus_bytes[0],
+                              opus_bytes.size()}};
+
+  int block_count = arraysize(block_infos);
+  scoped_ptr<Cluster> cluster(CreateCluster(0, block_infos, block_count));
+  int result = parser_->Parse(cluster->data(), cluster->size());
+  EXPECT_EQ(cluster->size(), result);
+
+  // BlockInfo duration will be used to verify buffer duration, so changing
+  // duration to be that of the Opus packet to verify it was preferred.
+  block_infos[0].duration = expected_duration_ms;
+  ASSERT_TRUE(VerifyBuffers(parser_, block_infos, block_count));
+}
+
+// From Opus RFC.

[wolenetz, 2015/01/30 20:46:12]

provide link please

[chcunningham, 2015/02/03 20:34:43]

Done.

+const int kNumPossibleOpusConfigs = 32;

[wolenetz, 2015/01/30 20:46:12]

constants like these, we more often than not put into enums.

[chcunningham, 2015/02/03 20:34:43]

Done.

+const int kMaxOpusPacketFrameCount = 48;
+
+INSTANTIATE_TEST_CASE_P(

[chcunningham, 2015/01/29 22:14:20]

This was a fun exploration for me, but I think it may be overkill. Across these
3 instantions we end up running over 3K tests! 

I'm considering moving the TEST_Ps to TEST_F and dealing with the
parameterization by simply calling a helper in both tests that generates an
array of the full set of opus configs. The tests would just iterate over that
list. 3k tests -> 2 tests. Thoughts?

[wolenetz, 2015/01/30 20:46:12]

yeah, overkill. Please consider categories of tests (such as negative tests
(that confirm expected parse failure), and the other edge cases like 0 duration,
duration of packet exceeding spec'ed max 120ms, etc). Since you're reusing the
same kOpusFrameDurationsMu as the real implementation, I'm not sure the value of
exhaustive testing is worth the number of test increase.

[chcunningham, 2015/02/03 20:34:43]

Done, mostly. We chatted about not adding the negative tests, but instead adding
a warning to the parser in cases of having > 120 ms (too many frames in the
frame count byte). I've added that warning :)

[wolenetz, 2015/02/03 22:47:01]

Acknowledged.

+    SingleFrameOpusPackets,
+    OpusWebMClusterParserTest,
+    ::testing::Combine(::testing::Range(0, kNumPossibleOpusConfigs),
+                       ::testing::Values(1),
+                       ::testing::Values(false)));
+
+INSTANTIATE_TEST_CASE_P(
+    DoubleFrameOpusPackets,
+    OpusWebMClusterParserTest,
+    ::testing::Combine(
+        ::testing::Range(0, kNumPossibleOpusConfigs),
+        ::testing::Values(2),
+        // Lets VBR flag varry, as this affects TOC config for 2 frame packets.

[wolenetz, 2015/01/30 20:46:12]

nit: varry spelling

[chcunningham, 2015/02/03 20:34:43]

Done.

+        ::testing::Bool()));
+
+INSTANTIATE_TEST_CASE_P(
+    MultiFrameOpusPackets,
+    OpusWebMClusterParserTest,
+    ::testing::Combine(::testing::Range(0, kNumPossibleOpusConfigs),
+                       ::testing::Range(3, kMaxOpusPacketFrameCount),
+                       ::testing::Values(false)));
+
 }  // namespace media