Fix and test CreateVideoStreamDecoder

create TaskQueue using provided factory instead of through constructor that uses GlobalTaskQueueFactory
Rename interface to avoid collision with class in video/video_stream_decoder.h
Add simple unittest to avoid future breakages.

Bug: webrtc:10284, webrtc:10521
Change-Id: I647b31cc99a2b6beb79b936f05f482788861f1cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131398
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27503}

api/BUILD.gn

@@ -803,6 +803,7 @@ if (rtc_include_tests) {
       "../test:test_support",
       "task_queue:task_queue_default_factory_unittests",
       "units:units_unittests",
+      "video:video_unittests",
     ]
   }
 

api/video/BUILD.gn

@@ -165,6 +165,7 @@ rtc_source_set("video_stream_decoder") {
   deps = [
     ":encoded_frame",
     ":video_frame",
+    "../task_queue",
     "../video_codecs:video_codecs_api",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
@@ -181,6 +182,8 @@ rtc_source_set("video_stream_decoder_create") {
     ":video_stream_decoder",
     "../../rtc_base:rtc_base_approved",
     "../../video:video_stream_decoder_impl",
+    "../task_queue",
+    "../video_codecs:video_codecs_api",
     "//third_party/abseil-cpp/absl/memory",
   ]
 }
@@ -246,3 +249,18 @@ rtc_static_library("builtin_video_bitrate_allocator_factory") {
     "//third_party/abseil-cpp/absl/memory",
   ]
 }
+
+if (rtc_include_tests) {
+  rtc_source_set("video_unittests") {
+    testonly = true
+    sources = [
+      "video_stream_decoder_create_unittest.cc",
+    ]
+    deps = [
+      ":video_stream_decoder_create",
+      "../../test:test_support",
+      "../task_queue:default_task_queue_factory",
+      "../video_codecs:builtin_video_decoder_factory",
+    ]
+  }
+}

api/video/video_stream_decoder.h

