Ensure cloning and then sending audio encoded frames propagates CSRCs

Bug: chromium:1508337
Change-Id: I9f28fc0958d28bc97f9378a46fbec3e45148736f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330260
Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
Commit-Queue: Tony Herre <herre@google.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41337}
This commit is contained in:
Tony Herre 2023-12-07 12:39:40 +01:00 committed by WebRTC LUCI CQ
parent b54bf8a9af
commit 5f3ac43551
8 changed files with 126 additions and 26 deletions

View File

@ -223,6 +223,7 @@ if (rtc_include_tests) {
"utility:utility_tests", "utility:utility_tests",
"//testing/gtest", "//testing/gtest",
] ]
absl_deps = [ "//third_party/abseil-cpp/absl/memory" ]
} }
rtc_library("channel_receive_unittest") { rtc_library("channel_receive_unittest") {

View File

@ -169,7 +169,8 @@ class ChannelSend : public ChannelSendInterface,
uint8_t payloadType, uint8_t payloadType,
uint32_t rtp_timestamp_without_offset, uint32_t rtp_timestamp_without_offset,
rtc::ArrayView<const uint8_t> payload, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs)
RTC_RUN_ON(encoder_queue_); RTC_RUN_ON(encoder_queue_);
void OnReceivedRtt(int64_t rtt_ms); void OnReceivedRtt(int64_t rtt_ms);
@ -294,14 +295,15 @@ int32_t ChannelSend::SendData(AudioFrameType frameType,
return 0; return 0;
} }
return SendRtpAudio(frameType, payloadType, rtp_timestamp, payload, return SendRtpAudio(frameType, payloadType, rtp_timestamp, payload,
absolute_capture_timestamp_ms); absolute_capture_timestamp_ms, /*csrcs=*/{});
} }
int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType, int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType,
uint8_t payloadType, uint8_t payloadType,
uint32_t rtp_timestamp_without_offset, uint32_t rtp_timestamp_without_offset,
rtc::ArrayView<const uint8_t> payload, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) { int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs) {
// E2EE Custom Audio Frame Encryption (This is optional). // E2EE Custom Audio Frame Encryption (This is optional).
// Keep this buffer around for the lifetime of the send call. // Keep this buffer around for the lifetime of the send call.
rtc::Buffer encrypted_audio_payload; rtc::Buffer encrypted_audio_payload;
@ -363,7 +365,8 @@ int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType,
.payload = payload, .payload = payload,
.payload_id = payloadType, .payload_id = payloadType,
.rtp_timestamp = .rtp_timestamp =
rtp_timestamp_without_offset + rtp_rtcp_->StartTimestamp()}; rtp_timestamp_without_offset + rtp_rtcp_->StartTimestamp(),
.csrcs = csrcs};
if (absolute_capture_timestamp_ms > 0) { if (absolute_capture_timestamp_ms > 0) {
frame.capture_time = Timestamp::Millis(absolute_capture_timestamp_ms); frame.capture_time = Timestamp::Millis(absolute_capture_timestamp_ms);
} }
@ -859,12 +862,13 @@ void ChannelSend::InitFrameTransformerDelegate(
[this](AudioFrameType frameType, uint8_t payloadType, [this](AudioFrameType frameType, uint8_t payloadType,
uint32_t rtp_timestamp_with_offset, uint32_t rtp_timestamp_with_offset,
rtc::ArrayView<const uint8_t> payload, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) { int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs) {
RTC_DCHECK_RUN_ON(&encoder_queue_); RTC_DCHECK_RUN_ON(&encoder_queue_);
return SendRtpAudio( return SendRtpAudio(
frameType, payloadType, frameType, payloadType,
rtp_timestamp_with_offset - rtp_rtcp_->StartTimestamp(), payload, rtp_timestamp_with_offset - rtp_rtcp_->StartTimestamp(), payload,
absolute_capture_timestamp_ms); absolute_capture_timestamp_ms, csrcs);
}; };
frame_transformer_delegate_ = frame_transformer_delegate_ =
rtc::make_ref_counted<ChannelSendFrameTransformerDelegate>( rtc::make_ref_counted<ChannelSendFrameTransformerDelegate>(

View File

@ -11,6 +11,7 @@
#include "audio/channel_send_frame_transformer_delegate.h" #include "audio/channel_send_frame_transformer_delegate.h"
#include <utility> #include <utility>
#include <vector>
namespace webrtc { namespace webrtc {
namespace { namespace {
@ -56,6 +57,7 @@ class TransformableOutgoingAudioFrame
size_t payload_size, size_t payload_size,
absl::optional<uint64_t> absolute_capture_timestamp_ms, absl::optional<uint64_t> absolute_capture_timestamp_ms,
uint32_t ssrc, uint32_t ssrc,
std::vector<uint32_t> csrcs,
const std::string& codec_mime_type) const std::string& codec_mime_type)
: frame_type_(frame_type), : frame_type_(frame_type),
payload_type_(payload_type), payload_type_(payload_type),
@ -63,6 +65,7 @@ class TransformableOutgoingAudioFrame
payload_(payload_data, payload_size), payload_(payload_data, payload_size),
absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms), absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms),
ssrc_(ssrc), ssrc_(ssrc),
csrcs_(std::move(csrcs)),
codec_mime_type_(codec_mime_type) {} codec_mime_type_(codec_mime_type) {}
~TransformableOutgoingAudioFrame() override = default; ~TransformableOutgoingAudioFrame() override = default;
rtc::ArrayView<const uint8_t> GetData() const override { return payload_; } rtc::ArrayView<const uint8_t> GetData() const override { return payload_; }
@ -81,7 +84,7 @@ class TransformableOutgoingAudioFrame
std::string GetMimeType() const override { return codec_mime_type_; } std::string GetMimeType() const override { return codec_mime_type_; }
rtc::ArrayView<const uint32_t> GetContributingSources() const override { rtc::ArrayView<const uint32_t> GetContributingSources() const override {
return {}; return csrcs_;
} }
const absl::optional<uint16_t> SequenceNumber() const override { const absl::optional<uint16_t> SequenceNumber() const override {
@ -103,6 +106,7 @@ class TransformableOutgoingAudioFrame
rtc::Buffer payload_; rtc::Buffer payload_;
absl::optional<uint64_t> absolute_capture_timestamp_ms_; absl::optional<uint64_t> absolute_capture_timestamp_ms_;
uint32_t ssrc_; uint32_t ssrc_;
std::vector<uint32_t> csrcs_;
std::string codec_mime_type_; std::string codec_mime_type_;
}; };
} // namespace } // namespace
@ -143,14 +147,15 @@ void ChannelSendFrameTransformerDelegate::Transform(
send_frame_callback_( send_frame_callback_(
frame_type, payload_type, rtp_timestamp, frame_type, payload_type, rtp_timestamp,
rtc::ArrayView<const uint8_t>(payload_data, payload_size), rtc::ArrayView<const uint8_t>(payload_data, payload_size),
absolute_capture_timestamp_ms); absolute_capture_timestamp_ms, /*csrcs=*/{});
return; return;
} }
} }
frame_transformer_->Transform( frame_transformer_->Transform(
std::make_unique<TransformableOutgoingAudioFrame>( std::make_unique<TransformableOutgoingAudioFrame>(
frame_type, payload_type, rtp_timestamp, payload_data, payload_size, frame_type, payload_type, rtp_timestamp, payload_data, payload_size,
absolute_capture_timestamp_ms, ssrc, codec_mimetype)); absolute_capture_timestamp_ms, ssrc,
/*csrcs=*/std::vector<uint32_t>(), codec_mimetype));
} }
void ChannelSendFrameTransformerDelegate::OnTransformedFrame( void ChannelSendFrameTransformerDelegate::OnTransformedFrame(
@ -184,17 +189,21 @@ void ChannelSendFrameTransformerDelegate::SendFrame(
transformed_frame->GetData(), transformed_frame->GetData(),
transformed_frame->AbsoluteCaptureTimestamp() transformed_frame->AbsoluteCaptureTimestamp()
? *transformed_frame->AbsoluteCaptureTimestamp() ? *transformed_frame->AbsoluteCaptureTimestamp()
: 0); : 0,
transformed_frame->GetContributingSources());
} }
std::unique_ptr<TransformableAudioFrameInterface> CloneSenderAudioFrame( std::unique_ptr<TransformableAudioFrameInterface> CloneSenderAudioFrame(
TransformableAudioFrameInterface* original) { TransformableAudioFrameInterface* original) {
std::vector<uint32_t> csrcs;
csrcs.assign(original->GetContributingSources().begin(),
original->GetContributingSources().end());
return std::make_unique<TransformableOutgoingAudioFrame>( return std::make_unique<TransformableOutgoingAudioFrame>(
InterfaceFrameTypeToInternalFrameType(original->Type()), InterfaceFrameTypeToInternalFrameType(original->Type()),
original->GetPayloadType(), original->GetTimestamp(), original->GetPayloadType(), original->GetTimestamp(),
original->GetData().data(), original->GetData().size(), original->GetData().data(), original->GetData().size(),
original->AbsoluteCaptureTimestamp(), original->GetSsrc(), original->AbsoluteCaptureTimestamp(), original->GetSsrc(),
original->GetMimeType()); std::move(csrcs), original->GetMimeType());
} }
} // namespace webrtc } // namespace webrtc

