Return buffers from StreamParsers in a single unified map

media/base/stream_parser_unittest.cc

Index: media/base/stream_parser_unittest.cc
diff --git a/media/base/stream_parser_unittest.cc b/media/base/stream_parser_unittest.cc
index 4aa96170124b2cdff52949f843cac42140eb967a..abb6d6555328988e130b83b42e48bdf3dc0e7ba9 100644
--- a/media/base/stream_parser_unittest.cc
+++ b/media/base/stream_parser_unittest.cc
@@ -17,7 +17,6 @@ namespace media {
 
 typedef StreamParser::TrackId TrackId;
 typedef StreamParser::BufferQueue BufferQueue;
-typedef StreamParser::TextBufferQueueMap TextBufferQueueMap;
 
 const int kEnd = -1;
 const uint8_t kFakeData[] = {0xFF};
@@ -78,36 +77,32 @@ class StreamParserTest : public testing::Test {
   // to |audio_buffers_|. See GenerateBuffers() for |decode_timestamps| format.
   void GenerateAudioBuffers(const int* decode_timestamps) {
     GenerateBuffers(decode_timestamps, DemuxerStream::AUDIO, kAudioTrackId,
-                    &audio_buffers_);
+                    &buffer_queue_map_[kAudioTrackId]);
   }
 
   // Appends test video buffers in the sequence described by |decode_timestamps|
   // to |video_buffers_|. See GenerateBuffers() for |decode_timestamps| format.
   void GenerateVideoBuffers(const int* decode_timestamps) {
     GenerateBuffers(decode_timestamps, DemuxerStream::VIDEO, kVideoTrackId,
-                    &video_buffers_);
+                    &buffer_queue_map_[kVideoTrackId]);
   }
 
   // Current tests only need up to two distinct text BufferQueues. This helper
-  // conditionally appends buffers to the underlying |text_buffers_a_| and
-  // |text_buffers_b_| and conditionally inserts these BufferQueues into
-  // |text_map_| keyed by the respective track ID. If |decode_timestamps_{a,b}|
-  // is NULL, then the corresponding BufferQueue is neither appended to nor
-  // inserted into |text_map_| (though it may previously have been inserted).
+  // conditionally appends buffers to the underlying |buffer_queue_map_| keyed
+  // by the respective track ID. If |decode_timestamps_{a,b}|
+  // is NULL, then the corresponding BufferQueue is not changed at all.
   // Note that key collision on map insertion does not replace the previous
   // value.
   void GenerateTextBuffers(const int* decode_timestamps_a,
                            const int* decode_timestamps_b) {
     if (decode_timestamps_a) {
       GenerateBuffers(decode_timestamps_a, DemuxerStream::TEXT, kTextTrackIdA,
-                      &text_buffers_a_);
-      text_map_.insert(std::make_pair(kTextTrackIdA, text_buffers_a_));
+                      &buffer_queue_map_[kTextTrackIdA]);
     }
 
     if (decode_timestamps_b) {
       GenerateBuffers(decode_timestamps_b, DemuxerStream::TEXT, kTextTrackIdB,
-                      &text_buffers_b_);
-      text_map_.insert(std::make_pair(kTextTrackIdB, text_buffers_b_));
+                      &buffer_queue_map_[kTextTrackIdB]);
     }
   }
 
