Skip to content

Commit

Permalink
[116] Set surrogate receive times for transformed sender frames
Browse files Browse the repository at this point in the history
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 <guidou@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Palak Agarwal <agpalak@google.com>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Tony Herre <herre@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#40416}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311662
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/branch-heads/5845@{#4}
Cr-Branched-From: f80cf81-refs/heads/main@{#40319}
  • Loading branch information
tonyherre authored and WebRTC LUCI CQ committed Jul 12, 2023
1 parent 04ee244 commit aa6c910
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,15 @@ class TransformableVideoReceiverFrame
RtpVideoStreamReceiverFrameTransformerDelegate::
RtpVideoStreamReceiverFrameTransformerDelegate(
RtpVideoFrameReceiver* receiver,
Clock* clock,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
rtc::Thread* network_thread,
uint32_t ssrc)
: receiver_(receiver),
frame_transformer_(std::move(frame_transformer)),
network_thread_(network_thread),
ssrc_(ssrc) {}
ssrc_(ssrc),
clock_(clock) {}

void RtpVideoStreamReceiverFrameTransformerDelegate::Init() {
RTC_DCHECK_RUN_ON(&network_sequence_checker_);
Expand Down Expand Up @@ -159,13 +161,14 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::ManageFrame(
RTPVideoHeader video_header = RTPVideoHeader::FromMetadata(metadata);
VideoSendTiming timing;
rtc::ArrayView<const uint8_t> data = transformed_frame->GetData();
int64_t receive_time = clock_->CurrentTime().ms();
receiver_->ManageFrame(std::make_unique<RtpFrameObject>(
/*first_seq_num=*/metadata.GetFrameId().value_or(0),
/*last_seq_num=*/metadata.GetFrameId().value_or(0),
/*markerBit=*/video_header.is_last_frame_in_picture,
/*times_nacked=*/0,
/*first_packet_received_time=*/0,
/*last_packet_received_time=*/0,
/*first_packet_received_time=*/receive_time,
/*last_packet_received_time=*/receive_time,
/*rtp_timestamp=*/transformed_frame->GetTimestamp(),
/*ntp_time_ms=*/0, timing, transformed_frame->GetPayloadType(),
metadata.GetCodec(), metadata.GetRotation(), metadata.GetContentType(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "modules/rtp_rtcp/source/frame_object.h"
#include "rtc_base/system/no_unique_address.h"
#include "rtc_base/thread.h"
#include "system_wrappers/include/clock.h"

namespace webrtc {

Expand All @@ -38,6 +39,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
public:
RtpVideoStreamReceiverFrameTransformerDelegate(
RtpVideoFrameReceiver* receiver,
Clock* clock,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
rtc::Thread* network_thread,
uint32_t ssrc);
Expand Down Expand Up @@ -67,6 +69,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
RTC_GUARDED_BY(network_sequence_checker_);
rtc::Thread* const network_thread_;
const uint32_t ssrc_;
Clock* const clock_;
};

} // namespace webrtc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
RegisterTransformedFrameCallbackSinkOnInit) {
TestRtpVideoFrameReceiver receiver;
auto frame_transformer(rtc::make_ref_counted<MockFrameTransformer>());
SimulatedClock clock(0);
auto delegate(
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, frame_transformer, rtc::Thread::Current(),
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
/*remote_ssrc*/ 1111));
EXPECT_CALL(*frame_transformer,
RegisterTransformedFrameSinkCallback(testing::_, 1111));
Expand All @@ -85,9 +86,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
UnregisterTransformedFrameSinkCallbackOnReset) {
TestRtpVideoFrameReceiver receiver;
auto frame_transformer(rtc::make_ref_counted<MockFrameTransformer>());
SimulatedClock clock(0);
auto delegate(
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, frame_transformer, rtc::Thread::Current(),
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
/*remote_ssrc*/ 1111));
EXPECT_CALL(*frame_transformer, UnregisterTransformedFrameSinkCallback(1111));
delegate->Reset();
Expand All @@ -97,9 +99,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, TransformFrame) {
TestRtpVideoFrameReceiver receiver;
auto frame_transformer(
rtc::make_ref_counted<testing::NiceMock<MockFrameTransformer>>());
SimulatedClock clock(0);
auto delegate(
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, frame_transformer, rtc::Thread::Current(),
&receiver, &clock, frame_transformer, rtc::Thread::Current(),
/*remote_ssrc*/ 1111));
auto frame = CreateRtpFrameObject();
EXPECT_CALL(*frame_transformer, Transform);
Expand All @@ -112,10 +115,11 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
TestRtpVideoFrameReceiver receiver;
auto mock_frame_transformer(
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
SimulatedClock clock(0);
std::vector<uint32_t> csrcs = {234, 345, 456};
auto delegate =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, mock_frame_transformer, rtc::Thread::Current(),
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
/*remote_ssrc*/ 1111);

rtc::scoped_refptr<TransformedFrameCallback> callback;
Expand Down Expand Up @@ -144,9 +148,11 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
TestRtpVideoFrameReceiver receiver;
auto mock_frame_transformer =
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>();
SimulatedClock clock(0);
auto delegate =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, mock_frame_transformer, rtc::Thread::Current(), 1111);
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
1111);
delegate->Init();
RTPVideoHeader video_header;
video_header.width = 1280u;
Expand Down Expand Up @@ -191,9 +197,10 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
TestRtpVideoFrameReceiver receiver;
auto mock_frame_transformer =
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>();
SimulatedClock clock(/*initial_timestamp_us=*/12345000);
auto delegate =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver, mock_frame_transformer, rtc::Thread::Current(),
&receiver, &clock, mock_frame_transformer, rtc::Thread::Current(),
/*remote_ssrc*/ 1111);

