FrameBuffer::NextFrame now return pair<frame, reason>.

webrtc/modules/video_coding/frame_buffer2.h

Index: webrtc/modules/video_coding/frame_buffer2.h
diff --git a/webrtc/modules/video_coding/frame_buffer2.h b/webrtc/modules/video_coding/frame_buffer2.h
index 0af2bf952350bc76674f5f42dcbf8f86fff8e80e..5d238c84fbd6c6487353a0203298ee787bb288a9 100644
--- a/webrtc/modules/video_coding/frame_buffer2.h
+++ b/webrtc/modules/video_coding/frame_buffer2.h
@@ -20,6 +20,7 @@
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/event.h"
+#include "webrtc/base/optional.h"

[danilchap, 2016/09/01 11:15:21]

not needed

[philipel, 2016/09/01 11:38:02]

Done.

 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
 #include "webrtc/modules/video_coding/inter_frame_delay.h"
@@ -40,13 +41,25 @@ class FrameBuffer {
               VCMJitterEstimator* jitter_estimator,
               VCMTiming* timing);
 
+  enum ReturnReason {

[danilchap, 2016/09/01 11:15:21]

enum should go before constructor.
Probably make it enum class.
ReturnStatus or FrameStatus probably better names (I'll take previously
suggested name Reason back. this type represent more than just a Reason for not
returning a frame)

[philipel, 2016/09/01 11:38:03]

Acknowledged.

+    kFrameFound,

[danilchap, 2016/09/01 11:15:21]

if you choose FrameStatus for the enum type, then this status probably better
rename to kFound

[philipel, 2016/09/01 11:38:03]

Acknowledged.

+    kTimeout,
+    kStopped
+  };
+
   // Insert a frame into the frame buffer.
   void InsertFrame(std::unique_ptr<FrameObject> frame);
 
   // Get the next frame for decoding. Will return at latest after
-  // |max_wait_time_ms|, with either a managed FrameObject or an empty
-  // unique ptr if there is no available frame for decoding.
-  std::unique_ptr<FrameObject> NextFrame(int64_t max_wait_time_ms);
+  // |max_wait_time_ms|.
+  //  - If a frame is availiable within |max_wait_time_ms| it will return
+  //    {kFrameFound, non-null unique ptr}
+  //  - If no frame is available after |max_wait_time_ms| it will return
+  //    {kTimeout, null unique ptr}
+  //  - If the FrameBuffer is stopped then it will return
+  //    {kStopped, null unique ptr}
+  std::pair<ReturnReason, std::unique_ptr<FrameObject>>

[danilchap, 2016/09/01 11:15:21]

in stl it is more often when a pair<object, status> is returned then object goes
first, status - second (thinking about methods like std::set::insert).
Unless you try to make it consistence with something else, make it more
consistent with stl and swap types in the pair.

+      NextFrame(int64_t max_wait_time_ms);
 
   // Tells the FrameBuffer which protection mode that is in use. Affects
   // the frame timing.