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}
diff --git a/audio/BUILD.gn b/audio/BUILD.gn index bbc294b..09562b9 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn
@@ -223,6 +223,7 @@ "utility:utility_tests", "//testing/gtest", ] + absl_deps = [ "//third_party/abseil-cpp/absl/memory" ] } rtc_library("channel_receive_unittest") {
diff --git a/audio/channel_send.cc b/audio/channel_send.cc index b71f564..ecf2cb5 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc
@@ -169,7 +169,8 @@ uint8_t payloadType, uint32_t rtp_timestamp_without_offset, 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_); void OnReceivedRtt(int64_t rtt_ms); @@ -294,14 +295,15 @@ return 0; } return SendRtpAudio(frameType, payloadType, rtp_timestamp, payload, - absolute_capture_timestamp_ms); + absolute_capture_timestamp_ms, /*csrcs=*/{}); } int32_t ChannelSend::SendRtpAudio(AudioFrameType frameType, uint8_t payloadType, uint32_t rtp_timestamp_without_offset, 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). // Keep this buffer around for the lifetime of the send call. rtc::Buffer encrypted_audio_payload; @@ -363,7 +365,8 @@ .payload = payload, .payload_id = payloadType, .rtp_timestamp = - rtp_timestamp_without_offset + rtp_rtcp_->StartTimestamp()}; + rtp_timestamp_without_offset + rtp_rtcp_->StartTimestamp(), + .csrcs = csrcs}; if (absolute_capture_timestamp_ms > 0) { frame.capture_time = Timestamp::Millis(absolute_capture_timestamp_ms); } @@ -859,12 +862,13 @@ [this](AudioFrameType frameType, uint8_t payloadType, uint32_t rtp_timestamp_with_offset, 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_); return SendRtpAudio( frameType, payloadType, rtp_timestamp_with_offset - rtp_rtcp_->StartTimestamp(), payload, - absolute_capture_timestamp_ms); + absolute_capture_timestamp_ms, csrcs); }; frame_transformer_delegate_ = rtc::make_ref_counted<ChannelSendFrameTransformerDelegate>(
diff --git a/audio/channel_send_frame_transformer_delegate.cc b/audio/channel_send_frame_transformer_delegate.cc index f66f29d..2eea0d2 100644 --- a/audio/channel_send_frame_transformer_delegate.cc +++ b/audio/channel_send_frame_transformer_delegate.cc
@@ -11,6 +11,7 @@ #include "audio/channel_send_frame_transformer_delegate.h" #include <utility> +#include <vector> namespace webrtc { namespace { @@ -56,6 +57,7 @@ size_t payload_size, absl::optional<uint64_t> absolute_capture_timestamp_ms, uint32_t ssrc, + std::vector<uint32_t> csrcs, const std::string& codec_mime_type) : frame_type_(frame_type), payload_type_(payload_type), @@ -63,6 +65,7 @@ payload_(payload_data, payload_size), absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms), ssrc_(ssrc), + csrcs_(std::move(csrcs)), codec_mime_type_(codec_mime_type) {} ~TransformableOutgoingAudioFrame() override = default; rtc::ArrayView<const uint8_t> GetData() const override { return payload_; } @@ -81,7 +84,7 @@ std::string GetMimeType() const override { return codec_mime_type_; } rtc::ArrayView<const uint32_t> GetContributingSources() const override { - return {}; + return csrcs_; } const absl::optional<uint16_t> SequenceNumber() const override { @@ -103,6 +106,7 @@ rtc::Buffer payload_; absl::optional<uint64_t> absolute_capture_timestamp_ms_; uint32_t ssrc_; + std::vector<uint32_t> csrcs_; std::string codec_mime_type_; }; } // namespace @@ -143,14 +147,15 @@ send_frame_callback_( frame_type, payload_type, rtp_timestamp, rtc::ArrayView<const uint8_t>(payload_data, payload_size), - absolute_capture_timestamp_ms); + absolute_capture_timestamp_ms, /*csrcs=*/{}); return; } } frame_transformer_->Transform( std::make_unique<TransformableOutgoingAudioFrame>( 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( @@ -184,17 +189,21 @@ transformed_frame->GetData(), transformed_frame->AbsoluteCaptureTimestamp() ? *transformed_frame->AbsoluteCaptureTimestamp() - : 0); + : 0, + transformed_frame->GetContributingSources()); } std::unique_ptr<TransformableAudioFrameInterface> CloneSenderAudioFrame( TransformableAudioFrameInterface* original) { + std::vector<uint32_t> csrcs; + csrcs.assign(original->GetContributingSources().begin(), + original->GetContributingSources().end()); return std::make_unique<TransformableOutgoingAudioFrame>( InterfaceFrameTypeToInternalFrameType(original->Type()), original->GetPayloadType(), original->GetTimestamp(), original->GetData().data(), original->GetData().size(), original->AbsoluteCaptureTimestamp(), original->GetSsrc(), - original->GetMimeType()); + std::move(csrcs), original->GetMimeType()); } } // namespace webrtc
diff --git a/audio/channel_send_frame_transformer_delegate.h b/audio/channel_send_frame_transformer_delegate.h index bcec018..97fc14f 100644 --- a/audio/channel_send_frame_transformer_delegate.h +++ b/audio/channel_send_frame_transformer_delegate.h
@@ -35,7 +35,8 @@ uint8_t payloadType, uint32_t rtp_timestamp_with_offset, 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 send_frame_callback, rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
diff --git a/audio/channel_send_frame_transformer_delegate_unittest.cc b/audio/channel_send_frame_transformer_delegate_unittest.cc index a47cbd5..4dcd15c 100644 --- a/audio/channel_send_frame_transformer_delegate_unittest.cc +++ b/audio/channel_send_frame_transformer_delegate_unittest.cc
@@ -12,7 +12,9 @@ #include <memory> #include <utility> +#include <vector> +#include "absl/memory/memory.h" #include "rtc_base/task_queue_for_test.h" #include "test/gmock.h" #include "test/gtest.h" @@ -24,10 +26,13 @@ using ::testing::_; using ::testing::ElementsAre; +using ::testing::ElementsAreArray; using ::testing::NiceMock; using ::testing::Return; using ::testing::SaveArg; +const uint8_t mock_data[] = {1, 2, 3, 4}; + class MockChannelSend { public: MockChannelSend() = default; @@ -39,30 +44,56 @@ uint8_t payloadType, 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)); ChannelSendFrameTransformerDelegate::SendFrameCallback callback() { return [this](AudioFrameType frameType, uint8_t payloadType, 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, - absolute_capture_timestamp_ms); + absolute_capture_timestamp_ms, csrcs); }; } }; -std::unique_ptr<MockTransformableAudioFrame> CreateMockReceiverFrame() { - const uint8_t mock_data[] = {1, 2, 3, 4}; +std::unique_ptr<TransformableAudioFrameInterface> CreateMockReceiverFrame( + std::vector<const uint32_t> csrcs) { std::unique_ptr<MockTransformableAudioFrame> mock_frame = - std::make_unique<MockTransformableAudioFrame>(); + std::make_unique<NiceMock<MockTransformableAudioFrame>>(); rtc::ArrayView<const uint8_t> payload(mock_data); ON_CALL(*mock_frame, GetData).WillByDefault(Return(payload)); ON_CALL(*mock_frame, GetPayloadType).WillByDefault(Return(0)); ON_CALL(*mock_frame, GetDirection) .WillByDefault(Return(TransformableFrameInterface::Direction::kReceiver)); + ON_CALL(*mock_frame, GetContributingSources).WillByDefault(Return(csrcs)); 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(ChannelSendFrameTransformerDelegateTest, RegisterTransformedFrameCallbackOnInit) { @@ -136,15 +167,16 @@ delegate->Init(); 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(_, 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) - .WillByDefault( - [&callback](std::unique_ptr<TransformableFrameInterface> frame) { - callback->OnTransformedFrame(CreateMockReceiverFrame()); - }); - delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, data, sizeof(data), 0, + .WillByDefault([&](std::unique_ptr<TransformableFrameInterface> frame) { + callback->OnTransformedFrame(CreateMockReceiverFrame(csrcs)); + }); + delegate->Transform(AudioFrameType::kEmptyFrame, 0, 0, mock_data, + sizeof(mock_data), 0, /*ssrc=*/0, /*mimeType=*/"audio/opus"); channel_queue.WaitForPreviouslyPostedTasks(); } @@ -188,5 +220,39 @@ /*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 webrtc
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc index b826c30..9d2258d 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc
@@ -254,7 +254,8 @@ 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->SetPayloadType(frame.payload_id); packet->SetTimestamp(frame.rtp_timestamp);
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.h b/modules/rtp_rtcp/source/rtp_sender_audio.h index 662f908..83a2cb2 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.h +++ b/modules/rtp_rtcp/source/rtp_sender_audio.h
@@ -61,6 +61,9 @@ // header-extension-for-audio-level-indication. // Valid range is [0,127]. Actual value is negative. absl::optional<int> audio_level_dbov; + + // Contributing sources list. + rtc::ArrayView<const uint32_t> csrcs; }; bool SendAudio(const RtpAudioFrame& frame);
diff --git a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc index 0db610c..724cd3a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc
@@ -222,4 +222,19 @@ 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