Remove DCHECK on setting audio rcvr encoded transform twice
Failing a DCHECK on a ChannelReceiver having its encoded transform set
more than once contradicts the comment above - this can happen when
reconfiguring a channel, eg as in the web platform test
webrtc/recvonly-transceiver-can-become-sendrecv.https.html.
It was added after the original code by a different author, and indeed
the video side doesn't have such a check.
Bug: chromium:1502781
Change-Id: Id36e52601da34ebc194ff058e4672046379f576e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328560
Commit-Queue: Tony Herre <herre@google.com>
Auto-Submit: Tony Herre <herre@google.com>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41246}
diff --git a/audio/BUILD.gn b/audio/BUILD.gn
index ec09e5a..bbc294b 100644
--- a/audio/BUILD.gn
+++ b/audio/BUILD.gn
@@ -241,6 +241,7 @@
"../rtc_base:logging",
"../rtc_base:threading",
"../test:audio_codec_mocks",
+ "../test:mock_frame_transformer",
"../test:mock_transport",
"../test:test_support",
"../test/time_controller",
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index efd9668..8367b00 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -925,12 +925,19 @@
void ChannelReceive::SetDepacketizerToDecoderFrameTransformer(
rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- // Depending on when the channel is created, the transformer might be set
- // twice. Don't replace the delegate if it was already initialized.
- if (!frame_transformer || frame_transformer_delegate_) {
+ if (!frame_transformer) {
RTC_DCHECK_NOTREACHED() << "Not setting the transformer?";
return;
}
+ if (frame_transformer_delegate_) {
+ // Depending on when the channel is created, the transformer might be set
+ // twice. Don't replace the delegate if it was already initialized.
+ // TODO(crbug.com/webrtc/15674): Prevent multiple calls during
+ // reconfiguration.
+ RTC_CHECK_EQ(frame_transformer_delegate_->FrameTransformer(),
+ frame_transformer);
+ return;
+ }
InitFrameTransformerDelegate(std::move(frame_transformer));
}
diff --git a/audio/channel_receive_frame_transformer_delegate.cc b/audio/channel_receive_frame_transformer_delegate.cc
index 3cad530..8a7dda8 100644
--- a/audio/channel_receive_frame_transformer_delegate.cc
+++ b/audio/channel_receive_frame_transformer_delegate.cc
@@ -157,4 +157,11 @@
// originally from this receiver.
receive_frame_callback_(frame->GetData(), header);
}
+
+rtc::scoped_refptr<FrameTransformerInterface>
+ChannelReceiveFrameTransformerDelegate::FrameTransformer() {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ return frame_transformer_;
+}
+
} // namespace webrtc
diff --git a/audio/channel_receive_frame_transformer_delegate.h b/audio/channel_receive_frame_transformer_delegate.h
index ac7cef0..37ff75c 100644
--- a/audio/channel_receive_frame_transformer_delegate.h
+++ b/audio/channel_receive_frame_transformer_delegate.h
@@ -62,6 +62,8 @@
// `channel_receive_thread_`, by calling `receive_frame_callback_`.
void ReceiveFrame(std::unique_ptr<TransformableFrameInterface> frame) const;
+ rtc::scoped_refptr<FrameTransformerInterface> FrameTransformer();
+
protected:
~ChannelReceiveFrameTransformerDelegate() override = default;
diff --git a/audio/channel_receive_unittest.cc b/audio/channel_receive_unittest.cc
index 4b7b7c0..aab8a95 100644
--- a/audio/channel_receive_unittest.cc
+++ b/audio/channel_receive_unittest.cc
@@ -29,6 +29,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/mock_audio_decoder_factory.h"
+#include "test/mock_frame_transformer.h"
#include "test/mock_transport.h"
#include "test/time_controller/simulated_time_controller.h"
@@ -226,6 +227,41 @@
EXPECT_NE(ProbeCaptureStartNtpTime(*channel), -1);
}
+TEST_F(ChannelReceiveTest, SettingFrameTransformer) {
+ auto channel = CreateTestChannelReceive();
+
+ rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer =
+ rtc::make_ref_counted<MockFrameTransformer>();
+
+ EXPECT_CALL(*mock_frame_transformer, RegisterTransformedFrameCallback);
+ channel->SetDepacketizerToDecoderFrameTransformer(mock_frame_transformer);
+
+ // Must start playout, otherwise packet is discarded.
+ channel->StartPlayout();
+
+ RtpPacketReceived packet = CreateRtpPacket();
+
+ // Receive one RTP packet, this should be transformed.
+ EXPECT_CALL(*mock_frame_transformer, Transform);
+ channel->OnRtpPacket(packet);
+}
+
+TEST_F(ChannelReceiveTest, SettingFrameTransformerMultipleTimes) {
+ auto channel = CreateTestChannelReceive();
+
+ rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer =
+ rtc::make_ref_counted<MockFrameTransformer>();
+
+ EXPECT_CALL(*mock_frame_transformer, RegisterTransformedFrameCallback);
+ channel->SetDepacketizerToDecoderFrameTransformer(mock_frame_transformer);
+
+ // Set the same transformer again, shouldn't cause any additional callback
+ // registration calls.
+ EXPECT_CALL(*mock_frame_transformer, RegisterTransformedFrameCallback)
+ .Times(0);
+ channel->SetDepacketizerToDecoderFrameTransformer(mock_frame_transformer);
+}
+
} // namespace
} // namespace voe
} // namespace webrtc