@@ -176,8 +171,11 @@ class StreamParserTest : public testing::Test {
     size_t original_video_in_merged = CountMatchingMergedBuffers(IsVideo);
     size_t original_text_in_merged = CountMatchingMergedBuffers(IsText);
 
-    EXPECT_TRUE(MergeBufferQueues(audio_buffers_, video_buffers_, text_map_,
-                                  &merged_buffers_));
+    StreamParser::BufferQueueMap buffer_queue_map;
+    for (const auto& it : buffer_queue_map_)

[DaleCurtis, 2016/08/19 01:06:25]

Needs {} -- again it seems like there should be an emplace/insert method for
doing this.

[servolk, 2016/08/19 01:29:29]

Sure I can add {}.
According to https://chromium-cpp.appspot.com/:
std::map::emplace() is not yet available on all libstdc++ versions we support.

I've just tried and indeed std::map::emplace is not found:
../../media/base/stream_parser_unittest.cc:178:26: error: no member named
'emplace' in 'std::__debug::map<int,
std::__debug::deque<scoped_refptr<media::StreamParserBuffer>,
std::allocator<scoped_refptr<media::StreamParserBuffer> > >, std::less<int>,
std::allocator<std::pair<const int,
std::__debug::deque<scoped_refptr<media::StreamParserBuffer>,
std::allocator<scoped_refptr<media::StreamParserBuffer> > > > > >'
        buffer_queue_map.emplace(it.first, it.second);
        ~~~~~~~~~~~~~~~~ ^

+      if (!it.second.empty())
+        buffer_queue_map.insert(std::make_pair(it.first, it.second));
+    EXPECT_TRUE(MergeBufferQueues(buffer_queue_map, &merged_buffers_));
 
     // Verify resulting contents of |merged_buffers| matches |expected|.
     EXPECT_EQ(expected,
@@ -193,17 +191,14 @@ class StreamParserTest : public testing::Test {
     EXPECT_GE(video_in_merged, original_video_in_merged);
     EXPECT_GE(text_in_merged, original_text_in_merged);
 
-    EXPECT_EQ(audio_buffers_.size(),
+    EXPECT_EQ(buffer_queue_map_[kAudioTrackId].size(),
               audio_in_merged - original_audio_in_merged);
-    EXPECT_EQ(video_buffers_.size(),
+    EXPECT_EQ(buffer_queue_map_[kVideoTrackId].size(),
               video_in_merged - original_video_in_merged);
 
     size_t expected_text_buffer_count = 0;
-    for (TextBufferQueueMap::const_iterator itr = text_map_.begin();
-         itr != text_map_.end();
-         ++itr) {
-      expected_text_buffer_count += itr->second.size();
-    }
+    expected_text_buffer_count += buffer_queue_map_[kTextTrackIdA].size();
+    expected_text_buffer_count += buffer_queue_map_[kTextTrackIdB].size();
     EXPECT_EQ(expected_text_buffer_count,
               text_in_merged - original_text_in_merged);
   }
@@ -211,28 +206,23 @@ class StreamParserTest : public testing::Test {
   // Verifies that MergeBufferQueues() of the current |audio_buffers_|,
   // |video_buffers_|, |text_map_|, and |merged_buffers_| returns false.
   void VerifyMergeFailure() {
-    EXPECT_FALSE(MergeBufferQueues(audio_buffers_, video_buffers_, text_map_,
-                                   &merged_buffers_));
+    StreamParser::BufferQueueMap buffer_queue_map;
+    for (const auto& it : buffer_queue_map_)

[DaleCurtis, 2016/08/19 01:06:25]

needs {}, ditto.

[servolk, 2016/08/19 01:29:29]

Acknowledged.

+      if (!it.second.empty())
+        buffer_queue_map.insert(std::make_pair(it.first, it.second));
+    EXPECT_FALSE(MergeBufferQueues(buffer_queue_map, &merged_buffers_));
   }
 
   // Helper to allow tests to clear all the input BufferQueues (except
-  // |merged_buffers_|) and the TextBufferQueueMap that are used in
+  // |merged_buffers_|) and the BufferQueueMap that are used in
   // VerifyMerge{Success/Failure}().
   void ClearQueuesAndTextMapButKeepAnyMergedBuffers() {
-    audio_buffers_.clear();
-    video_buffers_.clear();
-    text_buffers_a_.clear();
-    text_buffers_b_.clear();
-    text_map_.clear();
+    buffer_queue_map_.clear();
   }
 
  private:
-  BufferQueue audio_buffers_;
-  BufferQueue video_buffers_;
-  BufferQueue text_buffers_a_;
-  BufferQueue text_buffers_b_;
+  std::map<StreamParser::TrackId, StreamParser::BufferQueue> buffer_queue_map_;
   BufferQueue merged_buffers_;
-  TextBufferQueueMap text_map_;
 
   DISALLOW_COPY_AND_ASSIGN(StreamParserTest);
 };