View File

@ -35,7 +35,8 @@ class ChannelSendFrameTransformerDelegate : public TransformedFrameCallback {
uint8_t payloadType, uint8_t payloadType,
uint32_t rtp_timestamp_with_offset, uint32_t rtp_timestamp_with_offset,
rtc::ArrayView<const uint8_t> payload, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms)>; int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs)>;
ChannelSendFrameTransformerDelegate( ChannelSendFrameTransformerDelegate(
SendFrameCallback send_frame_callback, SendFrameCallback send_frame_callback,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer, rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,

View File

@ -12,7 +12,9 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include <vector>
#include "absl/memory/memory.h"
#include "rtc_base/task_queue_for_test.h" #include "rtc_base/task_queue_for_test.h"
#include "test/gmock.h" #include "test/gmock.h"
#include "test/gtest.h" #include "test/gtest.h"
@ -24,10 +26,13 @@ namespace {
using ::testing::_; using ::testing::_;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
using ::testing::NiceMock; using ::testing::NiceMock;
using ::testing::Return; using ::testing::Return;
using ::testing::SaveArg; using ::testing::SaveArg;
const uint8_t mock_data[] = {1, 2, 3, 4};
class MockChannelSend { class MockChannelSend {
public: public:
MockChannelSend() = default; MockChannelSend() = default;
@ -39,30 +44,56 @@ class MockChannelSend {
uint8_t payloadType, uint8_t payloadType,
uint32_t rtp_timestamp, uint32_t rtp_timestamp,
rtc::ArrayView<const uint8_t> payload, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms)); int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs));
ChannelSendFrameTransformerDelegate::SendFrameCallback callback() { ChannelSendFrameTransformerDelegate::SendFrameCallback callback() {
return [this](AudioFrameType frameType, uint8_t payloadType, return [this](AudioFrameType frameType, uint8_t payloadType,
uint32_t rtp_timestamp, rtc::ArrayView<const uint8_t> payload, uint32_t rtp_timestamp, rtc::ArrayView<const uint8_t> payload,
int64_t absolute_capture_timestamp_ms) { int64_t absolute_capture_timestamp_ms,
rtc::ArrayView<const uint32_t> csrcs) {
return SendFrame(frameType, payloadType, rtp_timestamp, payload, return SendFrame(frameType, payloadType, rtp_timestamp, payload,
absolute_capture_timestamp_ms); absolute_capture_timestamp_ms, csrcs);
}; };
} }
}; };
std::unique_ptr<MockTransformableAudioFrame> CreateMockReceiverFrame() { std::unique_ptr<TransformableAudioFrameInterface> CreateMockReceiverFrame(
const uint8_t mock_data[] = {1, 2, 3, 4}; std::vector<const uint32_t> csrcs) {
std::unique_ptr<MockTransformableAudioFrame> mock_frame = std::unique_ptr<MockTransformableAudioFrame> mock_frame =
std::make_unique<MockTransformableAudioFrame>(); std::make_unique<NiceMock<MockTransformableAudioFrame>>();
rtc::ArrayView<const uint8_t> payload(mock_data); rtc::ArrayView<const uint8_t> payload(mock_data);
ON_CALL(*mock_frame, GetData).WillByDefault(Return(payload)); ON_CALL(*mock_frame, GetData).WillByDefault(Return(payload));
ON_CALL(*mock_frame, GetPayloadType).WillByDefault(Return(0)); ON_CALL(*mock_frame, GetPayloadType).WillByDefault(Return(0));
ON_CALL(*mock_frame, GetDirection) ON_CALL(*mock_frame, GetDirection)
.WillByDefault(Return(TransformableFrameInterface::Direction::kReceiver)); .WillByDefault(Return(TransformableFrameInterface::Direction::kReceiver));
ON_CALL(*mock_frame, GetContributingSources).WillByDefault(Return(csrcs));
return mock_frame; return mock_frame;
} }
std::unique_ptr<TransformableAudioFrameInterface> CreateFrame() {
TaskQueueForTest channel_queue("channel_queue");
rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer =
rtc::make_ref_counted<NiceMock<MockFrameTransformer>>();
MockChannelSend mock_channel;
rtc::scoped_refptr<ChannelSendFrameTransformerDelegate> delegate =
rtc::make_ref_counted<ChannelSendFrameTransformerDelegate>(
mock_channel.callback(), mock_frame_transformer, &channel_queue);
std::unique_ptr<TransformableFrameInterface> frame;
ON_CALL(*mock_frame_transformer, Transform)
.WillByDefault(
[&frame](
std::unique_ptr<TransformableFrameInterface> transform_frame) {
frame = std::move(transform_frame);
});
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, mock_data,
sizeof(mock_data), 0,
/*ssrc=*/0, /*mimeType=*/"audio/opus");
return absl::WrapUnique(
static_cast<webrtc::TransformableAudioFrameInterface*>(frame.release()));
}
// Test that the delegate registers itself with the frame transformer on Init(). // Test that the delegate registers itself with the frame transformer on Init().
TEST(ChannelSendFrameTransformerDelegateTest, TEST(ChannelSendFrameTransformerDelegateTest,
RegisterTransformedFrameCallbackOnInit) { RegisterTransformedFrameCallbackOnInit) {
@ -136,15 +167,16 @@ TEST(ChannelSendFrameTransformerDelegateTest,
delegate->Init(); delegate->Init();
ASSERT_TRUE(callback); ASSERT_TRUE(callback);
const uint8_t data[] = {1, 2, 3, 4}; std::vector<const uint32_t> csrcs = {123, 234, 345, 456};
EXPECT_CALL(mock_channel, SendFrame).Times(0); EXPECT_CALL(mock_channel, SendFrame).Times(0);
EXPECT_CALL(mock_channel, SendFrame(_, 0, 0, ElementsAre(1, 2, 3, 4), _)); EXPECT_CALL(mock_channel, SendFrame(_, 0, 0, ElementsAreArray(mock_data), _,
ElementsAreArray(csrcs)));
ON_CALL(*mock_frame_transformer, Transform) ON_CALL(*mock_frame_transformer, Transform)
.WillByDefault( .WillByDefault([&](std::unique_ptr<TransformableFrameInterface> frame) {
[&callback](std::unique_ptr<TransformableFrameInterface> frame) { callback->OnTransformedFrame(CreateMockReceiverFrame(csrcs));
callback->OnTransformedFrame(CreateMockReceiverFrame());
}); });
delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, data, sizeof(data), 0, delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, mock_data,
sizeof(mock_data), 0,
/*ssrc=*/0, /*mimeType=*/"audio/opus"); /*ssrc=*/0, /*mimeType=*/"audio/opus");
channel_queue.WaitForPreviouslyPostedTasks(); channel_queue.WaitForPreviouslyPostedTasks();
} }
@ -188,5 +220,39 @@ TEST(ChannelSendFrameTransformerDelegateTest, ShortCircuitingSkipsTransform) {
/*ssrc=*/0, /*mimeType=*/"audio/opus"); /*ssrc=*/0, /*mimeType=*/"audio/opus");
} }
TEST(ChannelSendFrameTransformerDelegateTest,
CloningSenderFramePreservesInformation) {
std::unique_ptr<TransformableAudioFrameInterface> frame = CreateFrame();
std::unique_ptr<TransformableAudioFrameInterface> cloned_frame =
CloneSenderAudioFrame(frame.get());
EXPECT_EQ(cloned_frame->GetTimestamp(), frame->GetTimestamp());
EXPECT_EQ(cloned_frame->GetSsrc(), frame->GetSsrc());
EXPECT_EQ(cloned_frame->Type(), frame->Type());
EXPECT_EQ(cloned_frame->GetPayloadType(), frame->GetPayloadType());
EXPECT_EQ(cloned_frame->GetMimeType(), frame->GetMimeType());
EXPECT_THAT(cloned_frame->GetContributingSources(),
ElementsAreArray(frame->GetContributingSources()));
}
TEST(ChannelSendFrameTransformerDelegateTest, CloningReceiverFrameWithCsrcs) {
std::unique_ptr<TransformableAudioFrameInterface> frame =
CreateMockReceiverFrame(/*csrcs=*/{123, 234, 345});
std::unique_ptr<TransformableAudioFrameInterface> cloned_frame =
CloneSenderAudioFrame(frame.get());
EXPECT_EQ(cloned_frame->GetTimestamp(), frame->GetTimestamp());
EXPECT_EQ(cloned_frame->GetSsrc(), frame->GetSsrc());
EXPECT_EQ(cloned_frame->Type(), frame->Type());
EXPECT_EQ(cloned_frame->GetPayloadType(), frame->GetPayloadType());
EXPECT_EQ(cloned_frame->GetMimeType(), frame->GetMimeType());
EXPECT_EQ(cloned_frame->AbsoluteCaptureTimestamp(),
frame->AbsoluteCaptureTimestamp());
ASSERT_NE(frame->GetContributingSources().size(), 0u);
EXPECT_THAT(cloned_frame->GetContributingSources(),
ElementsAreArray(frame->GetContributingSources()));
}
} // namespace } // namespace
} // namespace webrtc } // namespace webrtc

