Optimize FindCodecById and ReferencedCodecsMatch
These functions currently copy cricket::Codec classes by value which is
expensive since they contain e.g. std::map<std::string, std::string>
containers with parameters. This CL avoids copying them altogether.
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2493733003
Cr-Commit-Position: refs/heads/master@{#15040}
diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc
index b7ef28e..d78112c 100644
--- a/webrtc/api/webrtcsdp.cc
+++ b/webrtc/api/webrtcsdp.cc
@@ -2438,10 +2438,12 @@
// with that payload type.
template <class T>
T GetCodecWithPayloadType(const std::vector<T>& codecs, int payload_type) {
+ const T* codec = FindCodecById(codecs, payload_type);
+ if (codec)
+ return *codec;
+ // Return empty codec with |payload_type|.
T ret_val;
- if (!FindCodecById(codecs, payload_type, &ret_val)) {
- ret_val.id = payload_type;
- }
+ ret_val.id = payload_type;
return ret_val;
}
diff --git a/webrtc/media/base/codec.h b/webrtc/media/base/codec.h
index bfbff23..3bd93a9 100644
--- a/webrtc/media/base/codec.h
+++ b/webrtc/media/base/codec.h
@@ -197,18 +197,14 @@
};
// Get the codec setting associated with |payload_type|. If there
-// is no codec associated with that payload type it returns false.
+// is no codec associated with that payload type it returns nullptr.
template <class Codec>
-bool FindCodecById(const std::vector<Codec>& codecs,
- int payload_type,
- Codec* codec_out) {
+const Codec* FindCodecById(const std::vector<Codec>& codecs, int payload_type) {
for (const auto& codec : codecs) {
- if (codec.id == payload_type) {
- *codec_out = codec;
- return true;
- }
+ if (codec.id == payload_type)
+ return &codec;
}
- return false;
+ return nullptr;
}
bool CodecNamesEq(const std::string& name1, const std::string& name2);
diff --git a/webrtc/media/base/rtpdataengine.cc b/webrtc/media/base/rtpdataengine.cc
index 6c3837d..e21aa00 100644
--- a/webrtc/media/base/rtpdataengine.cc
+++ b/webrtc/media/base/rtpdataengine.cc
@@ -225,8 +225,7 @@
return;
}
- DataCodec codec;
- if (!FindCodecById(recv_codecs_, header.payload_type, &codec)) {
+ if (!FindCodecById(recv_codecs_, header.payload_type)) {
// For bundling, this will be logged for every message.
// So disable this logging.
// LOG(LS_WARNING) << "Not receiving packet "
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index ebba9ac..cc93a8b 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -752,20 +752,12 @@
template <class C>
static bool ReferencedCodecsMatch(const std::vector<C>& codecs1,
- const std::string& codec1_id_str,
+ const int codec1_id,
const std::vector<C>& codecs2,
- const std::string& codec2_id_str) {
- int codec1_id;
- int codec2_id;
- C codec1;
- C codec2;
- if (!rtc::FromString(codec1_id_str, &codec1_id) ||
- !rtc::FromString(codec2_id_str, &codec2_id) ||
- !FindCodecById(codecs1, codec1_id, &codec1) ||
- !FindCodecById(codecs2, codec2_id, &codec2)) {
- return false;
- }
- return codec1.Matches(codec2);
+ const int codec2_id) {
+ const C* codec1 = FindCodecById(codecs1, codec1_id);
+ const C* codec2 = FindCodecById(codecs2, codec2_id);
+ return codec1 != nullptr && codec2 != nullptr && codec1->Matches(*codec2);
}
template <class C>
@@ -780,16 +772,15 @@
C negotiated = ours;
negotiated.IntersectFeedbackParams(theirs);
if (IsRtxCodec(negotiated)) {
- std::string offered_apt_value;
- theirs.GetParam(kCodecParamAssociatedPayloadType, &offered_apt_value);
+ const auto apt_it =
+ theirs.params.find(kCodecParamAssociatedPayloadType);
// FindMatchingCodec shouldn't return something with no apt value.
- RTC_DCHECK(!offered_apt_value.empty());
- negotiated.SetParam(kCodecParamAssociatedPayloadType,
- offered_apt_value);
+ RTC_DCHECK(apt_it != theirs.params.end());
+ negotiated.SetParam(kCodecParamAssociatedPayloadType, apt_it->second);
}
negotiated.id = theirs.id;
negotiated.name = theirs.name;
- negotiated_codecs->push_back(negotiated);
+ negotiated_codecs->push_back(std::move(negotiated));
}
}
// RFC3264: Although the answerer MAY list the formats in their desired
@@ -819,8 +810,8 @@
for (const C& potential_match : codecs2) {
if (potential_match.Matches(codec_to_match)) {
if (IsRtxCodec(codec_to_match)) {
- std::string apt_value_1;
- std::string apt_value_2;
+ int apt_value_1 = 0;
+ int apt_value_2 = 0;
if (!codec_to_match.GetParam(kCodecParamAssociatedPayloadType,
&apt_value_1) ||
!potential_match.GetParam(kCodecParamAssociatedPayloadType,
@@ -886,8 +877,9 @@
}
// Find the associated reference codec for the reference RTX codec.
- C associated_codec;
- if (!FindCodecById(reference_codecs, associated_pt, &associated_codec)) {
+ const C* associated_codec =
+ FindCodecById(reference_codecs, associated_pt);
+ if (!associated_codec) {
LOG(LS_WARNING) << "Couldn't find associated codec with payload type "
<< associated_pt << " for RTX codec " << rtx_codec.name
<< ".";
@@ -898,8 +890,8 @@
// Its payload type may be different than the reference codec.
C matching_codec;
if (!FindMatchingCodec<C>(reference_codecs, *offered_codecs,
- associated_codec, &matching_codec)) {
- LOG(LS_WARNING) << "Couldn't find matching " << associated_codec.name
+ *associated_codec, &matching_codec)) {
+ LOG(LS_WARNING) << "Couldn't find matching " << associated_codec->name
<< " codec.";
continue;
}
diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc
index f23e070..ca1e4b39 100644
--- a/webrtc/pc/mediasession_unittest.cc
+++ b/webrtc/pc/mediasession_unittest.cc
@@ -192,8 +192,7 @@
static void AddRtxCodec(const VideoCodec& rtx_codec,
std::vector<VideoCodec>* codecs) {
- VideoCodec rtx;
- ASSERT_FALSE(cricket::FindCodecById(*codecs, rtx_codec.id, &rtx));
+ ASSERT_FALSE(cricket::FindCodecById(*codecs, rtx_codec.id));
codecs->push_back(rtx_codec);
}
