Skip to content
This repository was archived by the owner on Oct 25, 2024. It is now read-only.

Commit dcef641

Browse files
perkjCommit Bot
perkj
authored and
Commit Bot
committed
Stop using VideoBitrateAllocationObserver in VideoStreamEncoder.
VideoBitrateAllocation is instead reported through the EncoderSink. Enable VideoBitrateAllocation reporting from WebRtcVideoChannel::AddSendStream in preparation for using the extension RtpVideoLayersAllocationExtension instead of RTCP XR. Bug: webrtc:12000 Change-Id: I5ea8e4f237a1c4e84a89cbfd97ac4353d4c2984f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186940 Commit-Queue: Per Kjellander <[email protected]> Reviewed-by: Erik Språng <[email protected]> Cr-Commit-Position: refs/heads/master@{#32347}
1 parent 5c71f77 commit dcef641

11 files changed

+142
-103
lines changed

‎api/video/video_stream_encoder_interface.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ class VideoStreamEncoderInterface : public rtc::VideoSinkInterface<VideoFrame> {
4949
bool is_svc,
5050
VideoEncoderConfig::ContentType content_type,
5151
int min_transmit_bitrate_bps) = 0;
52+
53+
virtual void OnBitrateAllocationUpdated(
54+
const VideoBitrateAllocation& allocation) = 0;
5255
};
5356

5457
// If the resource is overusing, the VideoStreamEncoder will try to reduce
@@ -110,11 +113,6 @@ class VideoStreamEncoderInterface : public rtc::VideoSinkInterface<VideoFrame> {
110113
int64_t round_trip_time_ms,
111114
double cwnd_reduce_ratio) = 0;
112115

