From b05fa2466a21ad0dfb4c750ebbb260675bce592f Mon Sep 17 00:00:00 2001 From: magjed Date: Fri, 11 Nov 2016 04:00:16 -0800 Subject: [PATCH] Optimize FindCodecById and ReferencedCodecsMatch These functions currently copy cricket::Codec classes by value which is expensive since they contain e.g. std::map 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} --- webrtc/api/webrtcsdp.cc | 8 +++--- webrtc/media/base/codec.h | 14 ++++------ webrtc/media/base/rtpdataengine.cc | 3 +-- webrtc/pc/mediasession.cc | 42 ++++++++++++------------------ webrtc/pc/mediasession_unittest.cc | 3 +-- 5 files changed, 29 insertions(+), 41 deletions(-) diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index b7ef28e38d..d78112c168 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -2438,10 +2438,12 @@ void AddFeedbackParameters(const cricket::FeedbackParams& feedback_params, // with that payload type. template T GetCodecWithPayloadType(const std::vector& 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 bfbff23c94..3bd93a9247 100644 --- a/webrtc/media/base/codec.h +++ b/webrtc/media/base/codec.h @@ -197,18 +197,14 @@ struct DataCodec : public Codec { }; // 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 -bool FindCodecById(const std::vector& codecs, - int payload_type, - Codec* codec_out) { +const Codec* FindCodecById(const std::vector& 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 6c3837d70d..e21aa00154 100644 --- a/webrtc/media/base/rtpdataengine.cc +++ b/webrtc/media/base/rtpdataengine.cc @@ -225,8 +225,7 @@ void RtpDataMediaChannel::OnPacketReceived( 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 ebba9acbcb..cc93a8b170 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -752,20 +752,12 @@ static bool CreateMediaContentOffer( template static bool ReferencedCodecsMatch(const std::vector& codecs1, - const std::string& codec1_id_str, + const int codec1_id, const std::vector& 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 @@ -780,16 +772,15 @@ static void NegotiateCodecs(const std::vector& local_codecs, 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 @@ static bool FindMatchingCodec(const std::vector& codecs1, 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 @@ static void FindCodecsToOffer( } // 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 @@ static void FindCodecsToOffer( // Its payload type may be different than the reference codec. C matching_codec; if (!FindMatchingCodec(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 f23e070797..ca1e4b3939 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -192,8 +192,7 @@ GetMediaDirection(const ContentInfo* content) { static void AddRtxCodec(const VideoCodec& rtx_codec, std::vector* 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); }