admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cleanup IncludeCaptureClockOffset field trial

Browse Source 
Bug: webrtc:10739
Change-Id: I642cdf7574277c4c1b4ceb62b9e8a6905325dcfb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299004
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39669}
This commit is contained in:
Danil Chapovalov 2023-03-23 18:29:55 +01:00 committed by WebRTC LUCI CQ
parent 2d1fa4713f
commit 7f60e5f753
6 changed files with 10 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
modules/rtp_rtcp/source/rtp_sender_audio.cc
View File

@ -47,18 +47,12 @@ namespace {
RTC_CHECK_NOTREACHED();
}
constexpr char kIncludeCaptureClockOffset[] =
"WebRTC-IncludeCaptureClockOffset";
} // namespace
RTPSenderAudio::RTPSenderAudio(Clock* clock, RTPSender* rtp_sender)
: clock_(clock),
rtp_sender_(rtp_sender),
absolute_capture_time_sender_(clock),
include_capture_clock_offset_(
!absl::StartsWith(field_trials_.Lookup(kIncludeCaptureClockOffset),
"Disabled")) {
absolute_capture_time_sender_(clock) {
RTC_DCHECK(clock_);
}
@ -284,8 +278,7 @@ bool RTPSenderAudio::SendAudio(AudioFrameType frame_type,
encoder_rtp_timestamp_frequency.value_or(0),
Int64MsToUQ32x32(clock_->ConvertTimestampToNtpTimeInMilliseconds(
absolute_capture_timestamp_ms)),
/*estimated_capture_clock_offset=*/
include_capture_clock_offset_ ? absl::make_optional(0) : absl::nullopt);
/*estimated_capture_clock_offset=*/0);
if (absolute_capture_time) {
// It also checks that extension was registered during SDP negotiation. If
// not then setter won't do anything.

4
modules/rtp_rtcp/source/rtp_sender_audio.h
View File

@ -17,7 +17,6 @@
#include <memory>
#include "absl/strings/string_view.h"
#include "api/transport/field_trial_based_config.h"
#include "modules/audio_coding/include/audio_coding_module_typedefs.h"
#include "modules/rtp_rtcp/source/absolute_capture_time_sender.h"
#include "modules/rtp_rtcp/source/dtmf_queue.h"
@ -112,9 +111,6 @@ class RTPSenderAudio {
RTC_GUARDED_BY(send_audio_mutex_);
AbsoluteCaptureTimeSender absolute_capture_time_sender_;
const FieldTrialBasedConfig field_trials_;
const bool include_capture_clock_offset_;
};
} // namespace webrtc

43
modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc
View File