View File

@ -254,7 +254,8 @@ bool RTPSenderAudio::SendAudio(const RtpAudioFrame& frame) {
return false; return false;
} }
std::unique_ptr<RtpPacketToSend> packet = rtp_sender_->AllocatePacket(); std::unique_ptr<RtpPacketToSend> packet =
rtp_sender_->AllocatePacket(frame.csrcs);
packet->SetMarker(MarkerBit(frame.type, frame.payload_id)); packet->SetMarker(MarkerBit(frame.type, frame.payload_id));
packet->SetPayloadType(frame.payload_id); packet->SetPayloadType(frame.payload_id);
packet->SetTimestamp(frame.rtp_timestamp); packet->SetTimestamp(frame.rtp_timestamp);

View File

@ -61,6 +61,9 @@ class RTPSenderAudio {
// header-extension-for-audio-level-indication. // header-extension-for-audio-level-indication.
// Valid range is [0,127]. Actual value is negative. // Valid range is [0,127]. Actual value is negative.
absl::optional<int> audio_level_dbov; absl::optional<int> audio_level_dbov;
// Contributing sources list.
rtc::ArrayView<const uint32_t> csrcs;
}; };
bool SendAudio(const RtpAudioFrame& frame); bool SendAudio(const RtpAudioFrame& frame);

View File

@ -222,4 +222,19 @@ TEST_F(RtpSenderAudioTest, CheckMarkerBitForTelephoneEvents) {
EXPECT_FALSE(transport_.last_sent_packet().Marker()); EXPECT_FALSE(transport_.last_sent_packet().Marker());
} }
TEST_F(RtpSenderAudioTest, SendsCsrcs) {
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};
std::vector<uint32_t> csrcs({123, 456, 789});
ASSERT_TRUE(rtp_sender_audio_->SendAudio(
{.payload = payload, .payload_id = payload_type, .csrcs = csrcs}));
EXPECT_EQ(transport_.last_sent_packet().Csrcs(), csrcs);
}
} // namespace webrtc } // namespace webrtc