Closed
Bug 1396107
Opened 7 years ago
Closed 7 years ago
Insert outgoing audio into peerconnections without using removed external media interfaces
Categories
(Core :: WebRTC: Audio/Video, enhancement, P1)
Core
WebRTC: Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: jesup, Assigned: jesup)
Details
Attachments
(1 file, 1 obsolete file)
7.95 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
We're doing a bunch of processing when inserting audio into webrtc.org code for encoding and sending that we don't need to do, and is part of why we're carrying the ExternalMedia code that's been removed from upstream. In particular, this is running the Agc/Vad code to analyze the outgoing stream before encoding it; in a linux perf trace I saw near 1% of samples hitting within that code. Switch to the same methods webrtcaudioengine.cc (in the tree) is using to insert audio.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8903802 -
Flags: review?(padenot)
Attachment #8903802 -
Flags: review?(dminor)
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=21c5422a6050c86f9de069db37e6122f3145d3f4 This is 'perf' output (ugh) that shows the issue: --3.72%--webrtc::VoEExternalMediaImpl::ExternalRecordingInsertData | |--2.23%--webrtc::voe::TransmitMixer::PrepareDemux | | | --2.23%--webrtc::voe::TransmitMixer::ProcessAudio | | | --2.23%--webrtc::AudioProcessingImpl::ProcessStream | | | |--1.27%--webrtc::AudioProcessingImpl::ProcessCaptureStreamLocked | | | | | --1.27%--webrtc::AgcManagerDirect::Process | | | | | --1.27%--webrtc::Agc::Process | | | | | --1.27%--webrtc::VoiceActivityDetector::ProcessChunk | | | | | --1.27%--webrtc::VadAudioProc::ExtractFeatures | | | | | --1.27%--webrtc::VadAudioProc::PitchAnalysis | | | | | --1.27%--WebRtcIsac_PitchAnalysis | | | | | --1.27%--FilterFrame | | | | | --1.27%--FilterSegment | | | --0.95%--webrtc::AudioBuffer::DeinterleaveFrom | | | --0.95%--webrtc::DownmixInterleavedToMono<short> | --1.49%--webrtc::voe::TransmitMixer::EncodeAndSend | --1.49%--webrtc::voe::Channel::EncodeAndSend | --1.49%--webrtc::(anonymous namespace)::AudioCodingModuleImpl::Add10MsData | --1.49%--webrtc::AudioEncoder::Encode | --1.49%--webrtc::AudioEncoderOpus::EncodeImpl | --1.49%--WebRtcOpus_Encode | --1.49%--opus_encode | --1.49%--opus_encode_native | --1.49%--run_analysis | --1.49%--pow@plt I imagine the formatting will go all to hell...
Assignee | ||
Comment 3•7 years ago
|
||
Go ahead and look, but the Try permafails 3 tests locally (and maybe a couple more on Try). Very close, though; should be easy to debug.
Comment 4•7 years ago
|
||
Comment on attachment 8903802 [details] [diff] [review] switch to using the same audio input method upstream webrtc.org does Review of attachment 8903802 [details] [diff] [review]: ----------------------------------------------------------------- Looks like it would work, but obviously it doesn't. ::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp @@ +647,5 @@ > > capture_delay = mCaptureDelay; > //Insert the samples > + mPtrVoEBase->audio_transport()->PushCaptureData(mChannel, audio_data, > + sizeof(audio_data[0]), This is called `bits_per_sample` so I suppose it should be `sizeof(audio_data[0]) * 8`. This particular parameter does not appear to be used for now, but maybe they'll implement a float32 API at some point instead of hard-coding to uint16_t.
Attachment #8903802 -
Flags: review?(padenot)
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=923d6fac5b3189e53d18dee03fe3719093a959b0 -- looks green with most tests done. Was missing a 'don't multiply by channels since we now pass channels down'
Attachment #8905311 -
Flags: review?(padenot)
Assignee | ||
Updated•7 years ago
|
Attachment #8903802 -
Attachment is obsolete: true
Attachment #8903802 -
Flags: review?(dminor)
Updated•7 years ago
|
Rank: 15
Priority: -- → P1
Updated•7 years ago
|
Attachment #8905311 -
Flags: review?(padenot) → review+
Pushed by rjesup@wgate.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f4561375608b switch to using the same audio input method upstream webrtc.org does r=dminor,padenot
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f4561375608b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•