FrameBuffer2 now has Start/Stop methods.

webrtc/modules/video_coding/frame_buffer2.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 29 matching lines @@
   return AheadOf(f2.first, f1.first);
 }
 
 FrameBuffer::FrameBuffer(Clock* clock,
                          VCMJitterEstimator* jitter_estimator,
                          const VCMTiming* timing)
     : 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) {
+    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;
-
-    crit_.Enter();
-    frame_inserted_event_.Reset();
     auto next_frame = frames_.end();
     for (auto frame_it = frames_.begin(); frame_it != frames_.end();
          ++frame_it) {
       const FrameObject& frame = *frame_it->second;
       if (IsContinuous(frame)) {
         next_frame = frame_it;
         int64_t render_time = timing_->RenderTimeMs(frame.timestamp, now);
         wait_ms = timing_->MaxWaitingTime(render_time, now);
 
         // This will cause the frame buffer to prefer high framerate rather
@@ skipping 23 matching lines @@
         crit_.Leave();
         return frame;
       } else {
         crit_.Leave();
         return std::unique_ptr<FrameObject>();
       }
     }
   }
 }
 
+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)
     newest_picture_id_ = frame->picture_id;
 
   if (AheadOf<uint16_t>(frame->picture_id, newest_picture_id_))
     newest_picture_id_ = frame->picture_id;
 
   // Remove frames as long as we have too many, |kMaxNumHistoryFrames|.
   while (decoded_frames_.size() > kMaxNumHistoryFrames)
@@ skipping 32 matching lines @@
     FrameKey ref_key(frame.picture_id, frame.spatial_layer - 1);
     if (decoded_frames_.find(ref_key) == decoded_frames_.end())
       return false;
   }
 
   return true;
 }
 
 }  // namespace video_coding
 }  // namespace webrtc