Reuse allocated encoders in SimulcastEncoderAdapter.

webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h

 /*
  *  Copyright (c) 2014 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_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_
 #define WEBRTC_MODULES_VIDEO_CODING_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_
 
 #include <memory>
+#include <stack>
 #include <string>
+#include <utility>
 #include <vector>
 
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/sequenced_task_checker.h"
 #include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h"
 
 namespace webrtc {
 
 class SimulcastRateAllocator;
 
+// TODO(brandtr): Remove this class and replace its use with a
+// WebRtcVideoEncoderFactory.
 class VideoEncoderFactory {
  public:
   virtual VideoEncoder* Create() = 0;
   virtual void Destroy(VideoEncoder* encoder) = 0;
   virtual ~VideoEncoderFactory() {}
 };
 
 // SimulcastEncoderAdapter implements simulcast support by creating multiple
 // webrtc::VideoEncoder instances with the given VideoEncoderFactory.
-// All the public interfaces are expected to be called from the same thread,
-// e.g the encoder thread.
+// The object is created and destroyed on the worker thread, but all public
+// interfaces should be called from the encoder task queue.
 class SimulcastEncoderAdapter : public VP8Encoder {
  public:
   explicit SimulcastEncoderAdapter(VideoEncoderFactory* factory);
   virtual ~SimulcastEncoderAdapter();
 
-  // Implements VideoEncoder
+  // Implements VideoEncoder.
   int Release() override;
   int InitEncode(const VideoCodec* inst,
                  int number_of_cores,
                  size_t max_payload_size) override;
   int Encode(const VideoFrame& input_image,
              const CodecSpecificInfo* codec_specific_info,
              const std::vector<FrameType>* frame_types) override;
   int RegisterEncodeCompleteCallback(EncodedImageCallback* callback) override;
   int SetChannelParameters(uint32_t packet_loss, int64_t rtt) override;
   int SetRateAllocation(const BitrateAllocation& bitrate,
                         uint32_t new_framerate) override;
 
   // Eventual handler for the contained encoders' EncodedImageCallbacks, but
   // called from an internal helper that also knows the correct stream
   // index.
   EncodedImageCallback::Result OnEncodedImage(
       size_t stream_idx,
       const EncodedImage& encoded_image,
       const CodecSpecificInfo* codec_specific_info,
       const RTPFragmentationHeader* fragmentation);
 
   VideoEncoder::ScalingSettings GetScalingSettings() const override;
 
   bool SupportsNativeHandle() const override;
   const char* ImplementationName() const override;
 
  private:
   struct StreamInfo {
-    StreamInfo()
-        : encoder(NULL),
-          callback(NULL),
-          width(0),
-          height(0),
-          key_frame_request(false),
-          send_stream(true) {}
     StreamInfo(VideoEncoder* encoder,
-               EncodedImageCallback* callback,
+               std::unique_ptr<EncodedImageCallback> callback,
                uint16_t width,
                uint16_t height,
                bool send_stream)
         : encoder(encoder),
-          callback(callback),
+          callback(std::move(callback)),
           width(width),
           height(height),
           key_frame_request(false),
           send_stream(send_stream) {}
-    // Deleted by SimulcastEncoderAdapter::Release().
+    // Deleted by SimulcastEncoderAdapter::DestroyStoredEncoders().
     VideoEncoder* encoder;
-    EncodedImageCallback* callback;
+    std::unique_ptr<EncodedImageCallback> callback;
     uint16_t width;
     uint16_t height;
     bool key_frame_request;
     bool send_stream;
   };
 
-  // Populate the codec settings for each stream.
-  void PopulateStreamCodec(const webrtc::VideoCodec* inst,
-                           int stream_index,
-                           uint32_t start_bitrate_kbps,
-                           bool highest_resolution_stream,
-                           webrtc::VideoCodec* stream_codec);
+  // Populate the codec settings for each simulcast stream.
+  static void PopulateStreamCodec(const webrtc::VideoCodec& inst,
+                                  int stream_index,
+                                  uint32_t start_bitrate_kbps,
+                                  bool highest_resolution_stream,
+                                  webrtc::VideoCodec* stream_codec);
 
   bool Initialized() const;
 
-  std::unique_ptr<VideoEncoderFactory> factory_;
+  void DestroyStoredEncoders();
+
+#if RTC_DCHECK_IS_ON
+  // Used for protecting |inited_| in debug builds. This is necessary for the
+  // DHECK in the destructor, as the object is created/destroyed on a different
+  // thread that it is being operated on.
+  rtc::CriticalSection inited_crit_;
+#endif

[sprang_webrtc, 2017/05/11 11:53:36]

Maybe you can use an atomic instead?

[brandtr, 2017/05/15 10:12:01]

Would be nice to not have this lock here at all. I'm not super familiar with
atomics, however. Could I just use rtc::AtomicOps::AcquireLoad and
rtc::AtomicOps::ReleaseStore on a volatile int, or do I need to use compare and
swap?

[sprang_webrtc, 2017/05/15 12:03:39]

rtc::AtomicOps::AcquireLoad and rtc::AtomicOps::ReleaseStore should be fine,
since the only thing you're really interested in is the memory barriers; you
don't need any atomic conditional changes.

+
+  bool inited_;
+  const std::unique_ptr<VideoEncoderFactory> factory_;
   VideoCodec codec_;
   std::vector<StreamInfo> streaminfos_;
   EncodedImageCallback* encoded_complete_callback_;
   std::string implementation_name_;
+
+  // Used for checking the single-threaded access of the encoder interface.
+  rtc::SequencedTaskChecker encoder_queue_;
+
+  // Store encoders in between calls to Release and InitEncode, so they don't
+  // have to be recreated. Remaining encoders are destroyed by the destructor.
+  std::stack<VideoEncoder*> stored_encoders_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_VIDEO_CODING_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_