Return buffers from StreamParsers in a single unified map

media/filters/frame_processor_unittest.cc

Index: media/filters/frame_processor_unittest.cc
diff --git a/media/filters/frame_processor_unittest.cc b/media/filters/frame_processor_unittest.cc
index d051e8e58aeba450e30f493f90bb333c092c529d..353635b723d5303dfd21085074505268d22b0b75 100644
--- a/media/filters/frame_processor_unittest.cc
+++ b/media/filters/frame_processor_unittest.cc
@@ -33,7 +33,6 @@ using ::testing::Values;
 namespace media {
 
 typedef StreamParser::BufferQueue BufferQueue;
-typedef StreamParser::TextBufferQueueMap TextBufferQueueMap;
 typedef StreamParser::TrackId TrackId;
 
 // Used for setting expectations on callbacks. Using a StrictMock also lets us
@@ -152,10 +151,17 @@ class FrameProcessorTest : public testing::TestWithParam<bool> {
 
   void ProcessFrames(const std::string& audio_timestamps,
                      const std::string& video_timestamps) {
+    StreamParser::BufferQueueMap buffer_queue_map;
+    const auto& audio_buffers =

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

Seems wrong to me to take the reference of retval of a method. audio_buffers
thus becomes a reference to a temporary that is no longer alive, right? (Or is
this some C++11-ism I'm missing?)
There are multiple occurrences of const auto& locals added in this CL that hold
retvals. Please tell me I'm wrong :) or fix.

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

Yes, it's actually pre-C++11 feature.
See
https://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-....
AFAIK base::Bind also relies on this feature, and it's been robust so far :)

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

TIL :) Thanks. I don't think your usage in this CL violates this, but "Note this
only applies to stack-based references. It doesn’t work for references that are
members of objects."

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

Acknowledged.

+        StringToBufferQueue(audio_timestamps, audio_id_, DemuxerStream::AUDIO);
+    if (!audio_buffers.empty())
+      buffer_queue_map.insert(std::make_pair(audio_id_, audio_buffers));
+    const auto& video_buffers =
+        StringToBufferQueue(video_timestamps, video_id_, DemuxerStream::VIDEO);
+    if (!video_buffers.empty())
+      buffer_queue_map.insert(std::make_pair(video_id_, video_buffers));
     ASSERT_TRUE(frame_processor_->ProcessFrames(
-        StringToBufferQueue(audio_timestamps, audio_id_, DemuxerStream::AUDIO),
-        StringToBufferQueue(video_timestamps, video_id_, DemuxerStream::VIDEO),
-        empty_text_buffers_, append_window_start_, append_window_end_,
+        buffer_queue_map, append_window_start_, append_window_end_,
         &timestamp_offset_));
   }
 
@@ -270,7 +276,6 @@ class FrameProcessorTest : public testing::TestWithParam<bool> {
   const TrackId audio_id_;
   const TrackId video_id_;
   const BufferQueue empty_queue_;
-  const TextBufferQueueMap empty_text_buffers_;
 
   // StoreStatusAndBuffer's most recent result.
   DemuxerStream::Status last_read_status_;
@@ -328,10 +333,13 @@ TEST_F(FrameProcessorTest, WrongTypeInAppendedBuffer) {
   AddTestTracks(HAS_AUDIO);
   EXPECT_FALSE(in_coded_frame_group());
 
-  ASSERT_FALSE(frame_processor_->ProcessFrames(
-      StringToBufferQueue("0K", audio_id_, DemuxerStream::VIDEO), empty_queue_,
-      empty_text_buffers_, append_window_start_, append_window_end_,
-      &timestamp_offset_));
+  StreamParser::BufferQueueMap buffer_queue_map;
+  const auto& audio_buffers =

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

ditto here and elsewhere const auto& = retval of some generator method smells
quite funny.

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

same as above

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

Acknowledged.

+      StringToBufferQueue("0K", audio_id_, DemuxerStream::VIDEO);
+  buffer_queue_map.insert(std::make_pair(audio_id_, audio_buffers));
+  ASSERT_FALSE(
+      frame_processor_->ProcessFrames(buffer_queue_map, append_window_start_,
+                                      append_window_end_, &timestamp_offset_));
   EXPECT_FALSE(in_coded_frame_group());
   EXPECT_EQ(base::TimeDelta(), timestamp_offset_);
   CheckExpectedRangesByTimestamp(audio_.get(), "{ }");
@@ -341,10 +349,13 @@ TEST_F(FrameProcessorTest, WrongTypeInAppendedBuffer) {
 TEST_F(FrameProcessorTest, NonMonotonicallyIncreasingTimestampInOneCall) {
   AddTestTracks(HAS_AUDIO);
 
-  ASSERT_FALSE(frame_processor_->ProcessFrames(
-      StringToBufferQueue("10K 0K", audio_id_, DemuxerStream::AUDIO),
-      empty_queue_, empty_text_buffers_, append_window_start_,
-      append_window_end_, &timestamp_offset_));
+  StreamParser::BufferQueueMap buffer_queue_map;
+  const auto& audio_buffers =
+      StringToBufferQueue("10K 0K", audio_id_, DemuxerStream::AUDIO);
+  buffer_queue_map.insert(std::make_pair(audio_id_, audio_buffers));
+  ASSERT_FALSE(
+      frame_processor_->ProcessFrames(buffer_queue_map, append_window_start_,
+                                      append_window_end_, &timestamp_offset_));
   EXPECT_FALSE(in_coded_frame_group());
   EXPECT_EQ(base::TimeDelta(), timestamp_offset_);
   CheckExpectedRangesByTimestamp(audio_.get(), "{ }");