Skip to content

Commit aa6c910

Browse files
tonyherreWebRTC LUCI CQ
authored andcommitted
[116] Set surrogate receive times for transformed sender frames
Without this, 'Sender' frames inserted into the writer of an encoded transform have an invalid receive time (0), which breaks all later heuristics which build on the receive time, eg the VCMTiming estimators used for controlling the playback delay. (cherry picked from commit 9d677f4) Bug: chromium:1463451 Change-Id: I413c884e08986148d4a854cd275212b21d093ceb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311544 Reviewed-by: Guido Urdaneta <[email protected]> Reviewed-by: Danil Chapovalov <[email protected]> Reviewed-by: Palak Agarwal <[email protected]> Reviewed-by: Erik Språng <[email protected]> Commit-Queue: Tony Herre <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#40416} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311662 Commit-Queue: Erik Språng <[email protected]> Cr-Commit-Position: refs/branch-heads/5845@{#4} Cr-Branched-From: f80cf81-refs/heads/main@{#40319}
1 parent 04ee244 commit aa6c910

4 files changed

+28
-13
lines changed

modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,15 @@ class TransformableVideoReceiverFrame
7878
RtpVideoStreamReceiverFrameTransformerDelegate::
7979
RtpVideoStreamReceiverFrameTransformerDelegate(
8080
RtpVideoFrameReceiver* receiver,
81+
Clock* clock,
8182
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
8283
rtc::Thread* network_thread,
8384
uint32_t ssrc)
8485
: receiver_(receiver),
8586
frame_transformer_(std::move(frame_transformer)),
8687
network_thread_(network_thread),
87-
ssrc_(ssrc) {}
88+
ssrc_(ssrc),
89+
clock_(clock) {}
8890

8991
void RtpVideoStreamReceiverFrameTransformerDelegate::Init() {
9092
RTC_DCHECK_RUN_ON(&network_sequence_checker_);
@@ -159,13 +161,14 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::ManageFrame(
159161
RTPVideoHeader video_header = RTPVideoHeader::FromMetadata(metadata);
160162
VideoSendTiming timing;
161163
rtc::ArrayView<const uint8_t> data = transformed_frame->GetData();
164+
int64_t receive_time = clock_->CurrentTime().ms();
162165
receiver_->ManageFrame(std::make_unique<RtpFrameObject>(
163166
/*first_seq_num=*/metadata.GetFrameId().value_or(0),
164167
/*last_seq_num=*/metadata.GetFrameId().value_or(0),
165168
/*markerBit=*/video_header.is_last_frame_in_picture,
166169
/*times_nacked=*/0,
167-
/*first_packet_received_time=*/0,
168-
/*last_packet_received_time=*/0,
170+
/*first_packet_received_time=*/receive_time,
171+
/*last_packet_received_time=*/receive_time,
169172
/*rtp_timestamp=*/transformed_frame->GetTimestamp(),
170173
/*ntp_time_ms=*/0, timing, transformed_frame->GetPayloadType(),
171174
metadata.GetCodec(), metadata.GetRotation(), metadata.GetContentType(),

modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "modules/rtp_rtcp/source/frame_object.h"
1919
#include "rtc_base/system/no_unique_address.h"
2020
#include "rtc_base/thread.h"
21+
#include "system_wrappers/include/clock.h"
2122

2223
namespace webrtc {
2324

@@ -38,6 +39,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
3839
public:
3940
RtpVideoStreamReceiverFrameTransformerDelegate(
4041
RtpVideoFrameReceiver* receiver,
42+
Clock* clock,
4143
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
4244
rtc::Thread* network_thread,
4345
uint32_t ssrc);
@@ -67,6 +69,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
6769
RTC_GUARDED_BY(network_sequence_checker_);
6870
rtc::Thread* const network_thread_;
6971
const uint32_t ssrc_;
72+
Clock* const clock_;
7073
};
7174

7275
} // namespace webrtc

modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
7272
RegisterTransformedFrameCallbackSinkOnInit) {
7373
TestRtpVideoFrameReceiver receiver;
7474
auto frame_transformer(rtc::make_ref_counted<MockFrameTransformer>());
75+
SimulatedClock clock(0);
7576
auto delegate(
7677
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
77-
&receiver, frame_transformer, rtc::Thread::Current(),
78+
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
7879
/*remote_ssrc*/ 1111));
7980
EXPECT_CALL(*frame_transformer,
8081
RegisterTransformedFrameSinkCallback(testing::_, 1111));
@@ -85,9 +86,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
8586
UnregisterTransformedFrameSinkCallbackOnReset) {
8687
TestRtpVideoFrameReceiver receiver;
8788
auto frame_transformer(rtc::make_ref_counted<MockFrameTransformer>());
89+
SimulatedClock clock(0);
8890
auto delegate(
8991
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
90-
&receiver, frame_transformer, rtc::Thread::Current(),
92+
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
9193
/*remote_ssrc*/ 1111));
9294
EXPECT_CALL(*frame_transformer, UnregisterTransformedFrameSinkCallback(1111));
9395
delegate->Reset();
@@ -97,9 +99,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, TransformFrame) {
9799
TestRtpVideoFrameReceiver receiver;
98100
auto frame_transformer(
99101
rtc::make_ref_counted<testing::NiceMock<MockFrameTransformer>>());
102+
SimulatedClock clock(0);
100103
auto delegate(
101104
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
102-
&receiver, frame_transformer, rtc::Thread::Current(),
105+
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
103106
/*remote_ssrc*/ 1111));
104107
auto frame = CreateRtpFrameObject();
105108
EXPECT_CALL(*frame_transformer, Transform);
@@ -112,10 +115,11 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
112115
TestRtpVideoFrameReceiver receiver;
113116
auto mock_frame_transformer(
114117
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
118+
SimulatedClock clock(0);
115119
std::vector<uint32_t> csrcs = {234, 345, 456};
116120
auto delegate =
117121
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
118-
&receiver, mock_frame_transformer, rtc::Thread::Current(),
122+
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
119123
/*remote_ssrc*/ 1111);
120124