auto mock_sender_frame =
Expand All @@ -218,6 +225,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
EXPECT_CALL(receiver, ManageFrame)
.WillOnce([&](std::unique_ptr<RtpFrameObject> frame) {
EXPECT_EQ(frame->codec_type(), metadata.GetCodec());
EXPECT_EQ(frame->ReceivedTime(), 12345);
});
callback->OnTransformedFrame(std::move(mock_sender_frame));
rtc::ThreadManager::ProcessAllMessageQueuesForTesting();
Expand All @@ -232,17 +240,18 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
TestRtpVideoFrameReceiver receiver1;
auto mock_frame_transformer1(
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
SimulatedClock clock(0);
auto delegate1 =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver1, mock_frame_transformer1, rtc::Thread::Current(),
&receiver1, &clock, mock_frame_transformer1, rtc::Thread::Current(),
/*remote_ssrc*/ 1111);

TestRtpVideoFrameReceiver receiver2;
auto mock_frame_transformer2(
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>());
auto delegate2 =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
&receiver2, mock_frame_transformer2, rtc::Thread::Current(),
&receiver2, &clock, mock_frame_transformer2, rtc::Thread::Current(),
/*remote_ssrc*/ 1111);

delegate1->Init();
Expand Down
4 changes: 2 additions & 2 deletions video/rtp_video_stream_receiver2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2(
if (frame_transformer) {
frame_transformer_delegate_ =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
this, std::move(frame_transformer), rtc::Thread::Current(),
this, clock_, std::move(frame_transformer), rtc::Thread::Current(),
config_.rtp.remote_ssrc);
frame_transformer_delegate_->Init();
}
Expand Down Expand Up @@ -950,7 +950,7 @@ void RtpVideoStreamReceiver2::SetDepacketizerToDecoderFrameTransformer(
RTC_DCHECK_RUN_ON(&worker_task_checker_);
frame_transformer_delegate_ =
rtc::make_ref_counted<RtpVideoStreamReceiverFrameTransformerDelegate>(
this, std::move(frame_transformer), rtc::Thread::Current(),
this, clock_, std::move(frame_transformer), rtc::Thread::Current(),
config_.rtp.remote_ssrc);
frame_transformer_delegate_->Init();
}
Expand Down

0 comments on commit aa6c910

Please sign in to comment.