Remove WebRTC-FrameBuffer3 field trial
The new frame buffer is already the default. Sync decoding can now be
inferred by the presence of a metronome rather than using the field
trial.
Tests have been updated to use the DecodeSynchronizer rather than the
field trial.
Bug: webrtc:14003
Change-Id: I33b457feaf4eac1500f3bf828680e445ae4d93cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274163
Auto-Submit: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38011}
diff --git a/video/BUILD.gn b/video/BUILD.gn
index 8033802..7c14b8f 100644
--- a/video/BUILD.gn
+++ b/video/BUILD.gn
@@ -55,6 +55,7 @@
deps = [
":frame_cadence_adapter",
":frame_dumping_decoder",
+ ":task_queue_frame_decode_scheduler",
":unique_timestamp_counter",
":video_stream_buffer_controller",
":video_stream_encoder_impl",
@@ -239,6 +240,7 @@
]
deps = [
":decode_synchronizer",
+ ":frame_decode_scheduler",
":frame_decode_timing",
":task_queue_frame_decode_scheduler",
":video_receive_stream_timeout_tracker",
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 0c2ab0f..647f4e5 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -54,6 +54,7 @@
#include "video/call_stats2.h"
#include "video/frame_dumping_decoder.h"
#include "video/receive_statistics_proxy2.h"
+#include "video/task_queue_frame_decode_scheduler.h"
namespace webrtc {
@@ -227,7 +228,6 @@
TimeDelta::Millis(config_.rtp.nack.rtp_history_ms),
false)),
maximum_pre_stream_decoders_("max", kDefaultMaximumPreStreamDecoders),
- decode_sync_(decode_sync),
decode_queue_(task_queue_factory_->CreateTaskQueue(
"DecodingQueue",
TaskQueueFactory::Priority::HIGH)) {
@@ -251,9 +251,13 @@
timing_->set_render_delay(TimeDelta::Millis(config_.render_delay_ms));
- buffer_ = VideoStreamBufferController::CreateFromFieldTrial(
+ std::unique_ptr<FrameDecodeScheduler> scheduler =
+ decode_sync ? decode_sync->CreateSynchronizedFrameScheduler()
+ : std::make_unique<TaskQueueFrameDecodeScheduler>(
+ clock, call_->worker_thread());
+ buffer_ = std::make_unique<VideoStreamBufferController>(
clock_, call_->worker_thread(), timing_.get(), &stats_proxy_, this,
- max_wait_for_keyframe_, max_wait_for_frame_, decode_sync_,
+ max_wait_for_keyframe_, max_wait_for_frame_, std::move(scheduler),
call_->trials());
if (rtx_ssrc()) {
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 9d4cac7..6b3d1ce 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -339,8 +339,6 @@
// any video frame has been received.
FieldTrialParameter<int> maximum_pre_stream_decoders_;
- DecodeSynchronizer* decode_sync_;
-
// Defined last so they are destroyed before all other members.
rtc::TaskQueue decode_queue_;
diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc
index 0a587fc..79c680f 100644
--- a/video/video_receive_stream2_unittest.cc
+++ b/video/video_receive_stream2_unittest.cc
@@ -203,12 +203,6 @@
&fake_metronome_,
time_controller_.GetMainThread()),
h264_decoder_factory_(&mock_decoder_) {
- if (UseMetronome()) {
- fake_call_.SetFieldTrial("WebRTC-FrameBuffer3/arm:SyncDecoding/");
- } else {
- fake_call_.SetFieldTrial("WebRTC-FrameBuffer3/arm:FrameBuffer3/");
- }
-
// By default, mock decoder factory is backed by VideoDecoderProxyFactory.
ON_CALL(mock_h264_decoder_factory_, CreateVideoDecoder)
.WillByDefault(
@@ -270,7 +264,7 @@
time_controller_.GetTaskQueueFactory(), &fake_call_,
kDefaultNumCpuCores, &packet_router_, config_.Copy(), &call_stats_,
clock_, absl::WrapUnique(timing_), &nack_periodic_processor_,
- GetParam() ? &decode_sync_ : nullptr, nullptr);
+ UseMetronome() ? &decode_sync_ : nullptr, nullptr);
video_receive_stream_->RegisterWithTransport(
&rtp_stream_receiver_controller_);
if (state)
diff --git a/video/video_stream_buffer_controller.cc b/video/video_stream_buffer_controller.cc
index 037754d..801b91c 100644
--- a/video/video_stream_buffer_controller.cc
+++ b/video/video_stream_buffer_controller.cc
@@ -28,6 +28,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/thread_annotations.h"
+#include "video/frame_decode_scheduler.h"
#include "video/frame_decode_timing.h"
#include "video/task_queue_frame_decode_scheduler.h"
#include "video/video_receive_stream_timeout_tracker.h"
@@ -70,68 +71,8 @@
return *ts;
}
-enum class FrameBufferArm {
- kFrameBuffer3,
- kSyncDecode,
-};
-
-constexpr const char* kFrameBufferFieldTrial = "WebRTC-FrameBuffer3";
-
-FrameBufferArm ParseFrameBufferFieldTrial(const FieldTrialsView& field_trials) {
- webrtc::FieldTrialEnum<FrameBufferArm> arm(
- "arm", FrameBufferArm::kFrameBuffer3,
- {
- {"FrameBuffer3", FrameBufferArm::kFrameBuffer3},
- {"SyncDecoding", FrameBufferArm::kSyncDecode},
- });
- ParseFieldTrial({&arm}, field_trials.Lookup(kFrameBufferFieldTrial));
- return arm.Get();
-}
-
} // namespace
-std::unique_ptr<VideoStreamBufferController>
-VideoStreamBufferController::CreateFromFieldTrial(
- Clock* clock,
- TaskQueueBase* worker_queue,
- VCMTiming* timing,
- VCMReceiveStatisticsCallback* stats_proxy,
- FrameSchedulingReceiver* receiver,
- TimeDelta max_wait_for_keyframe,
- TimeDelta max_wait_for_frame,
- DecodeSynchronizer* decode_sync,
- const FieldTrialsView& field_trials) {
- switch (ParseFrameBufferFieldTrial(field_trials)) {
- case FrameBufferArm::kSyncDecode: {
- std::unique_ptr<FrameDecodeScheduler> scheduler;
- if (decode_sync) {
- scheduler = decode_sync->CreateSynchronizedFrameScheduler();
- } else {
- RTC_LOG(LS_ERROR) << "In FrameBuffer with sync decode trial, but "
- "no DecodeSynchronizer was present!";
- // Crash in debug, but in production use the task queue scheduler.
- RTC_DCHECK_NOTREACHED();
- scheduler = std::make_unique<TaskQueueFrameDecodeScheduler>(
- clock, worker_queue);
- }
- return std::make_unique<VideoStreamBufferController>(
- clock, worker_queue, timing, stats_proxy, receiver,
- max_wait_for_keyframe, max_wait_for_frame, std::move(scheduler),
- field_trials);
- }
- case FrameBufferArm::kFrameBuffer3:
- ABSL_FALLTHROUGH_INTENDED;
- default: {
- auto scheduler =
- std::make_unique<TaskQueueFrameDecodeScheduler>(clock, worker_queue);
- return std::make_unique<VideoStreamBufferController>(
- clock, worker_queue, timing, stats_proxy, receiver,
- max_wait_for_keyframe, max_wait_for_frame, std::move(scheduler),
- field_trials);
- }
- }
-}
-
VideoStreamBufferController::VideoStreamBufferController(
Clock* clock,
TaskQueueBase* worker_queue,
diff --git a/video/video_stream_buffer_controller.h b/video/video_stream_buffer_controller.h
index 5251545..ed79b0f 100644
--- a/video/video_stream_buffer_controller.h
+++ b/video/video_stream_buffer_controller.h
@@ -38,17 +38,6 @@
class VideoStreamBufferController {
public:
- static std::unique_ptr<VideoStreamBufferController> CreateFromFieldTrial(
- Clock* clock,
- TaskQueueBase* worker_queue,
- VCMTiming* timing,
- VCMReceiveStatisticsCallback* stats_proxy,
- FrameSchedulingReceiver* receiver,
- TimeDelta max_wait_for_keyframe,
- TimeDelta max_wait_for_frame,
- DecodeSynchronizer* decode_sync,
- const FieldTrialsView& field_trials);
- virtual ~VideoStreamBufferController() = default;
VideoStreamBufferController(
Clock* clock,
TaskQueueBase* worker_queue,
@@ -59,6 +48,7 @@
TimeDelta max_wait_for_frame,
std::unique_ptr<FrameDecodeScheduler> frame_decode_scheduler,
const FieldTrialsView& field_trials);
+ virtual ~VideoStreamBufferController() = default;
void Stop();
void SetProtectionMode(VCMVideoProtection protection_mode);
diff --git a/video/video_stream_buffer_controller_unittest.cc b/video/video_stream_buffer_controller_unittest.cc
index 7f7d8db..9c627ca 100644
--- a/video/video_stream_buffer_controller_unittest.cc
+++ b/video/video_stream_buffer_controller_unittest.cc
@@ -15,6 +15,7 @@
#include <limits>
#include <memory>
#include <string>
+#include <tuple>
#include <utility>
#include <vector>
@@ -33,6 +34,7 @@
#include "test/scoped_key_value_config.h"
#include "test/time_controller/simulated_time_controller.h"
#include "video/decode_synchronizer.h"
+#include "video/task_queue_frame_decode_scheduler.h"
using ::testing::_;
using ::testing::AllOf;
@@ -107,11 +109,12 @@
constexpr auto kMaxWaitForKeyframe = TimeDelta::Millis(500);
constexpr auto kMaxWaitForFrame = TimeDelta::Millis(1500);
class VideoStreamBufferControllerFixture
- : public ::testing::WithParamInterface<std::string>,
+ : public ::testing::WithParamInterface<std::tuple<bool, std::string>>,
public FrameSchedulingReceiver {
public:
VideoStreamBufferControllerFixture()
- : field_trials_(GetParam()),
+ : sync_decoding_(std::get<0>(GetParam())),
+ field_trials_(std::get<1>(GetParam())),
time_controller_(kClockStart),
clock_(time_controller_.GetClock()),
fake_metronome_(time_controller_.GetTaskQueueFactory(),
@@ -120,7 +123,7 @@
&fake_metronome_,
time_controller_.GetMainThread()),
timing_(clock_, field_trials_),
- buffer_(VideoStreamBufferController::CreateFromFieldTrial(
+ buffer_(std::make_unique<VideoStreamBufferController>(
clock_,
time_controller_.GetMainThread(),
&timing_,
@@ -128,7 +131,10 @@
this,
kMaxWaitForKeyframe,
kMaxWaitForFrame,
- &decode_sync_,
+ sync_decoding_ ? decode_sync_.CreateSynchronizedFrameScheduler()
+ : std::make_unique<TaskQueueFrameDecodeScheduler>(
+ clock_,
+ time_controller_.GetMainThread()),
field_trials_)) {
// Avoid starting with negative render times.
timing_.set_min_playout_delay(TimeDelta::Millis(10));
@@ -198,6 +204,7 @@
int dropped_frames() const { return dropped_frames_; }
protected:
+ const bool sync_decoding_;
test::ScopedKeyValueConfig field_trials_;
GlobalSimulatedTimeController time_controller_;
Clock* const clock_;
@@ -732,11 +739,14 @@
EXPECT_THAT(WaitForFrameOrTimeout(kFps30Delay), Frame(test::WithId(2)));
}
-INSTANTIATE_TEST_SUITE_P(
- VideoStreamBufferController,
- VideoStreamBufferControllerTest,
- ::testing::Values("WebRTC-FrameBuffer3/arm:FrameBuffer3/",
- "WebRTC-FrameBuffer3/arm:SyncDecoding/"));
+INSTANTIATE_TEST_SUITE_P(VideoStreamBufferController,
+ VideoStreamBufferControllerTest,
+ ::testing::Combine(::testing::Bool(),
+ ::testing::Values("")),
+ [](const auto& info) {
+ return std::get<0>(info.param) ? "SyncDecoding"
+ : "UnsyncedDecoding";
+ });
class LowLatencyVideoStreamBufferControllerTest
: public ::testing::Test,
@@ -817,10 +827,11 @@
INSTANTIATE_TEST_SUITE_P(
VideoStreamBufferController,
LowLatencyVideoStreamBufferControllerTest,
- ::testing::Values(
- "WebRTC-FrameBuffer3/arm:FrameBuffer3/"
- "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/",
- "WebRTC-FrameBuffer3/arm:SyncDecoding/"
- "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/"));
+ ::testing::Combine(
+ ::testing::Bool(),
+ ::testing::Values(
+ "WebRTC-ZeroPlayoutDelay/min_pacing:16ms,max_decode_queue_size:5/",
+ "WebRTC-ZeroPlayoutDelay/"
+ "min_pacing:16ms,max_decode_queue_size:5/")));
} // namespace webrtc
