Implement timestamp translation/filter in VideoCapturer.

webrtc/media/base/videocapturer.h

 /*
  *  Copyright (c) 2010 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 207 matching lines @@
   void RemoveSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink) override;
 
  protected:
   // OnSinkWantsChanged can be overridden to change the default behavior
   // when a sink changes its VideoSinkWants by calling AddOrUpdateSink.
   virtual void OnSinkWantsChanged(const rtc::VideoSinkWants& wants);
 
   // Reports the appropriate frame size after adaptation. Returns true
   // if a frame is wanted. Returns false if there are no interested
   // sinks, or if the VideoAdapter decides to drop the frame.
+  // |capture_time_ns| is the camera's timestamp, using an arbitrary
+  // offset and possibly slightly off frequency. |time_us| is the
+  // timestamp translated into the same scale as rtc::TimeMicros.

[pthatcher1, 2016/05/27 22:47:44]

Then why not call it capture_time_us?  

And why require passing both in?  Shouldn't passing one in be sufficient?

[nisse-webrtc, 2016/05/30 08:57:29]

The nanosecond unit on the input argument, |capture_time_ns|, is kept for now
only to make the change a little smaller; it would be more consistent to use
microsecond unit here too.

The output argument is that time translated into rtc::TimeMicros timescale.

To eliminate the call to rtc::TimeMicros from the VideoCapturer class (using
global state complicates testing), I'm considering adding a second input
argument, which is the current rtc::TimeMicros, or, preferably, the value as
rtc::TimeMicros read as soon as possibly when the application gets the frame.

To recap, the model here is that the capture timestamp has pretty good accuracy,
in its own unknown timescale, while reading rtc::Time may exhibit jitter of
several ms due to scheduling or poor accuracy in the OS clock.

   bool AdaptFrame(int width,
                   int height,
                   int64_t capture_time_ns,
                   int* out_width,
                   int* out_height,
                   int* crop_width,
                   int* crop_height,
                   int* crop_x,
-                  int* crop_y);
+                  int* crop_y,
+                  int64_t* time_us);
 
   // Callback attached to SignalFrameCaptured where SignalVideoFrames is called.
   void OnFrameCaptured(VideoCapturer* video_capturer,
                        const CapturedFrame* captured_frame);
 
   // Called when a frame has been captured and converted to a
   // VideoFrame. OnFrame can be called directly by an implementation
   // that does not use SignalFrameCaptured or OnFrameCaptured. The
   // orig_width and orig_height are used only to produce stats.
   void OnFrame(const VideoFrame& frame, int orig_width, int orig_height);
@@ skipping 30 matching lines @@
   std::string ToString(const CapturedFrame* frame) const;
 
   // Updates filtered_supported_formats_ so that it contains the formats in
   // supported_formats_ that fulfill all applied restrictions.
   void UpdateFilteredSupportedFormats();
   // Returns true if format doesn't fulfill all applied restrictions.
   bool ShouldFilterFormat(const VideoFormat& format) const;
 
   void UpdateInputSize(int width, int height);
 
+  void UpdateOffset(int64_t capture_time_us);

[pthatcher1, 2016/05/27 22:47:44]

This method could use a comment explaining what it does.  It's not obvious
looking at it.

[nisse-webrtc, 2016/05/30 08:57:29]

Since it's a private method, I put the explanatory comment in the implementation
file. I'm adding a short comment in the header too.

+
   rtc::ThreadChecker thread_checker_;
   std::string id_;
   CaptureState capture_state_;
   std::unique_ptr<VideoFrameFactory> frame_factory_;
   std::unique_ptr<VideoFormat> capture_format_;
   std::vector<VideoFormat> supported_formats_;
   std::unique_ptr<VideoFormat> max_format_;
   std::vector<VideoFormat> filtered_supported_formats_;
 
   bool enable_camera_list_;
   int scaled_width_;  // Current output size from ComputeScale.
   int scaled_height_;
 
   rtc::VideoBroadcaster broadcaster_;
   bool enable_video_adapter_;
   VideoAdapter video_adapter_;
 
   rtc::CriticalSection frame_stats_crit_;
   // The captured frame size before potential adapation.
   bool input_size_valid_ GUARDED_BY(frame_stats_crit_) = false;
   int input_width_ GUARDED_BY(frame_stats_crit_);
   int input_height_ GUARDED_BY(frame_stats_crit_);
 
   // Whether capturer should apply rotation to the frame before signaling it.
   bool apply_rotation_;
 
+  // State for the timestamp translation.
+  unsigned frames_seen_;
+  int64_t offset_us_;

[pthatcher1, 2016/05/27 22:47:44]

This could use a little more clarity on what it means.

[nisse-webrtc, 2016/05/30 08:57:29]

I'm adding a short comment.

+
   RTC_DISALLOW_COPY_AND_ASSIGN(VideoCapturer);
 };
 
 }  // namespace cricket
 
 #endif  // WEBRTC_MEDIA_BASE_VIDEOCAPTURER_H_