Index: webrtc/audio/audio_send_stream.cc |
diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc |
index ec18125427282d4624543bc45b274d2b1a42692e..24507c53a803769333e7ecbfef2c0031bffba80c 100644 |
--- a/webrtc/audio/audio_send_stream.cc |
+++ b/webrtc/audio/audio_send_stream.cc |
@@ -11,6 +11,7 @@ |
#include "webrtc/audio/audio_send_stream.h" |
#include <string> |
+#include <utility> |
#include "webrtc/audio/audio_state.h" |
#include "webrtc/audio/conversion.h" |
@@ -19,6 +20,7 @@ |
#include "webrtc/base/event.h" |
#include "webrtc/base/logging.h" |
#include "webrtc/base/task_queue.h" |
+#include "webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng.h" |
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" |
#include "webrtc/modules/congestion_controller/include/congestion_controller.h" |
#include "webrtc/modules/pacing/paced_sender.h" |
@@ -30,15 +32,6 @@ |
namespace webrtc { |
-namespace { |
- |
-constexpr char kOpusCodecName[] = "opus"; |
- |
-bool IsCodec(const webrtc::CodecInst& codec, const char* ref_name) { |
- return (STR_CASE_CMP(codec.plname, ref_name) == 0); |
-} |
-} // namespace |
- |
namespace internal { |
AudioSendStream::AudioSendStream( |
const webrtc::AudioSendStream::Config& config, |
@@ -170,11 +163,10 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const { |
// implementation. |
stats.aec_quality_min = -1; |
- webrtc::CodecInst codec_inst = {0}; |
- if (channel_proxy_->GetSendCodec(&codec_inst)) { |
the sun
2017/03/16 08:48:19
Do you have a separate CL lined up to clean out th
|
- RTC_DCHECK_NE(codec_inst.pltype, -1); |
- stats.codec_name = codec_inst.plname; |
- stats.codec_payload_type = rtc::Optional<int>(codec_inst.pltype); |
+ const auto& spec = config_.send_codec_spec; |
+ if (config_.send_codec_spec.format.name != "") { |
kwiberg-webrtc
2017/03/01 12:26:47
Is this a test for whether the format is "empty"?
the sun
2017/03/02 20:37:24
nit: use spec.format.name
ossu
2017/03/02 20:43:27
Sure thing!
|
+ stats.codec_name = spec.format.name; |
+ stats.codec_payload_type = rtc::Optional<int>(spec.payload_type); |
// Get data from the last remote RTCP report. |
for (const auto& block : channel_proxy_->GetRemoteRTCPReportBlocks()) { |
@@ -183,10 +175,10 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const { |
stats.packets_lost = block.cumulative_num_packets_lost; |
stats.fraction_lost = Q8ToFloat(block.fraction_lost); |
stats.ext_seqnum = block.extended_highest_sequence_number; |
- // Convert samples to milliseconds. |
- if (codec_inst.plfreq / 1000 > 0) { |
+ // Convert timestamps to milliseconds. |
+ if (spec.format.clockrate_hz / 1000 > 0) { |
stats.jitter_ms = |
- block.interarrival_jitter / (codec_inst.plfreq / 1000); |
+ block.interarrival_jitter / (spec.format.clockrate_hz / 1000); |
} |
break; |
} |
@@ -274,113 +266,50 @@ VoiceEngine* AudioSendStream::voice_engine() const { |
// Apply current codec settings to a single voe::Channel used for sending. |
bool AudioSendStream::SetupSendCodec() { |
- // Disable VAD and FEC unless we know the other side wants them. |
- channel_proxy_->SetVADStatus(false); |
- channel_proxy_->SetCodecFECStatus(false); |
- |
- // We disable audio network adaptor here. This will on one hand make sure that |
- // audio network adaptor is disabled by default, and on the other allow audio |
- // network adaptor to be reconfigured, since SetReceiverFrameLengthRange can |
- // be only called when audio network adaptor is disabled. |
- channel_proxy_->DisableAudioNetworkAdaptor(); |
+ // TODO(ossu): This check is due to some Call tests creating AudioSendStreams |
+ // without a reasonable config. |
the sun
2017/03/16 08:48:19
Is that because they're explicitly setting up the
ossu
2017/03/20 18:19:48
After working through it, I realized streams getti
|
+ if (!config_.encoder_factory || config_.send_codec_spec.format.name == "") |
+ return false; |
const auto& send_codec_spec = config_.send_codec_spec; |
+ std::unique_ptr<AudioEncoder> encoder = |
+ config_.encoder_factory->MakeAudioEncoder(send_codec_spec.payload_type, |
+ send_codec_spec.format); |
- // We set the codec first, since the below extra configuration is only applied |
- // to the "current" codec. |
- |
- // If codec is already configured, we do not it again. |
- // TODO(minyue): check if this check is really needed, or can we move it into |
- // |codec->SetSendCodec|. |
- webrtc::CodecInst current_codec = {0}; |
- if (!channel_proxy_->GetSendCodec(¤t_codec) || |
- (send_codec_spec.codec_inst != current_codec)) { |
- if (!channel_proxy_->SetSendCodec(send_codec_spec.codec_inst)) { |
- LOG(LS_WARNING) << "SetSendCodec() failed."; |
- return false; |
- } |
+ if (!encoder) { |
the sun
2017/03/16 08:48:19
Any chance we could make this a CHECK or DCHECK go
ossu
2017/03/20 18:19:48
I've rephrased it and made it log an error instead
|
+ LOG(LS_WARNING) << "Unknown format " << send_codec_spec.format; |
+ return false; |
} |
- // Codec internal FEC. Treat any failure as fatal internal error. |
- if (send_codec_spec.enable_codec_fec) { |
- if (!channel_proxy_->SetCodecFECStatus(true)) { |
- LOG(LS_WARNING) << "SetCodecFECStatus() failed."; |
- return false; |
- } |
- } |
- |
- // DTX and maxplaybackrate are only set if current codec is Opus. |
- if (IsCodec(send_codec_spec.codec_inst, kOpusCodecName)) { |
- if (!channel_proxy_->SetOpusDtx(send_codec_spec.enable_opus_dtx)) { |
- LOG(LS_WARNING) << "SetOpusDtx() failed."; |
- return false; |
- } |
- |
- // If opus_max_playback_rate <= 0, the default maximum playback rate |
- // (48 kHz) will be used. |
- if (send_codec_spec.opus_max_playback_rate > 0) { |
- if (!channel_proxy_->SetOpusMaxPlaybackRate( |
- send_codec_spec.opus_max_playback_rate)) { |
- LOG(LS_WARNING) << "SetOpusMaxPlaybackRate() failed."; |
- return false; |
- } |
- } |
- |
- if (config_.audio_network_adaptor_config) { |
- // Audio network adaptor is only allowed for Opus currently. |
- // |SetReceiverFrameLengthRange| needs to be called before |
- // |EnableAudioNetworkAdaptor|. |
- channel_proxy_->SetReceiverFrameLengthRange(send_codec_spec.min_ptime_ms, |
- send_codec_spec.max_ptime_ms); |
- channel_proxy_->EnableAudioNetworkAdaptor( |
- *config_.audio_network_adaptor_config); |
- LOG(LS_INFO) << "Audio network adaptor enabled on SSRC " |
- << config_.rtp.ssrc; |
- } |
+ // If a bitrate has been specified for the codec, use it over the |
+ // codec's default. |
+ if (send_codec_spec.target_bitrate_bps) { |
+ encoder->OnReceivedTargetAudioBitrate(*send_codec_spec.target_bitrate_bps); |
} |
// Set the CN payloadtype and the VAD status. |
the sun
2017/03/16 08:48:19
Comment is a bit off.
ossu
2017/03/20 18:19:48
Acknowledged.
|
if (send_codec_spec.cng_payload_type != -1) { |
- // The CN payload type for 8000 Hz clockrate is fixed at 13. |
- if (send_codec_spec.cng_plfreq != 8000) { |
- webrtc::PayloadFrequencies cn_freq; |
- switch (send_codec_spec.cng_plfreq) { |
- case 16000: |
- cn_freq = webrtc::kFreq16000Hz; |
- break; |
- case 32000: |
- cn_freq = webrtc::kFreq32000Hz; |
- break; |
- default: |
- RTC_NOTREACHED(); |
- return false; |
- } |
- if (!channel_proxy_->SetSendCNPayloadType( |
- send_codec_spec.cng_payload_type, cn_freq)) { |
- LOG(LS_WARNING) << "SetSendCNPayloadType() failed."; |
- // TODO(ajm): This failure condition will be removed from VoE. |
- // Restore the return here when we update to a new enough webrtc. |
- // |
- // Not returning false because the SetSendCNPayloadType will fail if |
- // the channel is already sending. |
- // This can happen if the remote description is applied twice, for |
- // example in the case of ROAP on top of JSEP, where both side will |
- // send the offer. |
- } |
- } |
+ AudioEncoderCng::Config config; |
+ config.num_channels = encoder->NumChannels(); |
+ config.payload_type = send_codec_spec.cng_payload_type; |
+ config.speech_encoder = std::move(encoder); |
+ config.vad_mode = Vad::kVadNormal; |
+ encoder = |
+ std::unique_ptr<AudioEncoder>(new AudioEncoderCng(std::move(config))); |
kwiberg-webrtc
2017/03/01 12:26:47
encoder.reset(...) would be shorter here.
ossu
2017/03/02 01:30:28
Yupp! Will change.
|
+ } |
- // Only turn on VAD if we have a CN payload type that matches the |
- // clockrate for the codec we are going to use. |
- if (send_codec_spec.cng_plfreq == send_codec_spec.codec_inst.plfreq && |
- send_codec_spec.codec_inst.channels == 1) { |
- // TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the |
- // interaction between VAD and Opus FEC. |
- if (!channel_proxy_->SetVADStatus(true)) { |
- LOG(LS_WARNING) << "SetVADStatus() failed."; |
- return false; |
- } |
- } |
+ channel_proxy_->SetEncoder(send_codec_spec.payload_type, std::move(encoder)); |
+ |
+ // TODO(ossu): The encoder interface wants an RtcEventLogProxy and a Clock, |
the sun
2017/03/16 08:48:19
We've decided to use the webrtc/base/timeutils.h A
|
+ // both of which are in Channel. Solve this! |
+ if (config_.audio_network_adaptor_config) { |
+ // Audio network adaptor is only allowed for Opus currently. |
kwiberg-webrtc
2017/03/01 12:26:47
This comment doesn't appear to have any correspond
ossu
2017/03/02 01:30:28
Hmm... I agree that it doesn't describe the code t
|
+ channel_proxy_->EnableAudioNetworkAdaptor( |
the sun
2017/03/02 20:37:24
Is it possible to avoid this call to voe::Channel
ossu
2017/03/02 20:43:27
I'll look into it. The AudioEncoder version of thi
ossu
2017/03/20 18:19:48
Turns out avoiding the call through ChannelProxy h
|
+ *config_.audio_network_adaptor_config); |
+ LOG(LS_INFO) << "Audio network adaptor enabled on SSRC " |
+ << config_.rtp.ssrc; |
} |
+ |
return true; |
} |