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"
 #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);
 
   // 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
+  //    an optional with a pointer to the frame.
+  //  - If no frame is available after |max_wait_time_ms| it will return
+  //    an optional with a nullptr value.
+  //  - If the FrameBuffer is stopped then it will return an optional

[danilchap, 2016/08/31 15:04:36]

I do not think rtc::Optional is a good tool to return reason why FrameBuffer
can't return a FrameObject.

Do you need to keep and handle empty FrameObjects?
may be create and return special kind of FrameObjects then.
it will take more space, but might be more readable.

Don't you have other, straightforward way to find out FrameBuffer is stopped?
May be add it.

or have an additional parameter Reason/Status to NextFrame:
enum Reason {
  OK = 200,
  FRAME_NOT_FOUND_IN_TIME = 408,
  END_OF_FRAME_BUFFER = 410
};
std::unique_ptr<FrameObject> NextFrame(int64_t wait_time, Reason* status);
or reverse:
Reason NextFrame(int64_t wait_time, std::unique_ptr<FrameObject>* frame)
or keep same input parameters:
std::tuple<std::unique_ptr<FrameObject>, Reason> NextFrame(int64_t wait_time);

[philipel, 2016/09/01 09:25:23]

I'm not sure I understand why rtc::Optional is the wrong tool in this case,
could you expand on that?

There is always a reason why a function might return an empty optional, but as
long as it is only for one specific reason and not due to multiple different
reasons I think it makes sense.

[danilchap, 2016/09/01 09:44:09]

You have three possible outcomes for this function, i.e. two distinct reasons to
return no frame.

Optional is good when you have binary outcome/one reason to fail (or reason
doesn't matter).
One way to see an optional is as an alternative to a pointer way to return 'no
value'.
NextFrame's return type is already a pointer.
So with all smart types removed it looks like
FrameObject** NextFrame(wait_time);
where returning nullptr means one thing and returning pointer to nullptr means a
different thing.

+  //    without a value.
+  rtc::Optional<std::unique_ptr<FrameObject>>
+      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_