diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 2ce0b48f8d..01c6ed4fa3 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -13,8 +13,8 @@ #include +#include #include -#include #include #include "absl/types/optional.h" @@ -157,12 +157,12 @@ struct RTC_EXPORT RtpCodecCapability { // Contrary to ORTC, these parameters are named using all lowercase strings. // This helps make the mapping to SDP simpler, if an application is using SDP. // Boolean values are represented by the string "1". - std::unordered_map parameters; + std::map parameters; // Codec-specific parameters that may optionally be signaled to the remote // party. // TODO(deadbeef): Not implemented. - std::unordered_map options; + std::map options; // Maximum number of temporal layer extensions supported by this codec. // For example, a value of 1 indicates that 2 total layers are supported. @@ -500,7 +500,7 @@ struct RTC_EXPORT RtpCodecParameters { // Contrary to ORTC, these parameters are named using all lowercase strings. // This helps make the mapping to SDP simpler, if an application is using SDP. // Boolean values are represented by the string "1". - std::unordered_map parameters; + std::map parameters; bool operator==(const RtpCodecParameters& o) const { return name == o.name && kind == o.kind && payload_type == o.payload_type && diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index 43f8839b90..197a810d29 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -118,9 +118,7 @@ class RTC_EXPORT RTCCodecStats final : public RTCStats { RTCStatsMember payload_type; RTCStatsMember mime_type; RTCStatsMember clock_rate; - // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7061 RTCStatsMember channels; - // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7061 RTCStatsMember sdp_fmtp_line; }; diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 116b4ba497..5b34f1cac8 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -10,6 +10,7 @@ #include "pc/rtc_stats_collector.h" +#include #include #include #include @@ -24,6 +25,7 @@ #include "p2p/base/port.h" #include "pc/peer_connection.h" #include "pc/rtc_stats_traversal.h" +#include "pc/webrtc_sdp.h" #include "rtc_base/checks.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/time_utils.h" @@ -235,6 +237,14 @@ std::unique_ptr CodecStatsFromRtpCodecParameters( if (codec_params.clock_rate) { codec_stats->clock_rate = static_cast(*codec_params.clock_rate); } + if (codec_params.num_channels) { + codec_stats->channels = *codec_params.num_channels; + } + + rtc::StringBuilder fmtp; + if (WriteFmtpParameters(codec_params.parameters, &fmtp)) { + codec_stats->sdp_fmtp_line = fmtp.Release(); + } return codec_stats; } diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index f5b3d6d733..f886e14333 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -813,6 +813,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { inbound_audio_codec.kind = cricket::MEDIA_TYPE_AUDIO; inbound_audio_codec.name = "opus"; inbound_audio_codec.clock_rate = 1337; + inbound_audio_codec.num_channels = 1; + inbound_audio_codec.parameters = {{"minptime", "10"}, {"useinbandfec", "1"}}; voice_media_info.receive_codecs.insert( std::make_pair(inbound_audio_codec.payload_type, inbound_audio_codec)); @@ -821,6 +823,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { outbound_audio_codec.kind = cricket::MEDIA_TYPE_AUDIO; outbound_audio_codec.name = "isac"; outbound_audio_codec.clock_rate = 1338; + outbound_audio_codec.num_channels = 2; voice_media_info.send_codecs.insert( std::make_pair(outbound_audio_codec.payload_type, outbound_audio_codec)); @@ -835,6 +838,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { inbound_video_codec.kind = cricket::MEDIA_TYPE_VIDEO; inbound_video_codec.name = "H264"; inbound_video_codec.clock_rate = 1339; + inbound_video_codec.parameters = {{"level-asymmetry-allowed", "1"}, + {"packetization-mode", "1"}, + {"profile-level-id", "42001f"}}; video_media_info.receive_codecs.insert( std::make_pair(inbound_video_codec.payload_type, inbound_video_codec)); @@ -856,18 +862,23 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { expected_inbound_audio_codec.payload_type = 1; expected_inbound_audio_codec.mime_type = "audio/opus"; expected_inbound_audio_codec.clock_rate = 1337; + expected_inbound_audio_codec.channels = 1; + expected_inbound_audio_codec.sdp_fmtp_line = "minptime=10;useinbandfec=1"; RTCCodecStats expected_outbound_audio_codec("RTCCodec_AudioMid_Outbound_2", report->timestamp_us()); expected_outbound_audio_codec.payload_type = 2; expected_outbound_audio_codec.mime_type = "audio/isac"; expected_outbound_audio_codec.clock_rate = 1338; + expected_outbound_audio_codec.channels = 2; RTCCodecStats expected_inbound_video_codec("RTCCodec_VideoMid_Inbound_3", report->timestamp_us()); expected_inbound_video_codec.payload_type = 3; expected_inbound_video_codec.mime_type = "video/H264"; expected_inbound_video_codec.clock_rate = 1339; + expected_inbound_video_codec.sdp_fmtp_line = + "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f"; RTCCodecStats expected_outbound_video_codec("RTCCodec_VideoMid_Outbound_4", report->timestamp_us()); diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index e3f522bd47..93a46efb8e 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -445,8 +445,14 @@ class RTCStatsReportVerifier { verifier.TestMemberIsDefined(codec.payload_type); verifier.TestMemberIsDefined(codec.mime_type); verifier.TestMemberIsPositive(codec.clock_rate); - verifier.TestMemberIsUndefined(codec.channels); - verifier.TestMemberIsUndefined(codec.sdp_fmtp_line); + + if (codec.mime_type->rfind("audio", 0) == 0) + verifier.TestMemberIsPositive(codec.channels); + else + verifier.TestMemberIsUndefined(codec.channels); + + // sdp_fmtp_line is an optional field. + verifier.MarkMemberTested(codec.sdp_fmtp_line, true); return verifier.ExpectAllMembersSuccessfullyTested(); } diff --git a/pc/rtp_parameters_conversion.cc b/pc/rtp_parameters_conversion.cc index 93f28f1815..9c7a337ab4 100644 --- a/pc/rtp_parameters_conversion.cc +++ b/pc/rtp_parameters_conversion.cc @@ -164,7 +164,7 @@ RTCErrorOr ToCricketCodec(const RtpCodecParameters& codec) { } cricket_codec.AddFeedbackParam(result.MoveValue()); } - cricket_codec.params.insert(codec.parameters.begin(), codec.parameters.end()); + cricket_codec.params = codec.parameters; return std::move(cricket_codec); } @@ -366,8 +366,7 @@ RtpCodecParameters ToRtpCodecParameters(const C& cricket_codec) { } } ToRtpCodecParametersTypeSpecific(cricket_codec, &codec_param); - codec_param.parameters.insert(cricket_codec.params.begin(), - cricket_codec.params.end()); + codec_param.parameters = cricket_codec.params; return codec_param; } diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 575f3391a9..d49684e209 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -1785,24 +1785,6 @@ void WriteFmtpParameter(const std::string& parameter_name, *os << parameter_name << kSdpDelimiterEqual << parameter_value; } -void WriteFmtpParameters(const cricket::CodecParameterMap& parameters, - rtc::StringBuilder* os) { - bool first = true; - for (const auto& entry : parameters) { - const std::string& key = entry.first; - const std::string& value = entry.second; - // Parameters are a semicolon-separated list, no spaces. - // The list is separated from the header by a space. - if (first) { - *os << kSdpDelimiterSpace; - first = false; - } else { - *os << kSdpDelimiterSemicolon; - } - WriteFmtpParameter(key, value, os); - } -} - bool IsFmtpParam(const std::string& name) { // RFC 4855, section 3 specifies the mapping of media format parameters to SDP // parameters. Only ptime, maxptime, channels and rate are placed outside of @@ -1811,31 +1793,35 @@ bool IsFmtpParam(const std::string& name) { return name != kCodecParamPTime && name != kCodecParamMaxPTime; } -// Retreives fmtp parameters from |params|, which may contain other parameters -// as well, and puts them in |fmtp_parameters|. -void GetFmtpParams(const cricket::CodecParameterMap& params, - cricket::CodecParameterMap* fmtp_parameters) { - for (const auto& entry : params) { +bool WriteFmtpParameters(const cricket::CodecParameterMap& parameters, + rtc::StringBuilder* os) { + bool empty = true; + const char* delimiter = ""; // No delimiter before first parameter. + for (const auto& entry : parameters) { const std::string& key = entry.first; const std::string& value = entry.second; + if (IsFmtpParam(key)) { - (*fmtp_parameters)[key] = value; + *os << delimiter; + // A semicolon before each subsequent parameter. + delimiter = kSdpDelimiterSemicolon; + WriteFmtpParameter(key, value, os); + empty = false; } } + + return !empty; } template void AddFmtpLine(const T& codec, std::string* message) { - cricket::CodecParameterMap fmtp_parameters; - GetFmtpParams(codec.params, &fmtp_parameters); - if (fmtp_parameters.empty()) { - // No need to add an fmtp if it will have no (optional) parameters. - return; - } rtc::StringBuilder os; WriteFmtpHeader(codec.id, &os); - WriteFmtpParameters(fmtp_parameters, &os); - AddLine(os.str(), message); + os << kSdpDelimiterSpace; + // Create FMTP line and check that it's nonempty. + if (WriteFmtpParameters(codec.params, &os)) { + AddLine(os.str(), message); + } return; } diff --git a/pc/webrtc_sdp.h b/pc/webrtc_sdp.h index 94008a067f..588e02f139 100644 --- a/pc/webrtc_sdp.h +++ b/pc/webrtc_sdp.h @@ -22,12 +22,17 @@ #include +#include "media/base/codec.h" #include "rtc_base/system/rtc_export.h" namespace cricket { class Candidate; } // namespace cricket +namespace rtc { +class StringBuilder; +} // namespace rtc + namespace webrtc { class IceCandidateInterface; class JsepIceCandidate; @@ -95,6 +100,13 @@ RTC_EXPORT bool ParseCandidate(const std::string& message, SdpParseError* error, bool is_raw); +// Generates an FMTP line based on |parameters|. Please note that some +// parameters are not considered to be part of the FMTP line, see the function +// IsFmtpParam(). Returns true if the set of FMTP parameters is nonempty, false +// otherwise. +bool WriteFmtpParameters(const cricket::CodecParameterMap& parameters, + rtc::StringBuilder* os); + } // namespace webrtc #endif // PC_WEBRTC_SDP_H_