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}
This commit is contained in:
Tony Herre 2023-11-27 10:37:06 +01:00 committed by WebRTC LUCI CQ
parent 264547d084
commit f921d25320
5 changed files with 56 additions and 3 deletions

View File

@ -241,6 +241,7 @@ if (rtc_include_tests) {
"../rtc_base:logging", "../rtc_base:logging",
"../rtc_base:threading", "../rtc_base:threading",
"../test:audio_codec_mocks", "../test:audio_codec_mocks",
"../test:mock_frame_transformer",
"../test:mock_transport", "../test:mock_transport",
"../test:test_support", "../test:test_support",
"../test/time_controller", "../test/time_controller",

View File

@ -925,12 +925,19 @@ void ChannelReceive::SetAssociatedSendChannel(
void ChannelReceive::SetDepacketizerToDecoderFrameTransformer( void ChannelReceive::SetDepacketizerToDecoderFrameTransformer(
rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer) { rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_); RTC_DCHECK_RUN_ON(&worker_thread_checker_);
// Depending on when the channel is created, the transformer might be set if (!frame_transformer) {
// twice. Don't replace the delegate if it was already initialized.
if (!frame_transformer || frame_transformer_delegate_) {
RTC_DCHECK_NOTREACHED() << "Not setting the transformer?"; RTC_DCHECK_NOTREACHED() << "Not setting the transformer?";
return; 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)); InitFrameTransformerDelegate(std::move(frame_transformer));
} }

View File

@ -157,4 +157,11 @@ void ChannelReceiveFrameTransformerDelegate::ReceiveFrame(
// originally from this receiver. // originally from this receiver.
receive_frame_callback_(frame->GetData(), header); receive_frame_callback_(frame->GetData(), header);
} }
rtc::scoped_refptr<FrameTransformerInterface>
ChannelReceiveFrameTransformerDelegate::FrameTransformer() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
return frame_transformer_;
}
} // namespace webrtc } // namespace webrtc

View File

@ -62,6 +62,8 @@ class ChannelReceiveFrameTransformerDelegate : public TransformedFrameCallback {
// `channel_receive_thread_`, by calling `receive_frame_callback_`. // `channel_receive_thread_`, by calling `receive_frame_callback_`.
void ReceiveFrame(std::unique_ptr<TransformableFrameInterface> frame) const; void ReceiveFrame(std::unique_ptr<TransformableFrameInterface> frame) const;
rtc::scoped_refptr<FrameTransformerInterface> FrameTransformer();
protected: protected:
~ChannelReceiveFrameTransformerDelegate() override = default; ~ChannelReceiveFrameTransformerDelegate() override = default;

View File

@ -29,6 +29,7 @@
#include "test/gmock.h" #include "test/gmock.h"
#include "test/gtest.h" #include "test/gtest.h"
#include "test/mock_audio_decoder_factory.h" #include "test/mock_audio_decoder_factory.h"
#include "test/mock_frame_transformer.h"
#include "test/mock_transport.h" #include "test/mock_transport.h"
#include "test/time_controller/simulated_time_controller.h" #include "test/time_controller/simulated_time_controller.h"
@ -226,6 +227,41 @@ TEST_F(ChannelReceiveTest, CaptureStartTimeBecomesValid) {
EXPECT_NE(ProbeCaptureStartNtpTime(*channel), -1); 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
} // namespace voe } // namespace voe
} // namespace webrtc } // namespace webrtc