Improve WebRTC build scripts and add external_audio_source patch #1053
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| libwebrtc: patch | ||
| livekit: patch | ||
| livekit-ffi: patch | ||
| webrtc-sys: patch | ||
| --- | ||
|
|
||
| Improve WebRTC build scripts and add external_audio_source patch - #1053 (@xianshijing-lk) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| diff --git a/api/media_stream_interface.h b/api/media_stream_interface.h | ||
| index fb1cc4e58e..85062ba60e 100644 | ||
| --- a/api/media_stream_interface.h | ||
| +++ b/api/media_stream_interface.h | ||
| @@ -267,6 +267,11 @@ class RTC_EXPORT AudioSourceInterface : public MediaSourceInterface { | ||
| // (for some of the settings this approach is broken, e.g. setting | ||
| // audio network adaptation on the source is the wrong layer of abstraction). | ||
| virtual const AudioOptions options() const; | ||
| + | ||
| + // Returns true if this source delivers audio externally (via AddSink), | ||
| + // bypassing the ADM/AudioState audio distribution path. | ||
| + // When true, AudioSendStream should not register with AudioState. | ||
| + virtual bool is_external_source() const { return false; } | ||
| }; | ||
|
|
||
| // Interface of the audio processor used by the audio track to collect | ||
| diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc | ||
| index 76156ce830..10b59d3ff6 100644 | ||
| --- a/audio/audio_send_stream.cc | ||
| +++ b/audio/audio_send_stream.cc | ||
| @@ -373,8 +373,13 @@ void AudioSendStream::Start() { | ||
| } | ||
| channel_send_->StartSend(); | ||
| sending_ = true; | ||
| - audio_state()->AddSendingStream(this, encoder_sample_rate_hz_, | ||
| - encoder_num_channels_); | ||
| + // Only register with AudioState if not using external source. | ||
| + // External sources (like NativeAudioSource) deliver audio directly via AddSink, | ||
| + // so we don't want AudioState to also send device audio to this stream. | ||
| + if (!config_.external_source) { | ||
| + audio_state()->AddSendingStream(this, encoder_sample_rate_hz_, | ||
| + encoder_num_channels_); | ||
| + } | ||
| } | ||
|
|
||
| void AudioSendStream::Stop() { | ||
| @@ -386,7 +391,10 @@ void AudioSendStream::Stop() { | ||
| RemoveBitrateObserver(); | ||
| channel_send_->StopSend(); | ||
| sending_ = false; | ||
| - audio_state()->RemoveSendingStream(this); | ||
| + // Only unregister if we registered (when not using external source). | ||
| + if (!config_.external_source) { | ||
| + audio_state()->RemoveSendingStream(this); | ||
| + } | ||
| } | ||
|
|
||
| void AudioSendStream::SendAudioData(std::unique_ptr<AudioFrame> audio_frame) { | ||
| diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h | ||
| index 84341b5cb1..9359777bc9 100644 | ||
| --- a/call/audio_send_stream.h | ||
| +++ b/call/audio_send_stream.h | ||
| @@ -178,6 +178,12 @@ class AudioSendStream : public AudioSender { | ||
| // An optional frame transformer used by insertable streams to transform | ||
| // encoded frames. | ||
| scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer; | ||
| + | ||
| + // When true, this stream uses an external audio source (not ADM). | ||
| + // AudioState will NOT send device-captured audio to this stream. | ||
| + // Audio is delivered directly via the source's AddSink mechanism. | ||
| + // This prevents mixing of device audio with externally-sourced audio. | ||
| + bool external_source = false; | ||
| }; | ||
|
|
||
| virtual ~AudioSendStream() = default; | ||
| diff --git a/media/base/audio_source.h b/media/base/audio_source.h | ||
| index 04a7d19dfa..9f513f3c75 100644 | ||
| --- a/media/base/audio_source.h | ||
| +++ b/media/base/audio_source.h | ||
| @@ -49,6 +49,10 @@ class AudioSource { | ||
| // to the source at a time. | ||
| virtual void SetSink(Sink* sink) = 0; | ||
|
|
||
| + // Returns true if this source delivers audio externally (bypassing ADM). | ||
| + // When true, AudioSendStream should not register with AudioState. | ||
| + virtual bool is_external_source() const { return false; } | ||
| + | ||
| protected: | ||
| virtual ~AudioSource() {} | ||
| }; | ||
| diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc | ||
| index 762f9d584c..4ce07ddc9d 100644 | ||
| --- a/media/engine/webrtc_voice_engine.cc | ||
| +++ b/media/engine/webrtc_voice_engine.cc | ||
| @@ -1017,6 +1017,14 @@ class WebRtcVoiceSendChannel::WebRtcAudioSendStream : public AudioSource::Sink { | ||
| RTC_DCHECK(source_ == source); | ||
| return; | ||
| } | ||
| + | ||
| + // Check if this is an external audio source (delivers audio via AddSink). | ||
| + // If so, mark the config so AudioState doesn't send device audio to this | ||
| + // stream. This must be done before UpdateSendState() calls Start(). | ||
| + if (source->is_external_source() && !config_.external_source) { | ||
|
Contributor
There was a problem hiding this comment. QQ: This method ( In another Rust PR, is_external_source is overridden to Perhaps we're missing a copy of the
Contributor
Author
There was a problem hiding this comment. Good point, I think the problem with the current approach is that it will send both the external source file and mic source to one track. I try patching it, can you check it again ? It might worth coming up with some integration tests, let me work on it |
||
| + config_.external_source = true; | ||
| + stream_->Reconfigure(config_, nullptr); | ||
| + } | ||
| source->SetSink(this); | ||
| source_ = source; | ||
| UpdateSendState(); | ||
| diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc | ||
| index d5edbbf0ed..c14ddfe868 100644 | ||
| --- a/pc/rtp_sender.cc | ||
| +++ b/pc/rtp_sender.cc | ||
| @@ -786,6 +786,13 @@ void AudioRtpSender::SetSend() { | ||
| RTC_DCHECK_RUN_ON(signaling_thread_); | ||
| RTC_DCHECK(!stopped_); | ||
| RTC_DCHECK(can_send_track()); | ||
| + | ||
| + // Propagate is_external_source from AudioSourceInterface to the sink adapter. | ||
| + // This ensures the voice engine knows not to mix ADM audio with external sources. | ||
| + if (audio_track()->GetSource() && audio_track()->GetSource()->is_external_source()) { | ||
| + sink_adapter_->set_is_external_source(true); | ||
| + } | ||
| + | ||
| if (!media_channel_) { | ||
| RTC_LOG(LS_ERROR) << "SetAudioSend: No audio channel exists."; | ||
| return; | ||
| diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h | ||
| index eaffd4ef0f..d0489df9e7 100644 | ||
| --- a/pc/rtp_sender.h | ||
| +++ b/pc/rtp_sender.h | ||
| @@ -307,6 +307,12 @@ class LocalAudioSinkAdapter : public AudioTrackSinkInterface, | ||
| LocalAudioSinkAdapter(); | ||
| virtual ~LocalAudioSinkAdapter(); | ||
|
|
||
| + // Set whether the original AudioSourceInterface is an external source. | ||
| + // This propagates the is_external_source state from AudioSourceInterface | ||
| + // to this AudioSource adapter. | ||
| + void set_is_external_source(bool value) { is_external_source_ = value; } | ||
| + bool is_external_source() const override { return is_external_source_; } | ||
| + | ||
| private: | ||
| // AudioSinkInterface implementation. | ||
| void OnData(const void* audio_data, | ||
| @@ -337,6 +343,7 @@ class LocalAudioSinkAdapter : public AudioTrackSinkInterface, | ||
| // Critical section protecting `sink_`. | ||
| Mutex lock_; | ||
| int num_preferred_channels_ = -1; | ||
| + bool is_external_source_ = false; | ||
| }; | ||
|
|
||
| class AudioRtpSender : public DtmfProviderInterface, public RtpSenderBase { | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use_clang_modules=false seem to make the licence generate script fail, @cloudwebrtc , any concern of removing this flag ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah,
use_clang_modules=falsewas introduced in the m144 upgrade; I'll double-check if it's a required parameter.