@ -13,14 +13,12 @@
#include <memory>
#include <vector>
#include "api/transport/field_trial_based_config.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "rtc_base/thread.h"
#include "test/field_trial.h"
#include "test/gmock.h"
#include "test/gtest.h"
@ -150,41 +148,6 @@ TEST_F(RtpSenderAudioTest, SendAudioWithoutAbsoluteCaptureTime) {
.HasExtension<AbsoluteCaptureTimeExtension>());
}
// Essentially the same test as
// SendAudioWithAbsoluteCaptureTimeWithCaptureClockOffset but with a field
// trial. We will remove this test eventually.
TEST_F(RtpSenderAudioTest, SendAudioWithAbsoluteCaptureTime) {
// Recreate rtp_sender_audio_ with new field trial.
test::ScopedFieldTrials field_trial(
"WebRTC-IncludeCaptureClockOffset/Disabled/");
rtp_sender_audio_ =
std::make_unique<RTPSenderAudio>(&fake_clock_, rtp_module_->RtpSender());
rtp_module_->RegisterRtpHeaderExtension(AbsoluteCaptureTimeExtension::Uri(),
kAbsoluteCaptureTimeExtensionId);
constexpr uint32_t kAbsoluteCaptureTimestampMs = 521;
const char payload_name[] = "audio";
const uint8_t payload_type = 127;
ASSERT_EQ(0, rtp_sender_audio_->RegisterAudioPayload(
payload_name, payload_type, 48000, 0, 1500));
uint8_t payload[] = {47, 11, 32, 93, 89};
ASSERT_TRUE(rtp_sender_audio_->SendAudio(
AudioFrameType::kAudioFrameCN, payload_type, 4321, payload,
sizeof(payload), kAbsoluteCaptureTimestampMs));
auto absolute_capture_time =
transport_.last_sent_packet()
.GetExtension<AbsoluteCaptureTimeExtension>();
EXPECT_TRUE(absolute_capture_time);
EXPECT_EQ(
absolute_capture_time->absolute_capture_timestamp,
Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds(
kAbsoluteCaptureTimestampMs)));
EXPECT_FALSE(
absolute_capture_time->estimated_capture_clock_offset.has_value());
}
TEST_F(RtpSenderAudioTest,
SendAudioWithAbsoluteCaptureTimeWithCaptureClockOffset) {
rtp_module_->RegisterRtpHeaderExtension(AbsoluteCaptureTimeExtension::Uri(),
@ -203,14 +166,12 @@ TEST_F(RtpSenderAudioTest,
auto absolute_capture_time =
transport_.last_sent_packet()
.GetExtension<AbsoluteCaptureTimeExtension>();
EXPECT_TRUE(absolute_capture_time);
ASSERT_TRUE(absolute_capture_time);
EXPECT_EQ(
absolute_capture_time->absolute_capture_timestamp,
Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds(
kAbsoluteCaptureTimestampMs)));
EXPECT_TRUE(
absolute_capture_time->estimated_capture_clock_offset.has_value());
EXPECT_EQ(0, *absolute_capture_time->estimated_capture_clock_offset);
EXPECT_EQ(absolute_capture_time->estimated_capture_clock_offset, 0);
}
// As RFC4733, named telephone events are carried as part of the audio stream

11
modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -46,8 +46,6 @@ namespace webrtc {
namespace {
constexpr size_t kRedForFecHeaderLength = 1;
constexpr int64_t kMaxUnretransmittableFrameIntervalMs = 33 * 4;
constexpr char kIncludeCaptureClockOffset[] =
"WebRTC-IncludeCaptureClockOffset";
void BuildRedPayload(const RtpPacketToSend& media_packet,
RtpPacketToSend* red_packet) {
@ -172,10 +170,7 @@ RTPSenderVideo::RTPSenderVideo(const Config& config)
rtp_sender_->SSRC(),
rtp_sender_->Csrcs(),
config.task_queue_factory)
: nullptr),
include_capture_clock_offset_(!absl::StartsWith(
config.field_trials->Lookup(kIncludeCaptureClockOffset),
"Disabled")) {
: nullptr) {
if (frame_transformer_delegate_)
frame_transformer_delegate_->Init();
}
@ -572,9 +567,7 @@ bool RTPSenderVideo::SendVideo(
video_header.absolute_capture_time->absolute_capture_timestamp =
Int64MsToUQ32x32(
clock_->ConvertTimestampToNtpTime(*capture_time).ToMs());
if (include_capture_clock_offset_) {
video_header.absolute_capture_time->estimated_capture_clock_offset = 0;
}
video_header.absolute_capture_time->estimated_capture_clock_offset = 0;
}
// Let `absolute_capture_time_sender_` decide if the extension should be sent.

2
modules/rtp_rtcp/source/rtp_sender_video.h
View File

@ -254,8 +254,6 @@ class RTPSenderVideo : public RTPVideoFrameSenderInterface {
const rtc::scoped_refptr<RTPSenderVideoFrameTransformerDelegate>
frame_transformer_delegate_;
const bool include_capture_clock_offset_;
};
} // namespace webrtc

66
modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
View File

@ -16,13 +16,11 @@
#include <vector>
#include "absl/memory/memory.h"
#include "api/field_trials_registry.h"
#include "api/frame_transformer_factory.h"
#include "api/rtp_headers.h"
#include "api/task_queue/task_queue_base.h"
#include "api/task_queue/task_queue_factory.h"
#include "api/test/mock_frame_encryptor.h"
#include "api/transport/field_trial_based_config.h"
#include "api/transport/rtp/dependency_descriptor.h"
#include "api/units/timestamp.h"
#include "api/video/video_codec_constants.h"
@ -45,6 +43,7 @@
#include "rtc_base/logging.h"
#include "rtc_base/rate_limiter.h"
#include "rtc_base/thread.h"
#include "test/explicit_key_value_config.h"
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/mock_frame_transformer.h"
@ -156,25 +155,6 @@ class TestRtpSenderVideo : public RTPSenderVideo {
}
};
class FieldTrials : public FieldTrialsRegistry {
public:
FieldTrials() : include_capture_clock_offset_(false) {}
void set_include_capture_clock_offset(bool include_capture_clock_offset) {
include_capture_clock_offset_ = include_capture_clock_offset;
}
private:
std::string GetValue(absl::string_view key) const override {
if (key == "WebRTC-IncludeCaptureClockOffset") {
return include_capture_clock_offset_ ? "" : "Disabled";
}
return "";
}
bool include_capture_clock_offset_;
};
class RtpSenderVideoTest : public ::testing::Test {
public:
RtpSenderVideoTest()
@ -205,7 +185,7 @@ class RtpSenderVideoTest : public ::testing::Test {
protected:
rtc::AutoThread main_thread_;
const RtpRtcpInterface::Configuration config_;
FieldTrials field_trials_;
test::ExplicitKeyValueConfig field_trials_{""};
SimulatedClock fake_clock_;
LoopbackTransportTest transport_;
RateLimiter retransmission_rate_limiter_;
@ -1236,41 +1216,6 @@ TEST_F(RtpSenderVideoTest, VideoLayersAllocationNotSentOnHigherTemporalLayers) {
.HasExtension<RtpVideoLayersAllocationExtension>());
}
TEST_F(RtpSenderVideoTest, AbsoluteCaptureTime) {
constexpr int64_t kAbsoluteCaptureTimestampMs = 12345678;
uint8_t kFrame[kMaxPacketLength];
rtp_module_->RegisterRtpHeaderExtension(AbsoluteCaptureTimeExtension::Uri(),
kAbsoluteCaptureTimeExtensionId);
RTPVideoHeader hdr;
hdr.frame_type = VideoFrameType::kVideoFrameKey;
rtp_sender_video_->SendVideo(kPayload, kType, kTimestamp,
kAbsoluteCaptureTimestampMs, kFrame, hdr,
kDefaultExpectedRetransmissionTimeMs, {});
absl::optional<AbsoluteCaptureTime> absolute_capture_time;
// It is expected that one and only one of the packets sent on this video
// frame has absolute capture time header extension.
for (const RtpPacketReceived& packet : transport_.sent_packets()) {
if (absolute_capture_time.has_value()) {
EXPECT_FALSE(packet.HasExtension<AbsoluteCaptureTimeExtension>());
} else {
absolute_capture_time =
packet.GetExtension<AbsoluteCaptureTimeExtension>();
}
}
// Verify the capture timestamp and that the clock offset is not set.
ASSERT_TRUE(absolute_capture_time.has_value());
EXPECT_EQ(
absolute_capture_time->absolute_capture_timestamp,
Int64MsToUQ32x32(fake_clock_.ConvertTimestampToNtpTimeInMilliseconds(
kAbsoluteCaptureTimestampMs)));
EXPECT_FALSE(
absolute_capture_time->estimated_capture_clock_offset.has_value());
}
TEST_F(RtpSenderVideoTest,
AbsoluteCaptureTimeNotForwardedWhenImageHasNoCaptureTime) {
uint8_t kFrame[kMaxPacketLength];
@ -1289,10 +1234,7 @@ TEST_F(RtpSenderVideoTest,
}
}
// Essentially the same test as AbsoluteCaptureTime but with a field trial.
// After the field trial is experimented, we will remove AbsoluteCaptureTime.
TEST_F(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) {
field_trials_.set_include_capture_clock_offset(true);
TEST_F(RtpSenderVideoTest, AbsoluteCaptureTime) {
rtp_sender_video_ = std::make_unique<TestRtpSenderVideo>(
&fake_clock_, rtp_module_->RtpSender(), field_trials_);
@ -1499,7 +1441,7 @@ class RtpSenderVideoWithFrameTransformerTest : public ::testing::Test {
protected:
GlobalSimulatedTimeController time_controller_;
FieldTrialBasedConfig field_trials_;
test::ExplicitKeyValueConfig field_trials_{""};
LoopbackTransportTest transport_;
RateLimiter retransmission_rate_limiter_;
std::unique_ptr<ModuleRtpRtcpImpl2> rtp_module_;