FrameBuffer2 now has Start/Stop methods.

webrtc/modules/video_coding/frame_buffer2.cc

Index: webrtc/modules/video_coding/frame_buffer2.cc
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index c6a1a06e7563eeab1a96acdb1eeb4858f6d74822..8a3f43686ee65138f13e0a19002fb1da9f6b9e37 100644
--- a/webrtc/modules/video_coding/frame_buffer2.cc
+++ b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -47,16 +47,21 @@ FrameBuffer::FrameBuffer(Clock* clock,
       frame_inserted_event_(false, false),
       jitter_estimator_(jitter_estimator),
       timing_(timing),
-      newest_picture_id_(-1) {}
+      newest_picture_id_(-1),
+      stopped_(false) {}
 
 std::unique_ptr<FrameObject> FrameBuffer::NextFrame(int64_t max_wait_time_ms) {
   int64_t latest_return_time = clock_->TimeInMilliseconds() + max_wait_time_ms;
   while (true) {
-    int64_t now = clock_->TimeInMilliseconds();
-    int64_t wait_ms = max_wait_time_ms;
-
     crit_.Enter();

[pbos-webrtc, 2016/06/29 15:51:26]

Can you replace these explicit enters with rtc::CritScope? One before waiting
and one after, just add more scopes:

{
  CritScope ...;
}

wait

{ CS ...;}

[philipel, 2016/06/30 09:36:56]

Sorry, scopes wont work throughout this functions, so I'd rather keep to Enter()
and Leave() to keep things consistent.

[pbos-webrtc, 2016/06/30 09:44:07]

What do you mean? Have one scoped between line 56 here and line 83 in this
patchset. Then one inside the scope at line 90.

All other functions use CritScope, this would make it more consistent?

[philipel, 2016/06/30 11:37:35]

The problem is the |next_frame| iterator which we need after the timeout occurs.
If we use a CritScope then |next_frame| will go out of scope as we leave the
critical section, and if we declare |next_frame| outside of the CritScope then
we can't assign it to frames_.end() since we must hold the lock in order to use
|frames_|, so either it would have to be an uninitialized iterator, or we have
to lock extra time when we enter the function to initialize |next_frame|.

[pbos-webrtc, 2016/06/30 12:04:53]

Can frames_' size be modified while the lock is dropped? If so I think checking
frames_.end() after reacquiring the lock is not safe?

If you call InsertFrame while this lock is dropped that invalidates previous
iterators, right?

[philipel, 2016/06/30 12:20:04]

You can't call frames_.size() while not holding the lock and iterators are not
invalidated when a new frame is inserted/removed.

     frame_inserted_event_.Reset();
+    if (stopped_) {
+      crit_.Leave();
+      return std::unique_ptr<FrameObject>();
+    }
+
+    int64_t now = clock_->TimeInMilliseconds();
+    int64_t wait_ms = max_wait_time_ms;
     auto next_frame = frames_.end();
     for (auto frame_it = frames_.begin(); frame_it != frames_.end();
          ++frame_it) {
@@ -100,6 +105,17 @@ std::unique_ptr<FrameObject> FrameBuffer::NextFrame(int64_t max_wait_time_ms) {
   }
 }
 
+void FrameBuffer::Start() {
+  rtc::CritScope lock(&crit_);
+  stopped_ = false;
+}
+
+void FrameBuffer::Stop() {
+  rtc::CritScope lock(&crit_);
+  stopped_ = true;
+  frame_inserted_event_.Set();
+}
+
 void FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
   rtc::CritScope lock(&crit_);
   if (newest_picture_id_ == -1)