Implement timing frames.

webrtc/modules/video_coding/generic_encoder.h

 /*
  *  Copyright (c) 2012 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_GENERIC_ENCODER_H_
 #define WEBRTC_MODULES_VIDEO_CODING_GENERIC_ENCODER_H_
 
 #include <stdio.h>
+#include <map>
 #include <vector>
 
 #include "webrtc/modules/video_coding/include/video_codec_interface.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
 
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/race_checker.h"
 
 namespace webrtc {
 
@@ skipping 12 matching lines @@
  public:
   VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback,
                           media_optimization::MediaOptimization* media_opt);
   virtual ~VCMEncodedFrameCallback();
 
   // Implements EncodedImageCallback.
   EncodedImageCallback::Result OnEncodedImage(
       const EncodedImage& encoded_image,
       const CodecSpecificInfo* codec_specific_info,
       const RTPFragmentationHeader* fragmentation) override;
+
   void SetInternalSource(bool internal_source) {
     internal_source_ = internal_source;
   }
 
+  // Timing frames configuration methods. These 4 should be called before
+  // |OnEncodedImage| at least once.
+  void OnTargetBitrateChanged(size_t bitrate_bytes_per_sec,
+                              size_t simulcast_svc_idx);
+
+  void OnFrameRateChanged(size_t framerate);
+
+  void OnEncodeStarted(int64_t capture_time_ms, size_t simulcast_svc_idx);
+
+  void SetTimingFramesThresholds(
+      const VideoCodec::TimingFrameTriggerThresholds& thresholds) {
+    timing_frames_thresholds_ = thresholds;
+  }
+
  private:
   bool internal_source_;
   EncodedImageCallback* const post_encode_callback_;
   media_optimization::MediaOptimization* const media_opt_;
+
+  struct TimingFramesLayerInfo {
+    size_t target_bitrate_bytes_per_sec = 0;
+    std::map<int64_t, int64_t> encode_start_time_ms;
+  };
+  // Separate instance for each simulcast stream or spatial layer.
+  std::vector<TimingFramesLayerInfo> timing_frames_info_;
+  size_t framerate_;
+  int64_t last_timing_frame_time_ms_;
+  VideoCodec::TimingFrameTriggerThresholds timing_frames_thresholds_;

[sprang_webrtc, 2017/06/09 11:02:01]

Looks like there are potential raciness here, as rate setters etc are called by
the vie encoder queue (right?) but OnEncodedImage() is potentially called
asynchronously by an encoder-internal thread.
Add locks and annotate as appropriate.

[ilnik, 2017/06/09 12:06:05]

I think, there's no raciness here, as most methods of vcm_encoded_image are
called from methods of GenericEncoder directly, like OnFrameRateChanged,
OnEncodeStarted and OnTargetBitrateChanged. Only OnEncodedImage() is called from
encoder_->Encode(), which is called by GenericEncoder::Encode().
GenericEncoder::Encode(), SetEncoderParameters(), and RequestFrame() are all
protected by RTC_DCHECK_RUNS_SERIALIZED(&race_checker_) inside GenericEncoder.

I agree, that it must be documented, so I added a comment.

[sprang_webrtc, 2017/06/12 07:10:10]

OnEncodedImage() is called by the encoder, but there is no guarantee that it's
called by the same thread as the one that called Encode(). For the default
software encoders it is currently the case, but for external encoders it need
not be.

[ilnik, 2017/06/12 09:06:24]

Ok, Added protection using critical section.

 };

[sprang_webrtc, 2017/06/09 11:02:01]

This logic definitely warrants units tests as well. Pretty bad that don't
currently have a generic_encoder_unittests.cc, could you add one? No need to add
tests for the existing logic in this cl though.

[ilnik, 2017/06/09 12:06:06]

I am writing a second CL with tests in parallel. Generic encoder utittest will
be added there, Is that OK?

[sprang_webrtc, 2017/06/12 07:10:10]

I'd actually prefer they are in the same one. If it becomes too large it's
better to split the cl by functionality instead. Adding unit tests later usually
just means finding out the problems later.

[ilnik, 2017/06/12 09:06:24]

Working on it.

[ilnik, 2017/06/12 10:08:50]

Added generic_encoder_unittests and also another test to
video_send_stream_tests.

 
 class VCMGenericEncoder {
   friend class VCMCodecDataBase;
 
  public:
   VCMGenericEncoder(VideoEncoder* encoder,
                     VCMEncodedFrameCallback* encoded_frame_callback,
                     bool internal_source);
   ~VCMGenericEncoder();
   int32_t Release();
@@ skipping 15 matching lines @@
 
  private:
   rtc::RaceChecker race_checker_;
 
   VideoEncoder* const encoder_ GUARDED_BY(race_checker_);
   VCMEncodedFrameCallback* const vcm_encoded_frame_callback_;
   const bool internal_source_;
   rtc::CriticalSection params_lock_;
   EncoderParameters encoder_params_ GUARDED_BY(params_lock_);
   bool is_screenshare_;
+  size_t streams_or_svc_num_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_VIDEO_CODING_GENERIC_ENCODER_H_