@@ -22,7 +22,7 @@
 
 namespace webrtc {
 // NOTE: This class is still under development and may change without notice.
-class VideoStreamDecoder {
+class VideoStreamDecoderInterface {
  public:
   class Callbacks {
    public:
@@ -41,7 +41,7 @@ class VideoStreamDecoder {
                                 absl::optional<int> qp) = 0;
   };
 
-  virtual ~VideoStreamDecoder() = default;
+  virtual ~VideoStreamDecoderInterface() = default;
 
   virtual void OnFrame(std::unique_ptr<video_coding::EncodedFrame> frame) = 0;
 };

api/video/video_stream_decoder_create.cc

@@ -14,11 +14,15 @@
 #include "video/video_stream_decoder_impl.h"
 
 namespace webrtc {
-std::unique_ptr<VideoStreamDecoder> CreateVideoStreamDecoder(
-    VideoStreamDecoder::Callbacks* callbacks,
+
+std::unique_ptr<VideoStreamDecoderInterface> CreateVideoStreamDecoder(
+    VideoStreamDecoderInterface::Callbacks* callbacks,
     VideoDecoderFactory* decoder_factory,
+    TaskQueueFactory* task_queue_factory,
     std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings) {
   return absl::make_unique<VideoStreamDecoderImpl>(callbacks, decoder_factory,
+                                                   task_queue_factory,
                                                    std::move(decoder_settings));
 }
+
 }  // namespace webrtc

api/video/video_stream_decoder_create.h

@@ -15,16 +15,19 @@
 #include <memory>
 #include <utility>
 
+#include "api/task_queue/task_queue_factory.h"
 #include "api/video/video_stream_decoder.h"
+#include "api/video_codecs/sdp_video_format.h"
 
 namespace webrtc {
 // The |decoder_settings| parameter is a map between:
 // <payload type> -->  <<video format>, <number of cores>>.
 // The video format is used when instantiating a decoder, and
 // the number of cores is used when initializing the decoder.
-std::unique_ptr<VideoStreamDecoder> CreateVideoStreamDecoder(
-    VideoStreamDecoder::Callbacks* callbacks,
+std::unique_ptr<VideoStreamDecoderInterface> CreateVideoStreamDecoder(
+    VideoStreamDecoderInterface::Callbacks* callbacks,
     VideoDecoderFactory* decoder_factory,
+    TaskQueueFactory* task_queue_factory,
     std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings);
 
 }  // namespace webrtc

api/video/video_stream_decoder_create_unittest.cc

@@ -0,0 +1,47 @@
+/*
+ *  Copyright (c) 2019 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.
+ */
+
+#include "api/video/video_stream_decoder_create.h"
+
+#include "api/task_queue/default_task_queue_factory.h"
+#include "api/video_codecs/builtin_video_decoder_factory.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+namespace {
+
+class NullCallbacks : public VideoStreamDecoderInterface::Callbacks {
+ public:
+  ~NullCallbacks() override = default;
+  void OnNonDecodableState() override {}
+  void OnContinuousUntil(const video_coding::VideoLayerFrameId& key) override {}
+  void OnDecodedFrame(VideoFrame decodedImage,
+                      absl::optional<int> decode_time_ms,
+                      absl::optional<int> qp) override {}
+};
+
+TEST(VideoStreamDecoderCreate, CreateVideoStreamDecoder) {
+  std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings = {
+      {/*payload_type=*/111, {SdpVideoFormat("VP8"), /*number_of_cores=*/2}}};
+  NullCallbacks callbacks;
+  std::unique_ptr<VideoDecoderFactory> decoder_factory =
+      CreateBuiltinVideoDecoderFactory();
+
+  std::unique_ptr<TaskQueueFactory> task_queue_factory =
+      CreateDefaultTaskQueueFactory();
+
+  std::unique_ptr<VideoStreamDecoderInterface> decoder =
+      CreateVideoStreamDecoder(&callbacks, decoder_factory.get(),
+                               task_queue_factory.get(), decoder_settings);
+  EXPECT_TRUE(decoder);
+}
+
+}  // namespace
+}  // namespace webrtc

video/video_stream_decoder_impl.cc

@@ -19,13 +19,16 @@
 namespace webrtc {
 
 VideoStreamDecoderImpl::VideoStreamDecoderImpl(
-    VideoStreamDecoder::Callbacks* callbacks,
+    VideoStreamDecoderInterface::Callbacks* callbacks,
     VideoDecoderFactory* decoder_factory,
+    TaskQueueFactory* task_queue_factory,
     std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings)
     : callbacks_(callbacks),
       decoder_factory_(decoder_factory),
       decoder_settings_(std::move(decoder_settings)),
-      bookkeeping_queue_("video_stream_decoder_bookkeeping_queue"),
+      bookkeeping_queue_(task_queue_factory->CreateTaskQueue(
+          "video_stream_decoder_bookkeeping_queue",
+          TaskQueueFactory::Priority::NORMAL)),
       decode_thread_(&DecodeLoop,
                      this,
                      "video_stream_decoder_decode_thread",

video/video_stream_decoder_impl.h

@@ -27,12 +27,13 @@
 
 namespace webrtc {
 
-class VideoStreamDecoderImpl : public VideoStreamDecoder,
+class VideoStreamDecoderImpl : public VideoStreamDecoderInterface,
                                private DecodedImageCallback {
  public:
   VideoStreamDecoderImpl(
-      VideoStreamDecoder::Callbacks* callbacks,
+      VideoStreamDecoderInterface::Callbacks* callbacks,
       VideoDecoderFactory* decoder_factory,
+      TaskQueueFactory* task_queue_factory,
       std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings);
 
   ~VideoStreamDecoderImpl() override;
@@ -67,7 +68,7 @@ class VideoStreamDecoderImpl : public VideoStreamDecoder,
                absl::optional<int32_t> decode_time_ms,
                absl::optional<uint8_t> qp) override;
 
-  VideoStreamDecoder::Callbacks* const callbacks_
+  VideoStreamDecoderInterface::Callbacks* const callbacks_
       RTC_PT_GUARDED_BY(bookkeeping_queue_);
   VideoDecoderFactory* const decoder_factory_;
   std::map<int, std::pair<SdpVideoFormat, int>> decoder_settings_;