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}diff --git a/api/BUILD.gn b/api/BUILD.gn
index ba7570d..28ddff1 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -803,6 +803,7 @@
"../test:test_support",
"task_queue:task_queue_default_factory_unittests",
"units:units_unittests",
+ "video:video_unittests",
]
}
diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn
index 533e8d3..59b6a4b 100644
--- a/api/video/BUILD.gn
+++ b/api/video/BUILD.gn
@@ -165,6 +165,7 @@
deps = [
":encoded_frame",
":video_frame",
+ "../task_queue",
"../video_codecs:video_codecs_api",
"//third_party/abseil-cpp/absl/types:optional",
]
@@ -181,6 +182,8 @@
":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 @@
"//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",
+ ]
+ }
+}
diff --git a/api/video/video_stream_decoder.h b/api/video/video_stream_decoder.h
index dff60d8..80c70d3 100644
--- a/api/video/video_stream_decoder.h
+++ b/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 @@
absl::optional<int> qp) = 0;
};
- virtual ~VideoStreamDecoder() = default;
+ virtual ~VideoStreamDecoderInterface() = default;
virtual void OnFrame(std::unique_ptr<video_coding::EncodedFrame> frame) = 0;
};
diff --git a/api/video/video_stream_decoder_create.cc b/api/video/video_stream_decoder_create.cc
index d579255..d17d739 100644
--- a/api/video/video_stream_decoder_create.cc
+++ b/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
diff --git a/api/video/video_stream_decoder_create.h b/api/video/video_stream_decoder_create.h
index 0468290..4958dc1 100644
--- a/api/video/video_stream_decoder_create.h
+++ b/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
diff --git a/api/video/video_stream_decoder_create_unittest.cc b/api/video/video_stream_decoder_create_unittest.cc
new file mode 100644
index 0000000..7b142a9
--- /dev/null
+++ b/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
diff --git a/video/video_stream_decoder_impl.cc b/video/video_stream_decoder_impl.cc
index 56efca6..c3c1d0d 100644
--- a/video/video_stream_decoder_impl.cc
+++ b/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",
diff --git a/video/video_stream_decoder_impl.h b/video/video_stream_decoder_impl.h
index cd1f953..5578e70 100644
--- a/video/video_stream_decoder_impl.h
+++ b/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 @@
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_;