121125
rtc::scoped_refptr<TransformedFrameCallback> callback;
@@ -144,9 +148,11 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
144148
TestRtpVideoFrameReceiver receiver;
145149
auto mock_frame_transformer =
146150
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>();
151+
SimulatedClock clock(0);
147152
auto delegate =
148153
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
149-
&receiver, mock_frame_transformer, rtc::Thread::Current(), 1111);
154+
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
155+
1111);
150156
delegate->Init();
151157
RTPVideoHeader video_header;
152158
video_header.width = 1280u;
@@ -191,9 +197,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
191197
TestRtpVideoFrameReceiver receiver;
192198
auto mock_frame_transformer =
193199
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>();
200+
SimulatedClock clock(/*initial_timestamp_us=*/12345000);
194201
auto delegate =
195202
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
196-
&receiver, mock_frame_transformer, rtc::Thread::Current(),
203+
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
197204
/*remote_ssrc*/ 1111);
198205

199206
auto mock_sender_frame =
@@ -218,6 +225,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
218225
EXPECT_CALL(receiver, ManageFrame)
219226
.WillOnce([&](std::unique_ptr<RtpFrameObject> frame) {
220227
EXPECT_EQ(frame->codec_type(), metadata.GetCodec());
228+
EXPECT_EQ(frame->ReceivedTime(), 12345);
221229
});
222230
callback->OnTransformedFrame(std::move(mock_sender_frame));
223231
rtc::ThreadManager::ProcessAllMessageQueuesForTesting();
@@ -232,17 +240,18 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
232240
TestRtpVideoFrameReceiver receiver1;
233241
auto mock_frame_transformer1(
234242
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
243+
SimulatedClock clock(0);
235244
auto delegate1 =
236245
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
237-
&receiver1, mock_frame_transformer1, rtc::Thread::Current(),
246+
&receiver1, &clock, mock_frame_transformer1, rtc::Thread::Current(),
238247
/*remote_ssrc*/ 1111);
239248

240249
TestRtpVideoFrameReceiver receiver2;
241250
auto mock_frame_transformer2(
242251
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
243252
auto delegate2 =
244253
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
245-
&receiver2, mock_frame_transformer2, rtc::Thread::Current(),
254+
&receiver2, &clock, mock_frame_transformer2, rtc::Thread::Current(),
246255
/*remote_ssrc*/ 1111);
247256

248257
delegate1->Init();

video/rtp_video_stream_receiver2.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2(
336336
if (frame_transformer) {
337337
frame_transformer_delegate_ =
338338
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
339-
this, std::move(frame_transformer), rtc::Thread::Current(),
339+
this, clock_, std::move(frame_transformer), rtc::Thread::Current(),
340340
config_.rtp.remote_ssrc);
341341
frame_transformer_delegate_->Init();
342342
}
@@ -950,7 +950,7 @@ void RtpVideoStreamReceiver2::SetDepacketizerToDecoderFrameTransformer(
950950
RTC_DCHECK_RUN_ON(&worker_task_checker_);
951951
frame_transformer_delegate_ =
952952
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
953-
this, std::move(frame_transformer), rtc::Thread::Current(),
953+
this, clock_, std::move(frame_transformer), rtc::Thread::Current(),
954954
config_.rtp.remote_ssrc);
955955
frame_transformer_delegate_->Init();
956956
}

0 commit comments

Comments
 (0)