Clean up logging in AudioSendStream::SetupSendCodec().
As a side effect:
- Moved the AudioSendStream::Config::SendCodecSpec methods into the .cc.
- Which exposed an issue with event_visualizer_utils not having a dependency on api:call_api set up.
- Which further exposed clang warnings about large inlined default methods in webrtc/config.h.
BUG=webrtc:4690
Committed: https://crrev.com/1836fd6257a692959b3b49ba99ef587ad9995871
Review-Url: https://codereview.webrtc.org/2446963003
Cr-Original-Commit-Position: refs/heads/master@{#14771}
Cr-Commit-Position: refs/heads/master@{#14780}
diff --git a/webrtc/api/BUILD.gn b/webrtc/api/BUILD.gn
index 9a28602..69fd7f4 100644
--- a/webrtc/api/BUILD.gn
+++ b/webrtc/api/BUILD.gn
@@ -21,6 +21,7 @@
rtc_source_set("call_api") {
sources = [
"call/audio_receive_stream.h",
+ "call/audio_send_stream.cc",
"call/audio_send_stream.h",
"call/audio_sink.h",
"call/audio_state.h",
diff --git a/webrtc/api/api.gyp b/webrtc/api/api.gyp
index 8a7fe5a..b50dfd5 100644
--- a/webrtc/api/api.gyp
+++ b/webrtc/api/api.gyp
@@ -105,6 +105,7 @@
],
'sources': [
'call/audio_receive_stream.h',
+ 'call/audio_send_stream.cc',
'call/audio_send_stream.h',
'call/audio_sink.h',
'call/audio_state.h',
diff --git a/webrtc/api/call/audio_send_stream.cc b/webrtc/api/call/audio_send_stream.cc
new file mode 100644
index 0000000..06cbc54
--- /dev/null
+++ b/webrtc/api/call/audio_send_stream.cc
@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/api/call/audio_send_stream.h"
+
+#include <string>
+
+namespace {
+
+std::string ToString(const webrtc::CodecInst& codec_inst) {
+ std::stringstream ss;
+ ss << "{pltype: " << codec_inst.pltype;
+ ss << ", plname: \"" << codec_inst.plname << "\"";
+ ss << ", plfreq: " << codec_inst.plfreq;
+ ss << ", pacsize: " << codec_inst.pacsize;
+ ss << ", channels: " << codec_inst.channels;
+ ss << ", rate: " << codec_inst.rate;
+ ss << '}';
+ return ss.str();
+}
+} // namespace
+
+namespace webrtc {
+
+AudioSendStream::Stats::Stats() = default;
+
+AudioSendStream::Config::Config(Transport* send_transport)
+ : send_transport(send_transport) {}
+
+std::string AudioSendStream::Config::ToString() const {
+ std::stringstream ss;
+ ss << "{rtp: " << rtp.ToString();
+ ss << ", send_transport: " << (send_transport ? "(Transport)" : "nullptr");
+ ss << ", voe_channel_id: " << voe_channel_id;
+ ss << ", min_bitrate_kbps: " << min_bitrate_kbps;
+ ss << ", max_bitrate_kbps: " << max_bitrate_kbps;
+ ss << ", send_codec_spec: " << send_codec_spec.ToString();
+ ss << '}';
+ return ss.str();
+}
+
+AudioSendStream::Config::Rtp::Rtp() = default;
+
+AudioSendStream::Config::Rtp::~Rtp() = default;
+
+std::string AudioSendStream::Config::Rtp::ToString() const {
+ std::stringstream ss;
+ ss << "{ssrc: " << ssrc;
+ ss << ", extensions: [";
+ for (size_t i = 0; i < extensions.size(); ++i) {
+ ss << extensions[i].ToString();
+ if (i != extensions.size() - 1) {
+ ss << ", ";
+ }
+ }
+ ss << ']';
+ ss << ", nack: " << nack.ToString();
+ ss << ", c_name: " << c_name;
+ ss << '}';
+ return ss.str();
+}
+
+AudioSendStream::Config::SendCodecSpec::SendCodecSpec() {
+ webrtc::CodecInst empty_inst = {0};
+ codec_inst = empty_inst;
+ codec_inst.pltype = -1;
+}
+
+std::string AudioSendStream::Config::SendCodecSpec::ToString() const {
+ std::stringstream ss;
+ ss << "{nack_enabled: " << (nack_enabled ? "true" : "false");
+ ss << ", transport_cc_enabled: " << (transport_cc_enabled ? "true" : "false");
+ ss << ", enable_codec_fec: " << (enable_codec_fec ? "true" : "false");
+ ss << ", enable_opus_dtx: " << (enable_opus_dtx ? "true" : "false");
+ ss << ", opus_max_playback_rate: " << opus_max_playback_rate;
+ ss << ", cng_payload_type: " << cng_payload_type;
+ ss << ", cng_plfreq: " << cng_plfreq;
+ ss << ", codec_inst: " << ::ToString(codec_inst);
+ ss << '}';
+ return ss.str();
+}
+
+bool AudioSendStream::Config::SendCodecSpec::operator==(
+ const AudioSendStream::Config::SendCodecSpec& rhs) const {
+ if (nack_enabled != rhs.nack_enabled) {
+ return false;
+ }
+ if (transport_cc_enabled != rhs.transport_cc_enabled) {
+ return false;
+ }
+ if (enable_codec_fec != rhs.enable_codec_fec) {
+ return false;
+ }
+ if (enable_opus_dtx != rhs.enable_opus_dtx) {
+ return false;
+ }
+ if (opus_max_playback_rate != rhs.opus_max_playback_rate) {
+ return false;
+ }
+ if (cng_payload_type != rhs.cng_payload_type) {
+ return false;
+ }
+ if (cng_plfreq != rhs.cng_plfreq) {
+ return false;
+ }
+ if (codec_inst != rhs.codec_inst) {
+ return false;
+ }
+ return true;
+}
+} // namespace webrtc
diff --git a/webrtc/api/call/audio_send_stream.h b/webrtc/api/call/audio_send_stream.h
index 1956b97..7ff791e 100644
--- a/webrtc/api/call/audio_send_stream.h
+++ b/webrtc/api/call/audio_send_stream.h
@@ -30,6 +30,8 @@
class AudioSendStream {
public:
struct Stats {
+ Stats();
+
// TODO(solenberg): Harmonize naming and defaults with receive stream stats.
uint32_t local_ssrc = 0;
int64_t bytes_sent = 0;
@@ -52,13 +54,13 @@
struct Config {
Config() = delete;
- explicit Config(Transport* send_transport)
- : send_transport(send_transport) {}
-
+ explicit Config(Transport* send_transport);
std::string ToString() const;
// Send-stream specific RTP settings.
struct Rtp {
+ Rtp();
+ ~Rtp();
std::string ToString() const;
// Sender SSRC.
@@ -91,40 +93,10 @@
int max_bitrate_kbps = -1;
struct SendCodecSpec {
- SendCodecSpec() {
- webrtc::CodecInst empty_inst = {0};
- codec_inst = empty_inst;
- codec_inst.pltype = -1;
- }
- bool operator==(const SendCodecSpec& rhs) const {
- {
- if (nack_enabled != rhs.nack_enabled) {
- return false;
- }
- if (transport_cc_enabled != rhs.transport_cc_enabled) {
- return false;
- }
- if (enable_codec_fec != rhs.enable_codec_fec) {
- return false;
- }
- if (enable_opus_dtx != rhs.enable_opus_dtx) {
- return false;
- }
- if (opus_max_playback_rate != rhs.opus_max_playback_rate) {
- return false;
- }
- if (cng_payload_type != rhs.cng_payload_type) {
- return false;
- }
- if (cng_plfreq != rhs.cng_plfreq) {
- return false;
- }
- if (codec_inst != rhs.codec_inst) {
- return false;
- }
- return true;
- }
- }
+ SendCodecSpec();
+ std::string ToString() const;
+
+ bool operator==(const SendCodecSpec& rhs) const;
bool operator!=(const SendCodecSpec& rhs) const {
return !(*this == rhs);
}
diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc
index 8925b04..5c08c9b 100644
--- a/webrtc/audio/audio_send_stream.cc
+++ b/webrtc/audio/audio_send_stream.cc
@@ -35,56 +35,11 @@
constexpr char kOpusCodecName[] = "opus";
-// TODO(minyue): Remove |LOG_RTCERR2|.
-#define LOG_RTCERR2(func, a1, a2, err) \
- LOG(LS_WARNING) << "" << #func << "(" << a1 << ", " << a2 \
- << ") failed, err=" << err
-
-// TODO(minyue): Remove |LOG_RTCERR3|.
-#define LOG_RTCERR3(func, a1, a2, a3, err) \
- LOG(LS_WARNING) << "" << #func << "(" << a1 << ", " << a2 << ", " << a3 \
- << ") failed, err=" << err
-
-std::string ToString(const webrtc::CodecInst& codec) {
- std::stringstream ss;
- ss << codec.plname << "/" << codec.plfreq << "/" << codec.channels << " ("
- << codec.pltype << ")";
- return ss.str();
-}
-
bool IsCodec(const webrtc::CodecInst& codec, const char* ref_name) {
return (_stricmp(codec.plname, ref_name) == 0);
}
-
} // namespace
-std::string AudioSendStream::Config::Rtp::ToString() const {
- std::stringstream ss;
- ss << "{ssrc: " << ssrc;
- ss << ", extensions: [";
- for (size_t i = 0; i < extensions.size(); ++i) {
- ss << extensions[i].ToString();
- if (i != extensions.size() - 1) {
- ss << ", ";
- }
- }
- ss << ']';
- ss << ", nack: " << nack.ToString();
- ss << ", c_name: " << c_name;
- ss << '}';
- return ss.str();
-}
-
-std::string AudioSendStream::Config::ToString() const {
- std::stringstream ss;
- ss << "{rtp: " << rtp.ToString();
- ss << ", voe_channel_id: " << voe_channel_id;
- // TODO(solenberg): Encoder config.
- ss << ", cng_payload_type: " << send_codec_spec.cng_payload_type;
- ss << '}';
- return ss.str();
-}
-
namespace internal {
AudioSendStream::AudioSendStream(
const webrtc::AudioSendStream::Config& config,
@@ -333,11 +288,8 @@
const auto& send_codec_spec = config_.send_codec_spec;
- // Set the codec immediately, since SetVADStatus() depends on whether
- // the current codec is mono or stereo.
- LOG(LS_INFO) << "Send channel " << channel << " selected voice codec "
- << ToString(send_codec_spec.codec_inst)
- << ", bitrate=" << send_codec_spec.codec_inst.rate;
+ // 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
@@ -346,47 +298,33 @@
if (codec->GetSendCodec(channel, current_codec) != 0 ||
(send_codec_spec.codec_inst != current_codec)) {
if (codec->SetSendCodec(channel, send_codec_spec.codec_inst) == -1) {
- LOG_RTCERR2(SetSendCodec, channel, ToString(send_codec_spec.codec_inst),
- base->LastError());
+ LOG(LS_WARNING) << "SetSendCodec() failed: " << base->LastError();
return false;
}
}
- // FEC should be enabled after SetSendCodec.
+ // Codec internal FEC. Treat any failure as fatal internal error.
if (send_codec_spec.enable_codec_fec) {
- LOG(LS_INFO) << "Attempt to enable codec internal FEC on channel "
- << channel;
- if (codec->SetFECStatus(channel, true) == -1) {
- // Enable codec internal FEC. Treat any failure as fatal internal error.
- LOG_RTCERR2(SetFECStatus, channel, true, base->LastError());
+ if (codec->SetFECStatus(channel, true) != 0) {
+ LOG(LS_WARNING) << "SetFECStatus() failed: " << base->LastError();
return false;
}
}
+ // DTX and maxplaybackrate are only set if current codec is Opus.
if (IsCodec(send_codec_spec.codec_inst, kOpusCodecName)) {
- // DTX and maxplaybackrate should be set after SetSendCodec. Because current
- // send codec has to be Opus.
-
- // Set Opus internal DTX.
- LOG(LS_INFO) << "Attempt to "
- << (send_codec_spec.enable_opus_dtx ? "enable" : "disable")
- << " Opus DTX on channel " << channel;
- if (codec->SetOpusDtx(channel, send_codec_spec.enable_opus_dtx)) {
- LOG_RTCERR2(SetOpusDtx, channel, send_codec_spec.enable_opus_dtx,
- base->LastError());
+ if (codec->SetOpusDtx(channel, send_codec_spec.enable_opus_dtx) != 0) {
+ LOG(LS_WARNING) << "SetOpusDtx() failed: " << base->LastError();
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) {
- LOG(LS_INFO) << "Attempt to set maximum playback rate to "
- << send_codec_spec.opus_max_playback_rate
- << " Hz on channel " << channel;
if (codec->SetOpusMaxPlaybackRate(
- channel, send_codec_spec.opus_max_playback_rate) == -1) {
- LOG_RTCERR2(SetOpusMaxPlaybackRate, channel,
- send_codec_spec.opus_max_playback_rate, base->LastError());
+ channel, send_codec_spec.opus_max_playback_rate) != 0) {
+ LOG(LS_WARNING) << "SetOpusMaxPlaybackRate() failed: "
+ << base->LastError();
return false;
}
}
@@ -409,11 +347,9 @@
return false;
}
if (codec->SetSendCNPayloadType(channel, send_codec_spec.cng_payload_type,
- cn_freq) == -1) {
- LOG_RTCERR3(SetSendCNPayloadType, channel,
- send_codec_spec.cng_payload_type, cn_freq,
- base->LastError());
-
+ cn_freq) != 0) {
+ LOG(LS_WARNING) << "SetSendCNPayloadType() failed: "
+ << base->LastError();
// TODO(ajm): This failure condition will be removed from VoE.
// Restore the return here when we update to a new enough webrtc.
//
@@ -431,9 +367,8 @@
send_codec_spec.codec_inst.channels == 1) {
// TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the
// interaction between VAD and Opus FEC.
- LOG(LS_INFO) << "Enabling VAD";
- if (codec->SetVADStatus(channel, true) == -1) {
- LOG_RTCERR2(SetVADStatus, channel, true, base->LastError());
+ if (codec->SetVADStatus(channel, true) != 0) {
+ LOG(LS_WARNING) << "SetVADStatus() failed: " << base->LastError();
return false;
}
}
diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc
index 14ca0e0..a2832de 100644
--- a/webrtc/audio/audio_send_stream_unittest.cc
+++ b/webrtc/audio/audio_send_stream_unittest.cc
@@ -219,12 +219,26 @@
RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeId));
config.rtp.c_name = kCName;
config.voe_channel_id = kChannelId;
+ config.min_bitrate_kbps = 12;
+ config.max_bitrate_kbps = 34;
+ config.send_codec_spec.nack_enabled = true;
+ config.send_codec_spec.transport_cc_enabled = false;
+ config.send_codec_spec.enable_codec_fec = true;
+ config.send_codec_spec.enable_opus_dtx = false;
+ config.send_codec_spec.opus_max_playback_rate = 32000;
config.send_codec_spec.cng_payload_type = 42;
+ config.send_codec_spec.cng_plfreq = 56;
+ config.send_codec_spec.codec_inst = kIsacCodec;
EXPECT_EQ(
"{rtp: {ssrc: 1234, extensions: [{uri: "
"http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time, id: 3}], "
- "nack: {rtp_history_ms: 0}, c_name: foo_name}, voe_channel_id: 1, "
- "cng_payload_type: 42}",
+ "nack: {rtp_history_ms: 0}, c_name: foo_name}, send_transport: nullptr, "
+ "voe_channel_id: 1, min_bitrate_kbps: 12, max_bitrate_kbps: 34, "
+ "send_codec_spec: {nack_enabled: true, transport_cc_enabled: false, "
+ "enable_codec_fec: true, enable_opus_dtx: false, opus_max_playback_rate: "
+ "32000, cng_payload_type: 42, cng_plfreq: 56, codec_inst: {pltype: "
+ "103, plname: \"isac\", plfreq: 16000, pacsize: 320, channels: 1, rate: "
+ "32000}}}",
config.ToString());
}
diff --git a/webrtc/config.cc b/webrtc/config.cc
index 04f6a66..281b89f 100644
--- a/webrtc/config.cc
+++ b/webrtc/config.cc
@@ -31,6 +31,11 @@
return ss.str();
}
+FlexfecConfig::FlexfecConfig()
+ : flexfec_payload_type(-1), flexfec_ssrc(0), protected_media_ssrcs() {}
+
+FlexfecConfig::~FlexfecConfig() = default;
+
std::string FlexfecConfig::ToString() const {
std::stringstream ss;
ss << "{flexfec_payload_type: " << flexfec_payload_type;
@@ -134,6 +139,8 @@
max_bitrate_bps(0),
number_of_streams(0) {}
+VideoEncoderConfig::VideoEncoderConfig(VideoEncoderConfig&&) = default;
+
VideoEncoderConfig::~VideoEncoderConfig() = default;
std::string VideoEncoderConfig::ToString() const {
@@ -155,6 +162,8 @@
return ss.str();
}
+VideoEncoderConfig::VideoEncoderConfig(const VideoEncoderConfig&) = default;
+
void VideoEncoderConfig::EncoderSpecificSettings::FillEncoderSpecificSettings(
VideoCodec* codec) const {
if (codec->codecType == kVideoCodecH264) {
@@ -210,4 +219,8 @@
*vp9_settings = specifics_;
}
+DecoderSpecificSettings::DecoderSpecificSettings() = default;
+
+DecoderSpecificSettings::~DecoderSpecificSettings() = default;
+
} // namespace webrtc
diff --git a/webrtc/config.h b/webrtc/config.h
index 4ec895b..3b3c3d9 100644
--- a/webrtc/config.h
+++ b/webrtc/config.h
@@ -57,8 +57,8 @@
// Settings for FlexFEC forward error correction.
// Set the payload type to '-1' to disable.
struct FlexfecConfig {
- FlexfecConfig()
- : flexfec_payload_type(-1), flexfec_ssrc(0), protected_media_ssrcs() {}
+ FlexfecConfig();
+ ~FlexfecConfig();
std::string ToString() const;
// Payload type of FlexFEC.
@@ -163,7 +163,7 @@
virtual void FillVideoCodecVp9(VideoCodecVP9* vp9_settings) const;
virtual void FillVideoCodecH264(VideoCodecH264* h264_settings) const;
private:
- virtual ~EncoderSpecificSettings() {}
+ ~EncoderSpecificSettings() override {}
friend class VideoEncoderConfig;
};
@@ -211,7 +211,7 @@
const VideoEncoderConfig& encoder_config) = 0;
protected:
- virtual ~VideoStreamFactoryInterface() {}
+ ~VideoStreamFactoryInterface() override {}
};
VideoEncoderConfig& operator=(VideoEncoderConfig&&) = default;
@@ -221,7 +221,7 @@
VideoEncoderConfig Copy() const { return VideoEncoderConfig(*this); }
VideoEncoderConfig();
- VideoEncoderConfig(VideoEncoderConfig&&) = default;
+ VideoEncoderConfig(VideoEncoderConfig&&);
~VideoEncoderConfig();
std::string ToString() const;
@@ -243,7 +243,7 @@
private:
// Access to the copy constructor is private to force use of the Copy()
// method for those exceptional cases where we do use it.
- VideoEncoderConfig(const VideoEncoderConfig&) = default;
+ VideoEncoderConfig(const VideoEncoderConfig&);
};
struct VideoDecoderH264Settings {
@@ -252,7 +252,8 @@
class DecoderSpecificSettings {
public:
- virtual ~DecoderSpecificSettings() {}
+ DecoderSpecificSettings();
+ virtual ~DecoderSpecificSettings();
rtc::Optional<VideoDecoderH264Settings> h264_extra_settings;
};
diff --git a/webrtc/logging/BUILD.gn b/webrtc/logging/BUILD.gn
index fa28065..54daa20 100644
--- a/webrtc/logging/BUILD.gn
+++ b/webrtc/logging/BUILD.gn
@@ -32,6 +32,7 @@
deps = [
":rtc_event_log_api",
"..:webrtc_common",
+ "../api:call_api",
"../modules/rtp_rtcp",
]
diff --git a/webrtc/webrtc.gyp b/webrtc/webrtc.gyp
index 98d9498..1447a99 100644
--- a/webrtc/webrtc.gyp
+++ b/webrtc/webrtc.gyp
@@ -67,6 +67,7 @@
'dependencies': [
'rtc_event_log_api',
'rtc_event_log_proto',
+ '<(webrtc_root)/api/api.gyp:call_api',
],
'defines': [
'ENABLE_RTC_EVENT_LOG',
