Return buffers from StreamParsers in a single unified map

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 a87854f09d7302607b1da56d7fbdb2b76ead0a08..f748039af16e2de18a46cfc1fea3915964242cf8 100644
--- a/media/formats/webm/webm_cluster_parser_unittest.cc
+++ b/media/formats/webm/webm_cluster_parser_unittest.cc
@@ -179,13 +179,12 @@ std::unique_ptr<Cluster> CreateEncryptedCluster(int bytes_to_write) {
   return cb.Finish();
 }
 
-bool VerifyBuffers(const WebMClusterParser::BufferQueue& audio_buffers,
-                   const WebMClusterParser::BufferQueue& video_buffers,
-                   const WebMClusterParser::BufferQueue& text_buffers,
+bool VerifyBuffers(const StreamParser::BufferQueueMap& buffer_queue_map,
                    const BlockInfo* block_info,
                    int block_count) {
-  int buffer_count = audio_buffers.size() + video_buffers.size() +
-      text_buffers.size();
+  int buffer_count = 0;
+  for (const auto& it : buffer_queue_map)
+    buffer_count += it.second.size();
   if (block_count != buffer_count) {
     DVLOG(1) << __FUNCTION__ << " : block_count (" << block_count
              << ") mismatches buffer_count (" << buffer_count << ")";
@@ -196,20 +195,20 @@ bool VerifyBuffers(const WebMClusterParser::BufferQueue& audio_buffers,
   size_t video_offset = 0;
   size_t text_offset = 0;
   for (int i = 0; i < block_count; i++) {
-    const WebMClusterParser::BufferQueue* buffers = NULL;
+    const StreamParser::BufferQueue* buffers = NULL;
     size_t* offset;
     StreamParserBuffer::Type expected_type = DemuxerStream::UNKNOWN;
 
+    const auto& it = buffer_queue_map.find(block_info[i].track_num);
+    EXPECT_NE(buffer_queue_map.end(), it);
+    buffers = &it->second;
     if (block_info[i].track_num == kAudioTrackNum) {
-      buffers = &audio_buffers;
       offset = &audio_offset;
       expected_type = DemuxerStream::AUDIO;
     } else if (block_info[i].track_num == kVideoTrackNum) {
-      buffers = &video_buffers;
       offset = &video_offset;
       expected_type = DemuxerStream::VIDEO;
     } else if (block_info[i].track_num == kTextTrackNum) {
-      buffers = &text_buffers;
       offset = &text_offset;
       expected_type = DemuxerStream::TEXT;
     } else {
@@ -240,20 +239,9 @@ bool VerifyBuffers(const WebMClusterParser::BufferQueue& audio_buffers,
 bool VerifyBuffers(const std::unique_ptr<WebMClusterParser>& parser,
                    const BlockInfo* block_info,
                    int block_count) {
-  const WebMClusterParser::TextBufferQueueMap& text_map =
-      parser->GetTextBuffers();
-  const WebMClusterParser::BufferQueue* text_buffers;
-  const WebMClusterParser::BufferQueue no_text_buffers;
-  if (!text_map.empty())
-    text_buffers = &(text_map.rbegin()->second);
-  else
-    text_buffers = &no_text_buffers;
-
-  return VerifyBuffers(parser->GetAudioBuffers(),
-                       parser->GetVideoBuffers(),
-                       *text_buffers,
-                       block_info,
-                       block_count);
+  StreamParser::BufferQueueMap buffers;
+  parser->GetBuffers(&buffers);
+  return VerifyBuffers(buffers, block_info, block_count);
 }
 
 bool VerifyTextBuffers(const std::unique_ptr<WebMClusterParser>& parser,
@@ -294,14 +282,6 @@ void VerifyEncryptedBuffer(scoped_refptr<StreamParserBuffer> buffer) {
             buffer->decrypt_config()->iv().length());
 }
 
-void AppendToEnd(const WebMClusterParser::BufferQueue& src,
-                 WebMClusterParser::BufferQueue* dest) {
-  for (WebMClusterParser::BufferQueue::const_iterator itr = src.begin();
-       itr != src.end(); ++itr) {
-    dest->push_back(*itr);
-  }
-}
-
 }  // namespace
 
 class WebMClusterParserTest : public testing::Test {
@@ -515,10 +495,6 @@ TEST_F(WebMClusterParserTest, ParseClusterWithMultipleCalls) {
   std::unique_ptr<Cluster> cluster(
       CreateCluster(0, kDefaultBlockInfo, block_count));
 
-  WebMClusterParser::BufferQueue audio_buffers;
-  WebMClusterParser::BufferQueue video_buffers;
-  const WebMClusterParser::BufferQueue no_text_buffers;
-
   const uint8_t* data = cluster->data();
   int size = cluster->size();
   int default_parse_size = 3;
@@ -536,17 +512,14 @@ TEST_F(WebMClusterParserTest, ParseClusterWithMultipleCalls) {
       continue;
     }
 

[wolenetz, 2016/08/23 21:31:21]

The point of this unit test is to verify the behavior of incremental parses,
which involve incremental GetBuffers(). Please restore that to this test.

[servolk, 2016/08/23 23:56:32]

Wait, perhaps I'm missing something. This test still does multiple calls to
parser_->Parse. As far as I can see AppendToEnd haven't done any additional
verification of those intermediate results, and simply appended the buffers that
were ready so far to audio/video_buffers. The final verification was done
outside the loop. So I thought I've preserved the intended behavior here, no?

[wolenetz, 2016/08/24 21:57:51]

Hmm. I can see how this was confusing. WebMStreamParser executes incremental
parses, with getbuffers between each. This test was intended to mimic that
behavior. Especially on a muxed cluster, the "ready buffers" calculations are
important to not emit any frames with timestamp greater than any
parsed-but-with-duration-not-known-yet frames in the cluster. At cluster
boundary, all such frames are known. Incremental getBuffers tests the common
case where a full cluster might not be appended to the stream parser yet;
conforming to the reality where each Parse is interleaved with a GetBuffers.
Other tests in this file overlap this a little, but with specific cases around
where durations might be estimated or unknown until cluster end. Please restore
incremental getbuffers.

[servolk, 2016/08/24 23:17:44]

Ah, ok, restored calling GetBuffers after each Parse.

-    AppendToEnd(parser_->GetAudioBuffers(), &audio_buffers);
-    AppendToEnd(parser_->GetVideoBuffers(), &video_buffers);
-
     parse_size = default_parse_size;
 
     data += result;
     size -= result;
   }
-  ASSERT_TRUE(VerifyBuffers(audio_buffers, video_buffers,
-                            no_text_buffers, kDefaultBlockInfo,
-                            block_count));
+  StreamParser::BufferQueueMap buffers;
+  parser_->GetBuffers(&buffers);
+  ASSERT_TRUE(VerifyBuffers(buffers, kDefaultBlockInfo, block_count));
 }
 
 // Verify that both BlockGroups with the BlockDuration before the Block
@@ -748,8 +721,10 @@ TEST_F(WebMClusterParserTest, ParseEncryptedBlock) {
 
   int result = parser_->Parse(cluster->data(), cluster->size());
   EXPECT_EQ(cluster->size(), result);
-  ASSERT_EQ(1UL, parser_->GetVideoBuffers().size());
-  scoped_refptr<StreamParserBuffer> buffer = parser_->GetVideoBuffers()[0];
+  StreamParser::BufferQueueMap buffers;
+  parser_->GetBuffers(&buffers);
+  EXPECT_EQ(1UL, buffers[kVideoTrackNum].size());
+  scoped_refptr<StreamParserBuffer> buffer = buffers[kVideoTrackNum][0];
   VerifyEncryptedBuffer(buffer);
 }
 
@@ -879,8 +854,10 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsSimpleBlocks) {
   EXPECT_GT(result, 0);
   EXPECT_LT(result, cluster1->size());
   ASSERT_TRUE(VerifyBuffers(parser_, kBlockInfo1, block_count1 - 3));
-  EXPECT_EQ(3UL, parser_->GetAudioBuffers().size());
-  EXPECT_EQ(1UL, parser_->GetVideoBuffers().size());
+  StreamParser::BufferQueueMap buffers;
+  parser_->GetBuffers(&buffers);
+  EXPECT_EQ(3UL, buffers[kAudioTrackNum].size());
+  EXPECT_EQ(1UL, buffers[kVideoTrackNum].size());
 
   parser_->Reset();
 
@@ -951,8 +928,10 @@ TEST_F(WebMClusterParserTest, ParseWithoutAnyDurationsBlockGroups) {
   EXPECT_GT(result, 0);
   EXPECT_LT(result, cluster1->size());
   ASSERT_TRUE(VerifyBuffers(parser_, kBlockInfo1, block_count1 - 3));
-  EXPECT_EQ(3UL, parser_->GetAudioBuffers().size());
-  EXPECT_EQ(1UL, parser_->GetVideoBuffers().size());
+  StreamParser::BufferQueueMap buffers;
+  parser_->GetBuffers(&buffers);
+  EXPECT_EQ(3UL, buffers[kAudioTrackNum].size());
+  EXPECT_EQ(1UL, buffers[kVideoTrackNum].size());
 
   parser_->Reset();