Move RTPTimestamp offset handling out of encoded transform delegate

Keep the logic managing whether audio RTP timestamps have the random
start offset added or not inside ChannelSend, so that the
ChannelSendFrameTransformerDelegate doesn't need to worry about it.
Crucially, this means that frames moved between senders by encoded
transforms clients will always use the correct offset for the channel
where we actually get sent.

Also rename TS variables throughout both classes to be explicit over
whether the offset has been added or not.

Bug: chromium:1464847
Change-Id: I19955ec4c1cb834161b00dd74622725a070b713a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317900
Commit-Queue: Tony Herre <herre@google.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40655}
This commit is contained in:
Tony Herre 2023-08-29 12:01:32 +02:00 committed by WebRTC LUCI CQ
parent aa7d2f3b20
commit 36500ab634
5 changed files with 71 additions and 41 deletions

View File

@ -165,7 +165,7 @@ class ChannelSend : public ChannelSendInterface,
int32_t SendRtpAudio(AudioFrameType frameType,
uint8_t payloadType,
uint32_t rtp_timestamp,
uint32_t rtp_timestamp_without_offset,
rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms)
RTC_RUN_ON(encoder_queue_);
@ -280,7 +280,7 @@ int32_t ChannelSend::SendData(AudioFrameType frameType,
// Asynchronously transform the payload before sending it. After the payload
// is transformed, the delegate will call SendRtpAudio to send it.
frame_transformer_delegate_->Transform(
frameType, payloadType, rtp_timestamp, rtp_rtcp_->StartTimestamp(),
frameType, payloadType, rtp_timestamp + rtp_rtcp_->StartTimestamp(),
payloadData, payloadSize, absolute_capture_timestamp_ms,
rtp_rtcp_->SSRC());
return 0;
@ -291,7 +291,7 @@ int32_t ChannelSend::SendData(AudioFrameType frameType,
int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType,
uint8_t payloadType,
uint32_t rtp_timestamp,
uint32_t rtp_timestamp_without_offset,
rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) {
if (include_audio_level_indication_.load()) {
@ -341,7 +341,7 @@ int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType,
// Push data from ACM to RTP/RTCP-module to deliver audio frame for
// packetization.
if (!rtp_rtcp_->OnSendingRtpFrame(rtp_timestamp,
if (!rtp_rtcp_->OnSendingRtpFrame(rtp_timestamp_without_offset,
// Leaving the time when this frame was
// received from the capture device as
// undefined for voice for now.
@ -358,7 +358,8 @@ int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType,
// This call will trigger Transport::SendPacket() from the RTP/RTCP module.
if (!rtp_sender_audio_->SendAudio(
frameType, payloadType, rtp_timestamp + rtp_rtcp_->StartTimestamp(),
frameType, payloadType,
rtp_timestamp_without_offset + rtp_rtcp_->StartTimestamp(),
payload.data(), payload.size(), absolute_capture_timestamp_ms)) {
RTC_DLOG(LS_ERROR)
<< "ChannelSend::SendData() failed to send data to RTP/RTCP module";
@ -841,11 +842,14 @@ void ChannelSend::InitFrameTransformerDelegate(
// to send the transformed audio.
ChannelSendFrameTransformerDelegate::SendFrameCallback send_audio_callback =
[this](AudioFrameType frameType, uint8_t payloadType,
uint32_t rtp_timestamp, rtc::ArrayView<const uint8_t> payload,
uint32_t rtp_timestamp_with_offset,
rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
return SendRtpAudio(frameType, payloadType, rtp_timestamp, payload,
absolute_capture_timestamp_ms);
return SendRtpAudio(
frameType, payloadType,
rtp_timestamp_with_offset - rtp_rtcp_->StartTimestamp(), payload,
absolute_capture_timestamp_ms);
};
frame_transformer_delegate_ =
rtc::make_ref_counted<ChannelSendFrameTransformerDelegate>(

View File

@ -50,16 +50,14 @@ class TransformableOutgoingAudioFrame
public:
TransformableOutgoingAudioFrame(AudioFrameType frame_type,
uint8_t payload_type,
uint32_t rtp_timestamp,
uint32_t rtp_start_timestamp,
uint32_t rtp_timestamp_with_offset,
const uint8_t* payload_data,
size_t payload_size,
int64_t absolute_capture_timestamp_ms,
uint32_t ssrc)
: frame_type_(frame_type),
payload_type_(payload_type),
rtp_timestamp_(rtp_timestamp),
rtp_start_timestamp_(rtp_start_timestamp),
rtp_timestamp_with_offset_(rtp_timestamp_with_offset),
payload_(payload_data, payload_size),
absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms),
ssrc_(ssrc) {}
@ -68,10 +66,7 @@ class TransformableOutgoingAudioFrame
void SetData(rtc::ArrayView<const uint8_t> data) override {
payload_.SetData(data.data(), data.size());
}
uint32_t GetTimestamp() const override {
return rtp_timestamp_ + rtp_start_timestamp_;
}
uint32_t GetStartTimestamp() const { return rtp_start_timestamp_; }
uint32_t GetTimestamp() const override { return rtp_timestamp_with_offset_; }
uint32_t GetSsrc() const override { return ssrc_; }
IfaceFrameType Type() const override {
@ -89,8 +84,8 @@ class TransformableOutgoingAudioFrame
return absl::nullopt;
}
void SetRTPTimestamp(uint32_t timestamp) override {
rtp_timestamp_ = timestamp - rtp_start_timestamp_;
void SetRTPTimestamp(uint32_t rtp_timestamp_with_offset) override {
rtp_timestamp_with_offset_ = rtp_timestamp_with_offset;
}
absl::optional<uint64_t> AbsoluteCaptureTimestamp() const override {
@ -100,8 +95,7 @@ class TransformableOutgoingAudioFrame
private:
AudioFrameType frame_type_;
uint8_t payload_type_;
uint32_t rtp_timestamp_;
uint32_t rtp_start_timestamp_;
uint32_t rtp_timestamp_with_offset_;
rtc::Buffer payload_;
int64_t absolute_capture_timestamp_ms_;
uint32_t ssrc_;
@ -133,15 +127,14 @@ void ChannelSendFrameTransformerDelegate::Transform(
AudioFrameType frame_type,
uint8_t payload_type,
uint32_t rtp_timestamp,
uint32_t rtp_start_timestamp,
const uint8_t* payload_data,
size_t payload_size,
int64_t absolute_capture_timestamp_ms,
uint32_t ssrc) {
frame_transformer_->Transform(
std::make_unique<TransformableOutgoingAudioFrame>(
frame_type, payload_type, rtp_timestamp, rtp_start_timestamp,
payload_data, payload_size, absolute_capture_timestamp_ms, ssrc));
frame_type, payload_type, rtp_timestamp, payload_data, payload_size,
absolute_capture_timestamp_ms, ssrc));
}
void ChannelSendFrameTransformerDelegate::OnTransformedFrame(
@ -162,19 +155,11 @@ void ChannelSendFrameTransformerDelegate::SendFrame(
RTC_DCHECK_RUN_ON(encoder_queue_);
if (!send_frame_callback_)
return;
uint32_t rtp_start_timestamp = 0;
if (frame->GetDirection() ==
TransformableFrameInterface::Direction::kSender) {
auto* outgoing_frame =
static_cast<TransformableOutgoingAudioFrame*>(frame.get());
rtp_start_timestamp = outgoing_frame->GetStartTimestamp();
}
auto* transformed_frame =
static_cast<TransformableAudioFrameInterface*>(frame.get());
send_frame_callback_(
InterfaceFrameTypeToInternalFrameType(transformed_frame->Type()),
transformed_frame->GetPayloadType(),
transformed_frame->GetTimestamp() - rtp_start_timestamp,
transformed_frame->GetPayloadType(), transformed_frame->GetTimestamp(),
transformed_frame->GetData(),
transformed_frame->AbsoluteCaptureTimestamp()
? *transformed_frame->AbsoluteCaptureTimestamp()
@ -187,9 +172,8 @@ std::unique_ptr<TransformableAudioFrameInterface> CloneSenderAudioFrame(
return std::make_unique<TransformableOutgoingAudioFrame>(
InterfaceFrameTypeToInternalFrameType(original->Type()),
original->GetPayloadType(), original->GetTimestamp(),
/*rtp_start_timestamp=*/0u, original->GetData().data(),
original->GetData().size(), original->GetTimestamp(),
original->GetSsrc());
original->GetData().data(), original->GetData().size(),
original->GetTimestamp(), original->GetSsrc());
}
} // namespace webrtc

View File

@ -32,7 +32,7 @@ class ChannelSendFrameTransformerDelegate : public TransformedFrameCallback {
using SendFrameCallback =
std::function<int32_t(AudioFrameType frameType,
uint8_t payloadType,
uint32_t rtp_timestamp,
uint32_t rtp_timestamp_with_offset,
rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms)>;
ChannelSendFrameTransformerDelegate(
@ -54,7 +54,6 @@ class ChannelSendFrameTransformerDelegate : public TransformedFrameCallback {
void Transform(AudioFrameType frame_type,
uint8_t payload_type,
uint32_t rtp_timestamp,
uint32_t rtp_start_timestamp,
const uint8_t* payload_data,
size_t payload_size,
int64_t absolute_capture_timestamp_ms,

View File

@ -114,8 +114,8 @@ TEST(ChannelSendFrameTransformerDelegateTest,
[&callback](std::unique_ptr<TransformableFrameInterface> frame) {
callback->OnTransformedFrame(std::move(frame));
});
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, 0, data, sizeof(data),
0, 0);
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, data, sizeof(data), 0,
0);
channel_queue.WaitForPreviouslyPostedTasks();
}
@ -144,8 +144,8 @@ TEST(ChannelSendFrameTransformerDelegateTest,
[&callback](std::unique_ptr<TransformableFrameInterface> frame) {
callback->OnTransformedFrame(CreateMockReceiverFrame());
});
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, 0, data, sizeof(data),
0, 0);
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, data, sizeof(data), 0,
0);
channel_queue.WaitForPreviouslyPostedTasks();
}

View File

@ -19,7 +19,9 @@
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "call/rtp_transport_controller_send.h"
#include "rtc_base/gunit.h"
#include "test/gtest.h"
#include "test/mock_frame_transformer.h"
#include "test/mock_transport.h"
#include "test/scoped_key_value_config.h"
#include "test/time_controller/simulated_time_controller.h"
@ -31,6 +33,7 @@ namespace {
using ::testing::Invoke;
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::SaveArg;
constexpr int kRtcpIntervalMs = 1000;
constexpr int kSsrc = 333;
@ -145,6 +148,46 @@ TEST_F(ChannelSendTest, IncreaseRtpTimestampByPauseDuration) {
EXPECT_EQ(timestamp_gap_ms, 10020);
}
TEST_F(ChannelSendTest, FrameTransformerGetsCorrectTimestamp) {
rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer =
rtc::make_ref_counted<MockFrameTransformer>();
channel_->SetEncoderToPacketizerFrameTransformer(mock_frame_transformer);
rtc::scoped_refptr<TransformedFrameCallback> callback;
EXPECT_CALL(*mock_frame_transformer, RegisterTransformedFrameCallback)
.WillOnce(SaveArg<0>(&callback));
EXPECT_CALL(*mock_frame_transformer, UnregisterTransformedFrameCallback);
absl::optional<uint32_t> sent_timestamp;
auto send_rtp = [&](rtc::ArrayView<const uint8_t> data,
const PacketOptions& options) {
RtpPacketReceived packet;
packet.Parse(data);
if (!sent_timestamp) {
sent_timestamp = packet.Timestamp();
}
return true;
};
EXPECT_CALL(transport_, SendRtp).WillRepeatedly(Invoke(send_rtp));
channel_->StartSend();
int64_t transformable_frame_timestamp = -1;
EXPECT_CALL(*mock_frame_transformer, Transform)
.WillOnce([&](std::unique_ptr<TransformableFrameInterface> frame) {
transformable_frame_timestamp = frame->GetTimestamp();
callback->OnTransformedFrame(std::move(frame));
});
// Insert two frames which should trigger a new packet.
ProcessNextFrame();
ProcessNextFrame();
// Ensure the RTP timestamp on the frame passed to the transformer
// includes the RTP offset and matches the actual RTP timestamp on the sent
// packet.
EXPECT_EQ_WAIT(transformable_frame_timestamp,
0 + channel_->GetRtpRtcp()->StartTimestamp(), 1000);
EXPECT_TRUE_WAIT(sent_timestamp, 1000);
EXPECT_EQ(*sent_timestamp, transformable_frame_timestamp);
}
} // namespace
} // namespace voe
} // namespace webrtc