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

webrtc/modules/video_coding/frame_buffer2.h

 /*
  *  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.
  */
 
 #ifndef WEBRTC_MODULES_VIDEO_CODING_FRAME_BUFFER2_H_
 #define WEBRTC_MODULES_VIDEO_CODING_FRAME_BUFFER2_H_
 
 #include <array>
 #include <map>
 #include <memory>
 #include <set>
 #include <utility>
 
 #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"
 
 namespace webrtc {
 
 class Clock;
 class VCMJitterEstimator;
 class VCMTiming;
 
 namespace video_coding {
 
 class FrameObject;
 
 class FrameBuffer {
  public:
   FrameBuffer(Clock* clock,
               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.
   // TODO(philipel): Remove this when new timing calculations has been
   //                 implemented.
   void SetProtectionMode(VCMVideoProtection mode);
 
   // Start the frame buffer, has no effect if the frame buffer is started.
   // The frame buffer is started upon construction.
   void Start();
@@ skipping 33 matching lines @@
   bool stopped_ GUARDED_BY(crit_);
   VCMVideoProtection protection_mode_ GUARDED_BY(crit_);
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(FrameBuffer);
 };
 
 }  // namespace video_coding
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_VIDEO_CODING_FRAME_BUFFER2_H_