113-
// Register observer for the bitrate allocation between the temporal
114-
// and spatial layers.
115-
virtual void SetBitrateAllocationObserver(
116-
VideoBitrateAllocationObserver* bitrate_observer) = 0;
117-
118116
// Set a FecControllerOverride, through which the encoder may override
119117
// decisions made by FecController.
120118
virtual void SetFecControllerOverride(

‎api/video/video_stream_encoder_settings.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ class EncoderSwitchRequestCallback {
3939
};
4040

4141
struct VideoStreamEncoderSettings {
42+
enum class BitrateAllocationCallbackType {
43+
kVideoBitrateAllocation,
44+
kVideoBitrateAllocationWhenScreenSharing,
45+
kVideoLayersAllocation
46+
};
47+
4248
explicit VideoStreamEncoderSettings(
4349
const VideoEncoder::Capabilities& capabilities)
4450
: capabilities(capabilities) {}
@@ -59,6 +65,11 @@ struct VideoStreamEncoderSettings {
5965
// Negotiated capabilities which the VideoEncoder may expect the other
6066
// side to use.
6167
VideoEncoder::Capabilities capabilities;
68+
69+
// TODO(bugs.webrtc.org/12000): Reporting of VideoBitrateAllocation is beeing
70+
// deprecated. Instead VideoLayersAllocation should be reported.
71+
BitrateAllocationCallbackType allocation_cb_type =
72+
BitrateAllocationCallbackType::kVideoBitrateAllocationWhenScreenSharing;
6273
};
6374

6475
} // namespace webrtc

‎media/engine/webrtc_video_engine.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,15 @@ bool WebRtcVideoChannel::AddSendStream(const StreamParams& sp) {
12971297
video_config_.periodic_alr_bandwidth_probing;
12981298
config.encoder_settings.experiment_cpu_load_estimator =
12991299
video_config_.experiment_cpu_load_estimator;
1300+
// TODO(bugs.webrtc.org/12000): Enable allocation callback type
1301+
// VideoLayersAllocation if RtpVideoLayersAllocationExtension has been
1302+
// negotiated in `send_rtp_extensions_`.
1303+
config.encoder_settings.allocation_cb_type =
1304+
IsEnabled(call_->trials(), "WebRTC-Target-Bitrate-Rtcp")
1305+
? webrtc::VideoStreamEncoderSettings::BitrateAllocationCallbackType::
1306+
kVideoBitrateAllocation
1307+
: webrtc::VideoStreamEncoderSettings::BitrateAllocationCallbackType::
1308+
kVideoBitrateAllocationWhenScreenSharing;
13001309
config.encoder_settings.encoder_factory = encoder_factory_;
13011310
config.encoder_settings.bitrate_allocator_factory =
13021311
bitrate_allocator_factory_;

‎video/end_to_end_tests/extended_reports_tests.cc

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ class RtcpXrObserver : public test::EndToEndTest {
6262
RtcpXrObserver(bool enable_rrtr,
6363
bool expect_target_bitrate,
6464
bool enable_zero_target_bitrate,
65+
bool enable_target_bitrate,
6566
VideoEncoderConfig::ContentType content_type)
6667
: EndToEndTest(test::CallTest::kDefaultTimeoutMs),
6768
enable_rrtr_(enable_rrtr),
6869
expect_target_bitrate_(expect_target_bitrate),
6970
enable_zero_target_bitrate_(enable_zero_target_bitrate),
71+
enable_target_bitrate_(enable_target_bitrate),
7072
content_type_(content_type),
7173
sent_rtcp_sr_(0),
7274
sent_rtcp_rr_(0),
@@ -175,6 +177,12 @@ class RtcpXrObserver : public test::EndToEndTest {
175177
VideoSendStream::Config* send_config,
176178
std::vector<VideoReceiveStream::Config>* receive_configs,
177179
VideoEncoderConfig* encoder_config) override {
180+
if (enable_target_bitrate_) {
181+
send_config->encoder_settings.allocation_cb_type =
182+
VideoStreamEncoderSettings::BitrateAllocationCallbackType::
183+
kVideoBitrateAllocation;
184+
}
185+
178186
if (enable_zero_target_bitrate_) {
179187
// Configure VP8 to be able to use simulcast.
180188
send_config->rtp.payload_name = "VP8";
@@ -202,6 +210,7 @@ class RtcpXrObserver : public test::EndToEndTest {
202210
const bool enable_rrtr_;
203211
const bool expect_target_bitrate_;
204212
const bool enable_zero_target_bitrate_;
213+
const bool enable_target_bitrate_;
205214
const VideoEncoderConfig::ContentType content_type_;
206215
int sent_rtcp_sr_;
207216
int sent_rtcp_rr_ RTC_GUARDED_BY(&mutex_);
@@ -217,6 +226,7 @@ TEST_F(ExtendedReportsEndToEndTest,
217226
TestExtendedReportsWithRrtrWithoutTargetBitrate) {
218227
RtcpXrObserver test(/*enable_rrtr=*/true, /*expect_target_bitrate=*/false,
219228
/*enable_zero_target_bitrate=*/false,
229+
/*enable_target_bitrate=*/false,
220230
VideoEncoderConfig::ContentType::kRealtimeVideo);
221231
RunBaseTest(&test);
222232
}
@@ -225,6 +235,7 @@ TEST_F(ExtendedReportsEndToEndTest,
225235
TestExtendedReportsWithoutRrtrWithoutTargetBitrate) {
226236
RtcpXrObserver test(/*enable_rrtr=*/false, /*expect_target_bitrate=*/false,
227237
/*enable_zero_target_bitrate=*/false,
238+
/*enable_target_bitrate=*/false,
228239
VideoEncoderConfig::ContentType::kRealtimeVideo);
229240
RunBaseTest(&test);
230241
}
@@ -233,6 +244,7 @@ TEST_F(ExtendedReportsEndToEndTest,
233244
TestExtendedReportsWithRrtrWithTargetBitrate) {
234245
RtcpXrObserver test(/*enable_rrtr=*/true, /*expect_target_bitrate=*/true,
235246
/*enable_zero_target_bitrate=*/false,
247+
/*enable_target_bitrate=*/false,
236248
VideoEncoderConfig::ContentType::kScreen);
237249
RunBaseTest(&test);
238250
}
@@ -241,15 +253,16 @@ TEST_F(ExtendedReportsEndToEndTest,
241253
TestExtendedReportsWithoutRrtrWithTargetBitrate) {
242254
RtcpXrObserver test(/*enable_rrtr=*/false, /*expect_target_bitrate=*/true,
243255
/*enable_zero_target_bitrate=*/false,
256+
/*enable_target_bitrate=*/false,
244257
VideoEncoderConfig::ContentType::kScreen);
245258
RunBaseTest(&test);
246259
}
247260

248261
TEST_F(ExtendedReportsEndToEndTest,
249-
TestExtendedReportsWithoutRrtrWithTargetBitrateFromFieldTrial) {
250-
test::ScopedFieldTrials field_trials("WebRTC-Target-Bitrate-Rtcp/Enabled/");
262+
TestExtendedReportsWithoutRrtrWithTargetBitrateExplicitlySet) {
251263
RtcpXrObserver test(/*enable_rrtr=*/false, /*expect_target_bitrate=*/true,
252264
/*enable_zero_target_bitrate=*/false,
265+
/*enable_target_bitrate=*/true,
253266
VideoEncoderConfig::ContentType::kRealtimeVideo);
254267
RunBaseTest(&test);
255268
}
@@ -258,6 +271,7 @@ TEST_F(ExtendedReportsEndToEndTest,
258271
TestExtendedReportsCanSignalZeroTargetBitrate) {
259272
RtcpXrObserver test(/*enable_rrtr=*/false, /*expect_target_bitrate=*/true,
260273
/*enable_zero_target_bitrate=*/true,
274+
/*enable_target_bitrate=*/false,
261275
VideoEncoderConfig::ContentType::kScreen);
262276
RunBaseTest(&test);
263277
}

‎video/test/mock_video_stream_encoder.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,6 @@ class MockVideoStreamEncoder : public VideoStreamEncoderInterface {
4444
(DataRate, DataRate, DataRate, uint8_t, int64_t, double),
4545
(override));
4646
MOCK_METHOD(void, OnFrame, (const VideoFrame&), (override));
47-
MOCK_METHOD(void,
48-
SetBitrateAllocationObserver,
49-
(VideoBitrateAllocationObserver*),
50-
(override));
5147
MOCK_METHOD(void,
5248
SetFecControllerOverride,
5349
(FecControllerOverride*),

‎video/video_send_stream.cc

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ namespace webrtc {
2929

3030
namespace {
3131

32-
constexpr char kTargetBitrateRtcpFieldTrial[] = "WebRTC-Target-Bitrate-Rtcp";
33-
3432
size_t CalculateMaxHeaderSize(const RtpConfig& config) {
3533
size_t header_size = kRtpHeaderSize;
3634
size_t extensions_size = 0;
@@ -113,13 +111,6 @@ VideoSendStream::VideoSendStream(
113111
// it was created on.
114112
thread_sync_event_.Wait(rtc::Event::kForever);
115113
send_stream_->RegisterProcessThread(module_process_thread);
116-
// TODO(sprang): Enable this also for regular video calls by default, if it
117-
// works well.
118-
if (encoder_config.content_type == VideoEncoderConfig::ContentType::kScreen ||
119-
field_trial::IsEnabled(kTargetBitrateRtcpFieldTrial)) {
120-
video_stream_encoder_->SetBitrateAllocationObserver(send_stream_.get());
121-
}
122-
123114
ReconfigureVideoEncoder(std::move(encoder_config));
124115
}
125116

‎video/video_send_stream_impl.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ struct PacingConfig {
6767
// An encoder may deliver frames through the EncodedImageCallback on an
6868
// arbitrary thread.
6969
class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
70-
public VideoStreamEncoderInterface::EncoderSink,
71-
public VideoBitrateAllocationObserver {
70+
public VideoStreamEncoderInterface::EncoderSink {
7271
public:
7372
VideoSendStreamImpl(
7473
Clock* clock,
@@ -113,12 +112,16 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
113112
// Implements BitrateAllocatorObserver.
114113
uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override;
115114

115+
// Implements VideoStreamEncoderInterface::EncoderSink
116116
void OnEncoderConfigurationChanged(
117117
std::vector<VideoStream> streams,
118118
bool is_svc,
119119
VideoEncoderConfig::ContentType content_type,
120120
int min_transmit_bitrate_bps) override;
121121

122+
void OnBitrateAllocationUpdated(
123+
const VideoBitrateAllocation& allocation) override;
124+
122125
// Implements EncodedImageCallback. The implementation routes encoded frames
123126
// to the |payload_router_| and |config.pre_encode_callback| if set.
124127
// Called on an arbitrary encoder callback thread.
@@ -129,10 +132,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
129132
// Implements EncodedImageCallback.
130133
void OnDroppedFrame(EncodedImageCallback::DropReason reason) override;
131134

132-
// Implements VideoBitrateAllocationObserver.
133-
void OnBitrateAllocationUpdated(
134-
const VideoBitrateAllocation& allocation) override;
135-
136135
// Starts monitoring and sends a keyframe.
137136
void StartupVideoSendStream();
138137
// Removes the bitrate observer, stops monitoring and notifies the video

‎video/video_send_stream_impl_unittest.cc

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,10 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) {
435435
auto vss_impl = CreateVideoSendStreamImpl(
436436
kDefaultInitialBitrateBps, kDefaultBitratePriority,
437437
VideoEncoderConfig::ContentType::kScreen);
438+
VideoStreamEncoderInterface::EncoderSink* const sink =
439+
static_cast<VideoStreamEncoderInterface::EncoderSink*>(
440+
vss_impl.get());
438441
vss_impl->Start();
439-
VideoBitrateAllocationObserver* const observer =
440-
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
441442

442443
// Populate a test instance of video bitrate allocation.
443444
VideoBitrateAllocation alloc;
@@ -449,7 +450,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) {
449450
// Encoder starts out paused, don't forward allocation.
450451
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
451452
.Times(0);
452-
observer->OnBitrateAllocationUpdated(alloc);
453+
sink->OnBitrateAllocationUpdated(alloc);
453454

454455
// Unpause encoder, allocation should be passed through.
455456
const uint32_t kBitrateBps = 100000;
@@ -460,7 +461,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) {
460461
->OnBitrateUpdated(CreateAllocation(kBitrateBps));
461462
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
462463
.Times(1);
463-
observer->OnBitrateAllocationUpdated(alloc);
464+
sink->OnBitrateAllocationUpdated(alloc);
464465

465466
// Pause encoder again, and block allocations.
466467
EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps())
@@ -470,7 +471,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) {
470471
->OnBitrateUpdated(CreateAllocation(0));
471472
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
472473
.Times(0);
473-
observer->OnBitrateAllocationUpdated(alloc);
474+
sink->OnBitrateAllocationUpdated(alloc);
474475

475476
vss_impl->Stop();
476477
},
@@ -491,8 +492,9 @@ TEST_F(VideoSendStreamImplTest, ThrottlesVideoBitrateAllocationWhenTooSimilar) {
491492
.WillOnce(Return(kBitrateBps));
492493
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
493494
->OnBitrateUpdated(CreateAllocation(kBitrateBps));
494-
VideoBitrateAllocationObserver* const observer =
495-
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
495+
VideoStreamEncoderInterface::EncoderSink* const sink =
496+
static_cast<VideoStreamEncoderInterface::EncoderSink*>(
497+
vss_impl.get());
496498

497499
// Populate a test instance of video bitrate allocation.
498500
VideoBitrateAllocation alloc;
@@ -504,7 +506,7 @@ TEST_F(VideoSendStreamImplTest, ThrottlesVideoBitrateAllocationWhenTooSimilar) {
504506
// Initial value.
505507
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
506508
.Times(1);
507-
observer->OnBitrateAllocationUpdated(alloc);
509+
sink->OnBitrateAllocationUpdated(alloc);
508510

509511
VideoBitrateAllocation updated_alloc = alloc;
510512
// Needs 10% increase in bitrate to trigger immediate forward.
@@ -514,22 +516,22 @@ TEST_F(VideoSendStreamImplTest, ThrottlesVideoBitrateAllocationWhenTooSimilar) {
514516
// Too small increase, don't forward.
515517
updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps - 1);
516518
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(_)).Times(0);
517-
observer->OnBitrateAllocationUpdated(updated_alloc);
519+
sink->OnBitrateAllocationUpdated(updated_alloc);
518520

519521
// Large enough increase, do forward.
520522
updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps);
521523
EXPECT_CALL(rtp_video_sender_,
522524
OnBitrateAllocationUpdated(updated_alloc))
523525
.Times(1);
524-
observer->OnBitrateAllocationUpdated(updated_alloc);
526+
sink->OnBitrateAllocationUpdated(updated_alloc);
525527

526528
// This is now a decrease compared to last forward allocation, forward
527529
// immediately.
528530
updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps - 1);
529531
EXPECT_CALL(rtp_video_sender_,
530532
OnBitrateAllocationUpdated(updated_alloc))
531533
.Times(1);
532-
observer->OnBitrateAllocationUpdated(updated_alloc);
534+
sink->OnBitrateAllocationUpdated(updated_alloc);
533535

534536
vss_impl->Stop();
535537
},
@@ -550,8 +552,9 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationOnLayerChange) {
550552
.WillOnce(Return(kBitrateBps));
551553
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
552554
->OnBitrateUpdated(CreateAllocation(kBitrateBps));
553-
VideoBitrateAllocationObserver* const observer =
554-
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
555+
VideoStreamEncoderInterface::EncoderSink* const sink =
556+
static_cast<VideoStreamEncoderInterface::EncoderSink*>(
557+
vss_impl.get());
555558

556559
// Populate a test instance of video bitrate allocation.
557560
VideoBitrateAllocation alloc;
@@ -563,7 +566,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationOnLayerChange) {
563566
// Initial value.
564567
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
565568
.Times(1);
566-
observer->OnBitrateAllocationUpdated(alloc);
569+
sink->OnBitrateAllocationUpdated(alloc);
567570

568571
// Move some bitrate from one layer to a new one, but keep sum the same.
569572
// Since layout has changed, immediately trigger forward.
@@ -574,7 +577,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationOnLayerChange) {
574577
EXPECT_CALL(rtp_video_sender_,
575578
OnBitrateAllocationUpdated(updated_alloc))
576579
.Times(1);
577-
observer->OnBitrateAllocationUpdated(updated_alloc);
580+
sink->OnBitrateAllocationUpdated(updated_alloc);
578581

579582
vss_impl->Stop();
580583
},
@@ -595,8 +598,9 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) {
595598
.WillRepeatedly(Return(kBitrateBps));
596599
static_cast<BitrateAllocatorObserver*>(vss_impl.get())
597600
->OnBitrateUpdated(CreateAllocation(kBitrateBps));
598-
VideoBitrateAllocationObserver* const observer =
599-
static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
601+
VideoStreamEncoderInterface::EncoderSink* const sink =
602+
static_cast<VideoStreamEncoderInterface::EncoderSink*>(
603+
vss_impl.get());
600604

601605
// Populate a test instance of video bitrate allocation.
602606
VideoBitrateAllocation alloc;
@@ -618,14 +622,14 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) {
618622
// Initial value.
619623
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
620624
.Times(1);
621-
observer->OnBitrateAllocationUpdated(alloc);
625+
sink->OnBitrateAllocationUpdated(alloc);
622626
}
623627

624628
{
625629
// Sending same allocation again, this one should be throttled.
626630
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
627631
.Times(0);
628-
observer->OnBitrateAllocationUpdated(alloc);
632+
sink->OnBitrateAllocationUpdated(alloc);
629633
}
630634

631635
clock_.AdvanceTimeMicroseconds(kMaxVbaThrottleTimeMs * 1000);
@@ -634,14 +638,14 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) {
634638
// Sending similar allocation again after timeout, should forward.
635639
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
636640
.Times(1);
637-
observer->OnBitrateAllocationUpdated(alloc);
641+
sink->OnBitrateAllocationUpdated(alloc);
638642
}
639643

640644
{
641645
// Sending similar allocation again without timeout, throttle.
642646
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
643647
.Times(0);
644-
observer->OnBitrateAllocationUpdated(alloc);
648+
sink->OnBitrateAllocationUpdated(alloc);
645649
}
646650

647651
{

0 commit comments

Comments
 (0)