Do not encode frames in MultithreadedFakeH264Encoder after Release().

Other minor changes:
- Define locks after stuff it is protecting
- Use explicit default dtors
- Replace unnecessary lock in DelayedEncoder with SequencedTaskChecker

BUG=webrtc:7130

Review-Url: https://codereview.webrtc.org/2686103002
Cr-Commit-Position: refs/heads/master@{#16554}

webrtc/test/fake_encoder.cc

@@ -15,7 +15,6 @@
 #include <algorithm>
 #include <memory>
 
-#include "webrtc/base/atomicops.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/video_coding/include/video_codec_interface.h"
@@ -36,8 +35,6 @@ FakeEncoder::FakeEncoder(Clock* clock)
   }
 }
 
-FakeEncoder::~FakeEncoder() {}
-
 void FakeEncoder::SetMaxBitrate(int max_kbps) {
   RTC_DCHECK_GE(max_kbps, -1);  // max_kbps == -1 disables it.
   rtc::CritScope cs(&crit_sect_);
@@ -254,33 +251,47 @@ EncodedImageCallback::Result FakeH264Encoder::OnEncodedImage(
 }
 
 DelayedEncoder::DelayedEncoder(Clock* clock, int delay_ms)
-    : test::FakeEncoder(clock),
+    : test::FakeEncoder(clock), delay_ms_(delay_ms) {
-      delay_ms_(delay_ms) {}
+  // The encoder could be created on a different thread than
+  // it is being used on.
+  sequence_checker_.Detach();
+}
 
 void DelayedEncoder::SetDelay(int delay_ms) {
-  rtc::CritScope cs(&local_crit_sect_);
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
   delay_ms_ = delay_ms;
 }
 
 int32_t DelayedEncoder::Encode(const VideoFrame& input_image,
                                const CodecSpecificInfo* codec_specific_info,
                                const std::vector<FrameType>* frame_types) {
-  int delay_ms = 0;
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
-  {
+
-    rtc::CritScope cs(&local_crit_sect_);
+  SleepMs(delay_ms_);
-    delay_ms = delay_ms_;
+
-  }
-  SleepMs(delay_ms);
   return FakeEncoder::Encode(input_image, codec_specific_info, frame_types);
 }
 
 MultithreadedFakeH264Encoder::MultithreadedFakeH264Encoder(Clock* clock)
     : test::FakeH264Encoder(clock),
       current_queue_(0),
-      queue1_("Queue 1"),
+      queue1_(nullptr),
-      queue2_("Queue 2") {}
+      queue2_(nullptr) {
+  // The encoder could be created on a different thread than
+  // it is being used on.
+  sequence_checker_.Detach();
+}
 
-MultithreadedFakeH264Encoder::~MultithreadedFakeH264Encoder() = default;
+int32_t MultithreadedFakeH264Encoder::InitEncode(const VideoCodec* config,
+                                                 int32_t number_of_cores,
+                                                 size_t max_payload_size) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
+
+  queue1_.reset(new rtc::TaskQueue("Queue 1"));
+  queue2_.reset(new rtc::TaskQueue("Queue 2"));
+
+  return FakeH264Encoder::InitEncode(config, number_of_cores, max_payload_size);
+}
 
 class MultithreadedFakeH264Encoder::EncodeTask : public rtc::QueuedTask {
  public:
@@ -313,13 +324,19 @@ int32_t MultithreadedFakeH264Encoder::Encode(
     const VideoFrame& input_image,
     const CodecSpecificInfo* codec_specific_info,
     const std::vector<FrameType>* frame_types) {
-  int current_queue = rtc::AtomicOps::Increment(&current_queue_);
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
-  rtc::TaskQueue& queue = (current_queue % 2 == 0) ? queue1_ : queue2_;
 
-  queue.PostTask(std::unique_ptr<rtc::QueuedTask>(
+  std::unique_ptr<rtc::TaskQueue>& queue =
+      (current_queue_++ % 2 == 0) ? queue1_ : queue2_;
+
+  if (!queue) {
+    return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  }
+
+  queue->PostTask(std::unique_ptr<rtc::QueuedTask>(
       new EncodeTask(this, input_image, codec_specific_info, frame_types)));
 
-  return 0;
+  return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t MultithreadedFakeH264Encoder::EncodeCallback(
@@ -329,5 +346,14 @@ int32_t MultithreadedFakeH264Encoder::EncodeCallback(
   return FakeH264Encoder::Encode(input_image, codec_specific_info, frame_types);
 }
 
+int32_t MultithreadedFakeH264Encoder::Release() {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
+
+  queue1_.reset();
+  queue2_.reset();
+
+  return FakeH264Encoder::Release();
+}
+
 }  // namespace test
 }  // namespace webrtc

webrtc/test/fake_encoder.h

@@ -12,8 +12,10 @@
 #define WEBRTC_TEST_FAKE_ENCODER_H_
 
 #include <vector>
+#include <memory>
 
 #include "webrtc/base/criticalsection.h"
+#include "webrtc/base/sequenced_task_checker.h"
 #include "webrtc/base/task_queue.h"
 #include "webrtc/common_types.h"
 #include "webrtc/system_wrappers/include/clock.h"
@@ -25,7 +27,7 @@ namespace test {
 class FakeEncoder : public VideoEncoder {
  public:
   explicit FakeEncoder(Clock* clock);
-  virtual ~FakeEncoder();
+  virtual ~FakeEncoder() = default;
 
   // Sets max bitrate. Not thread-safe, call before registering the encoder.
   void SetMaxBitrate(int max_kbps);
@@ -47,20 +49,21 @@ class FakeEncoder : public VideoEncoder {
   static const char* kImplementationName;
 
  protected:
-  rtc::CriticalSection crit_sect_;
   Clock* const clock_;
   VideoCodec config_ GUARDED_BY(crit_sect_);
   EncodedImageCallback* callback_ GUARDED_BY(crit_sect_);
   BitrateAllocation target_bitrate_ GUARDED_BY(crit_sect_);
   int max_target_bitrate_kbps_ GUARDED_BY(crit_sect_);
   int64_t last_encode_time_ms_ GUARDED_BY(crit_sect_);
+  rtc::CriticalSection crit_sect_;
+
   uint8_t encoded_buffer_[100000];
 };
 
 class FakeH264Encoder : public FakeEncoder, public EncodedImageCallback {
  public:
   explicit FakeH264Encoder(Clock* clock);
-  virtual ~FakeH264Encoder() {}
+  virtual ~FakeH264Encoder() = default;
 
   int32_t RegisterEncodeCompleteCallback(
       EncodedImageCallback* callback) override;
@@ -70,15 +73,15 @@ class FakeH264Encoder : public FakeEncoder, public EncodedImageCallback {
                         const RTPFragmentationHeader* fragments) override;
 
  private:
-  rtc::CriticalSection local_crit_sect_;
   EncodedImageCallback* callback_ GUARDED_BY(local_crit_sect_);
   int idr_counter_ GUARDED_BY(local_crit_sect_);
+  rtc::CriticalSection local_crit_sect_;
 };
 
 class DelayedEncoder : public test::FakeEncoder {
  public:
   DelayedEncoder(Clock* clock, int delay_ms);
-  virtual ~DelayedEncoder() {}
+  virtual ~DelayedEncoder() = default;
 
   void SetDelay(int delay_ms);
   int32_t Encode(const VideoFrame& input_image,
@@ -86,17 +89,22 @@ class DelayedEncoder : public test::FakeEncoder {
                  const std::vector<FrameType>* frame_types) override;
 
  private:
-  rtc::CriticalSection local_crit_sect_;
+  int delay_ms_ ACCESS_ON(sequence_checker_);
-  int delay_ms_ GUARDED_BY(&local_crit_sect_);
+  rtc::SequencedTaskChecker sequence_checker_;
 };
 
 // This class implements a multi-threaded fake encoder by posting
 // FakeH264Encoder::Encode(.) tasks to |queue1_| and |queue2_|, in an
-// alternating fashion.
+// alternating fashion. The class itself does not need to be thread safe,
+// as it is called from the task queue in ViEEncoder.
 class MultithreadedFakeH264Encoder : public test::FakeH264Encoder {
  public:
   explicit MultithreadedFakeH264Encoder(Clock* clock);
-  virtual ~MultithreadedFakeH264Encoder() override;
+  virtual ~MultithreadedFakeH264Encoder() = default;
+
+  int32_t InitEncode(const VideoCodec* config,
+                     int32_t number_of_cores,
+                     size_t max_payload_size) override;
 
   int32_t Encode(const VideoFrame& input_image,
                  const CodecSpecificInfo* codec_specific_info,
@@ -106,12 +114,15 @@ class MultithreadedFakeH264Encoder : public test::FakeH264Encoder {
                          const CodecSpecificInfo* codec_specific_info,
                          const std::vector<FrameType>* frame_types);
 
+  int32_t Release() override;
+
  protected:
   class EncodeTask;
 
-  int current_queue_;
+  int current_queue_ ACCESS_ON(sequence_checker_);
-  rtc::TaskQueue queue1_;
+  std::unique_ptr<rtc::TaskQueue> queue1_ ACCESS_ON(sequence_checker_);
-  rtc::TaskQueue queue2_;
+  std::unique_ptr<rtc::TaskQueue> queue2_ ACCESS_ON(sequence_checker_);
+  rtc::SequencedTaskChecker sequence_checker_;
 };
 
 }  // namespace test