From 4ad141e69b8e6a1650450e97b323a159883e5141 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Fri, 2 Jun 2023 09:59:45 +0000 Subject: [PATCH] Add callback for send codec in audio too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It turns out there's a similar linkage as the one for video. Tests are coming in https://webrtc-review.googlesource.com/c/src/+/307461 Bug: webrtc:13931 Change-Id: I638d1a1907116a71481aa88dce932492323ae5b7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/307463 Commit-Queue: Harald Alvestrand Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#40206} --- media/base/fake_media_engine.h | 3 +++ media/base/media_channel.h | 9 +++++---- media/base/media_channel_impl.h | 7 +++++++ media/engine/webrtc_voice_engine.cc | 4 +++- media/engine/webrtc_voice_engine.h | 9 +++++++++ pc/channel.cc | 16 +++++++++++++++- pc/channel.h | 1 + pc/test/mock_voice_media_channel.h | 5 +++++ 8 files changed, 48 insertions(+), 6 deletions(-) diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index b00de20523..74128cbe30 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -420,6 +420,9 @@ class FakeVoiceMediaChannel : public RtpHelper { bool SenderNonSenderRttEnabled() const override { return false; } void SetReceiveNackEnabled(bool enabled) {} void SetReceiveNonSenderRttEnabled(bool enabled) {} + bool SendCodecHasNack() const override { return false; } + void SetSendCodecChangedCallback( + absl::AnyInvocable callback) override {} private: class VoiceChannelAudioSink : public AudioSource::Sink { diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 9749cd0568..e982284785 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -247,9 +247,14 @@ class MediaSendChannelInterface { webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { } virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0; + virtual bool SendCodecHasNack() const = 0; // Called whenever the list of sending SSRCs changes. virtual void SetSsrcListChangedCallback( absl::AnyInvocable&)> callback) = 0; + // TODO(bugs.webrtc.org/13931): Remove when configuration is more sensible + virtual void SetSendCodecChangedCallback( + absl::AnyInvocable callback) = 0; + // Get the underlying send/receive implementation channel for testing. // TODO(bugs.webrtc.org/13931): Remove method and the fakes that depend on it. virtual MediaChannel* ImplForTesting() = 0; @@ -952,11 +957,7 @@ class VideoMediaSendChannelInterface : public MediaSendChannelInterface { // Information queries to support SetReceiverFeedbackParameters virtual webrtc::RtcpMode SendCodecRtcpMode() const = 0; virtual bool SendCodecHasLntf() const = 0; - virtual bool SendCodecHasNack() const = 0; virtual absl::optional SendCodecRtxTime() const = 0; - // TODO(bugs.webrtc.org/13931): Remove when configuration is more sensible - virtual void SetSendCodecChangedCallback( - absl::AnyInvocable callback) = 0; }; class VideoMediaReceiveChannelInterface : public MediaReceiveChannelInterface { diff --git a/media/base/media_channel_impl.h b/media/base/media_channel_impl.h index dfd9fb3ec7..d4d8182ddb 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -206,6 +206,8 @@ class MediaChannel : public MediaChannelUtil, void OnNetworkRouteChanged(absl::string_view transport_name, const rtc::NetworkRoute& network_route) override = 0; + void SetSendCodecChangedCallback( + absl::AnyInvocable callback) override = 0; // Methods from the APIs that are implemented in MediaChannelUtil using MediaChannelUtil::ExtmapAllowMixed; @@ -469,6 +471,11 @@ class VoiceMediaSendChannel : public VoiceMediaSendChannelInterface { bool SenderNonSenderRttEnabled() const override { return impl_->SenderNonSenderRttEnabled(); } + bool SendCodecHasNack() const override { return impl()->SendCodecHasNack(); } + void SetSendCodecChangedCallback( + absl::AnyInvocable callback) override { + impl()->SetSendCodecChangedCallback(std::move(callback)); + } MediaChannel* ImplForTesting() override { return impl_; } private: diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index a763ddb107..1b240d67d2 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1761,15 +1761,17 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } call_->GetTransportControllerSend()->SetSdpBitrateParameters(bitrate_config); + send_codecs_ = codecs; // In legacy kBoth mode, the MediaChannel sets the NACK status. // In other modes, this is done externally. if (role() == MediaChannel::Role::kBoth) { SetReceiveNackEnabled(send_codec_spec_->nack_enabled); SetReceiveNonSenderRttEnabled(send_codec_spec_->enable_non_sender_rtt); + } else if (send_codec_changed_callback_) { + send_codec_changed_callback_(); } - send_codecs_ = codecs; return true; } diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index e73c24c5f5..474acc4e4a 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "absl/functional/any_invocable.h" @@ -304,9 +305,14 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, } return send_codec_spec_->enable_non_sender_rtt; } + bool SendCodecHasNack() const override { return SenderNackEnabled(); } void SetReceiveNackEnabled(bool enabled) override; void SetReceiveNonSenderRttEnabled(bool enabled) override; + void SetSendCodecChangedCallback( + absl::AnyInvocable callback) override { + send_codec_changed_callback_ = std::move(callback); + } private: bool SetOptions(const AudioOptions& options); @@ -404,6 +410,9 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, void FillSendCodecStats(VoiceMediaSendInfo* voice_media_info); void FillReceiveCodecStats(VoiceMediaReceiveInfo* voice_media_info); + // Callback invoked whenever the send codec changes. + // TODO(bugs.webrtc.org/13931): Remove again when coupling isn't needed. + absl::AnyInvocable send_codec_changed_callback_; // Callback invoked whenever the list of SSRCs changes. absl::AnyInvocable&)> ssrc_list_changed_callback_; diff --git a/pc/channel.cc b/pc/channel.cc index bf9827869a..f9ee3ff068 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -886,7 +886,9 @@ VoiceChannel::VoiceChannel( crypto_options, ssrc_generator), send_channel_(media_channel_impl_->AsVoiceChannel()), - receive_channel_(media_channel_impl_->AsVoiceChannel()) {} + receive_channel_(media_channel_impl_->AsVoiceChannel()) { + InitCallback(); +} VoiceChannel::~VoiceChannel() { TRACE_EVENT0("webrtc", "VoiceChannel::~VoiceChannel"); @@ -894,6 +896,18 @@ VoiceChannel::~VoiceChannel() { DisableMedia_w(); } +void VoiceChannel::InitCallback() { + RTC_DCHECK_RUN_ON(worker_thread()); + // TODO(bugs.webrtc.org/13931): Remove when values are set + // in a more sensible fashion + send_channel_.SetSendCodecChangedCallback([this]() { + RTC_DCHECK_RUN_ON(worker_thread()); + // Adjust receive streams based on send codec. + receive_channel_.SetReceiveNackEnabled(send_channel_.SendCodecHasNack()); + receive_channel_.SetReceiveNonSenderRttEnabled( + send_channel_.SenderNonSenderRttEnabled()); + }); +} void VoiceChannel::UpdateMediaSendRecvState_w() { // Render incoming data if we're the active call, and we have the local // content. We receive data on the default channel and multiplexed streams. diff --git a/pc/channel.h b/pc/channel.h index a84a8427a3..89af8902e1 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -435,6 +435,7 @@ class VoiceChannel : public BaseChannel { } private: + void InitCallback(); // overrides from BaseChannel void UpdateMediaSendRecvState_w() RTC_RUN_ON(worker_thread()) override; bool SetLocalContent_w(const MediaContentDescription* content, diff --git a/pc/test/mock_voice_media_channel.h b/pc/test/mock_voice_media_channel.h index 8748532205..e3eb42d157 100644 --- a/pc/test/mock_voice_media_channel.h +++ b/pc/test/mock_voice_media_channel.h @@ -67,10 +67,15 @@ class MockVoiceMediaChannel : public VoiceMediaChannel { ChooseReceiverReportSsrc, (const std::set&), (override)); + MOCK_METHOD(bool, SendCodecHasNack, (), (const, override)); MOCK_METHOD(void, SetSsrcListChangedCallback, (absl::AnyInvocable&)>), (override)); + MOCK_METHOD(void, + SetSendCodecChangedCallback, + (absl::AnyInvocable), + (override)); MOCK_METHOD(void, OnDemuxerCriteriaUpdatePending, (), (override)); MOCK_METHOD(void, OnDemuxerCriteriaUpdateComplete, (), (override)); MOCK_METHOD(int, GetRtpSendTimeExtnId, (), (const, override));