Skip to content

Commit 919f06e

Browse files
paullouisageneaupaullouisageneau
authored
Merge pull request #1168 from paullouisageneau/fix-codecs-priority
Preserve payload types order in description
2 parents 8116593 + 687ff6a commit 919f06e

File tree

3 files changed

+55
-35
lines changed

3 files changed

+55
-35
lines changed

‎include/rtc/description.hpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class RTC_CPP_EXPORT Description {
9090
virtual ~Entry() = default;
9191

9292
virtual string type() const;
93+
virtual string protocol() const;
9394
virtual string description() const;
9495
virtual string mid() const;
9596

@@ -140,6 +141,7 @@ class RTC_CPP_EXPORT Description {
140141

141142
private:
142143
string mType;
144+
string mProtocol;
143145
string mDescription;
144146
string mMid;
145147
std::vector<string> mRids;
@@ -153,7 +155,6 @@ class RTC_CPP_EXPORT Description {
153155
Application(const string &mline, string mid);
154156
virtual ~Application() = default;
155157

156-
string description() const override;
157158
Application reciprocate() const;
158159

159160
void setSctpPort(uint16_t port);
@@ -175,8 +176,8 @@ class RTC_CPP_EXPORT Description {
175176
// Media (non-data)
176177
class RTC_CPP_EXPORT Media : public Entry {
177178
public:
178-
Media(const string &sdp);
179179
Media(const string &mline, string mid, Direction dir = Direction::SendOnly);
180+
Media(const string &sdp);
180181
virtual ~Media() = default;
181182

182183
string description() const override;
@@ -234,6 +235,7 @@ class RTC_CPP_EXPORT Description {
234235

235236
int mBas = -1;
236237

238+
std::vector<int> mOrderedPayloadTypes;
237239
std::map<int, RtpMap> mRtpMaps;
238240
std::vector<uint32_t> mSsrcs;
239241
std::map<uint32_t, string> mCNameMap;

‎src/description.cpp

+48-32
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ void Description::hintType(Type type) {
218218

219219
void Description::setFingerprint(CertificateFingerprint f) {
220220
if (!f.isValid())
221-
throw std::invalid_argument("Invalid " + CertificateFingerprint::AlgorithmIdentifier(f.algorithm) + " fingerprint \"" + f.value + "\"");
221+
throw std::invalid_argument("Invalid " +
222+
CertificateFingerprint::AlgorithmIdentifier(f.algorithm) +
223+
" fingerprint \"" + f.value + "\"");
222224

223225
std::transform(f.value.begin(), f.value.end(), f.value.begin(),
224226
[](char c) { return char(std::toupper(c)); });
@@ -540,9 +542,11 @@ Description::Entry::Entry(const string &mline, string mid, Direction dir)
540542
std::istringstream ss(match_prefix(mline, "m=") ? mline.substr(2) : mline);
541543
ss >> mType;
542544
ss >> port;
543-
ss >> mDescription;
545+
ss >> mProtocol;
546+
ss >> std::ws;
547+
std::getline(ss, mDescription);
544548

545-
if (mType.empty() || mDescription.empty())
549+
if (mType.empty() || mProtocol.empty())
546550
throw std::invalid_argument("Invalid media description line");
547551

548552
// RFC 3264: Existing media streams are removed by creating a new SDP with the port number for
@@ -555,6 +559,8 @@ Description::Entry::Entry(const string &mline, string mid, Direction dir)
555559

556560
string Description::Entry::type() const { return mType; }
557561

562+
string Description::Entry::protocol() const { return mProtocol; }
563+
558564
string Description::Entry::description() const { return mDescription; }
559565

560566
string Description::Entry::mid() const { return mMid; }
@@ -621,7 +627,8 @@ string Description::Entry::generateSdp(string_view eol, string_view addr, uint16
621627
// RFC 3264: Existing media streams are removed by creating a new SDP with the port number for
622628
// that stream set to zero. [...] A stream that is offered with a port of zero MUST be marked
623629
// with port zero in the answer.
624-
sdp << "m=" << type() << ' ' << (mIsRemoved ? 0 : port) << ' ' << description() << eol;
630+
sdp << "m=" << type() << ' ' << (mIsRemoved ? 0 : port) << ' ' << protocol() << ' '
631+
<< description() << eol;
625632
sdp << "c=IN " << addr << eol;
626633
sdp << generateSdpLines(eol);
627634

@@ -825,15 +832,12 @@ optional<string> Description::Media::getCNameForSsrc(uint32_t ssrc) const {
825832
}
826833

827834
Description::Application::Application(string mid)
828-
: Entry("application 9 UDP/DTLS/SCTP", std::move(mid), Direction::SendRecv) {}
835+
: Entry("application 9 UDP/DTLS/SCTP webrtc-datachannel", std::move(mid), Direction::SendRecv) {
836+
}
829837

830838
Description::Application::Application(const string &mline, string mid)
831839
: Entry(mline, std::move(mid), Direction::SendRecv) {}
832840

833-
string Description::Application::description() const {
834-
return Entry::description() + " webrtc-datachannel";
835-
}
836-
837841
Description::Application Description::Application::reciprocate() const {
838842
Application reciprocated(*this);
839843

@@ -882,7 +886,15 @@ void Description::Application::parseSdpLine(string_view line) {
882886
}
883887
}
884888

885-
Description::Media::Media(const string &sdp) : Entry(get_first_line(sdp), "", Direction::Unknown) {
889+
Description::Media::Media(const string &mline, string mid, Direction dir)
890+
: Entry(mline, std::move(mid), dir) {
891+
std::istringstream ss(Entry::description());
892+
int payloadType;
893+
while (ss >> payloadType)
894+
mOrderedPayloadTypes.push_back(payloadType);
895+
}
896+
897+
Description::Media::Media(const string &sdp) : Media(get_first_line(sdp), "", Direction::Unknown) {
886898
string line;
887899
std::istringstream ss(sdp);
888900
std::getline(ss, line); // discard first line
@@ -899,16 +911,16 @@ Description::Media::Media(const string &sdp) : Entry(get_first_line(sdp), "", Di
899911
throw std::invalid_argument("Missing mid in media description");
900912
}
901913

902-
Description::Media::Media(const string &mline, string mid, Direction dir)
903-
: Entry(mline, std::move(mid), dir) {}
904-
905914
string Description::Media::description() const {
906-
std::ostringstream desc;
907-
desc << Entry::description();
908-
for (auto it = mRtpMaps.begin(); it != mRtpMaps.end(); ++it)
909-
desc << ' ' << it->first;
915+
std::ostringstream ss;
916+
for (auto it = mOrderedPayloadTypes.begin(); it != mOrderedPayloadTypes.end(); ++it) {
917+
if (it != mOrderedPayloadTypes.begin())
918+
ss << ' ';
919+
920+
ss << *it;
921+
}
910922

911-
return desc.str();
923+
return ss.str();
912924
}
913925

914926
Description::Media Description::Media::reciprocate() const {
@@ -961,14 +973,7 @@ bool Description::Media::hasPayloadType(int payloadType) const {
961973
return mRtpMaps.find(payloadType) != mRtpMaps.end();
962974
}
963975

964-
std::vector<int> Description::Media::payloadTypes() const {
965-
std::vector<int> result;
966-
result.reserve(mRtpMaps.size());
967-
for (auto it = mRtpMaps.begin(); it != mRtpMaps.end(); ++it)
968-
result.push_back(it->first);
969-
970-
return result;
971-
}
976+
std::vector<int> Description::Media::payloadTypes() const { return mOrderedPayloadTypes; }
972977

973978
Description::Media::RtpMap *Description::Media::rtpMap(int payloadType) {
974979
auto it = mRtpMaps.find(payloadType);
@@ -987,23 +992,34 @@ const Description::Media::RtpMap *Description::Media::rtpMap(int payloadType) co
987992
}
988993

989994
void Description::Media::addRtpMap(RtpMap map) {
990-
auto payloadType = map.payloadType;
995+
int payloadType = map.payloadType;
996+
if (std::find(mOrderedPayloadTypes.begin(), mOrderedPayloadTypes.end(), payloadType) ==
997+
mOrderedPayloadTypes.end())
998+
mOrderedPayloadTypes.push_back(payloadType);
999+
9911000
mRtpMaps.emplace(payloadType, std::move(map));
9921001
}
9931002

9941003
void Description::Media::removeRtpMap(int payloadType) {
9951004
// Remove the actual format
1005+
mOrderedPayloadTypes.erase(
1006+
std::remove(mOrderedPayloadTypes.begin(), mOrderedPayloadTypes.end(), payloadType),
1007+
mOrderedPayloadTypes.end());
9961008
mRtpMaps.erase(payloadType);
9971009

9981010
// Remove any other rtpmaps that depend on the format we just removed
9991011
auto it = mRtpMaps.begin();
10001012
while (it != mRtpMaps.end()) {
10011013
const auto &fmtps = it->second.fmtps;
10021014
if (std::find(fmtps.begin(), fmtps.end(), "apt=" + std::to_string(payloadType)) !=
1003-
fmtps.end())
1015+
fmtps.end()) {
1016+
mOrderedPayloadTypes.erase(
1017+
std::remove(mOrderedPayloadTypes.begin(), mOrderedPayloadTypes.end(), it->first),
1018+
mOrderedPayloadTypes.end());
10041019
it = mRtpMaps.erase(it);
1005-
else
1020+
} else {
10061021
++it;
1022+
}
10071023
}
10081024
}
10091025

@@ -1306,8 +1322,8 @@ CertificateFingerprint::AlgorithmSize(CertificateFingerprint::Algorithm fingerpr
13061322
return 48;
13071323
case CertificateFingerprint::Algorithm::Sha512:
13081324
return 64;
1309-
default:
1310-
return 0;
1325+
default:
1326+
return 0;
13111327
}
13121328
}
13131329

@@ -1325,7 +1341,7 @@ std::string CertificateFingerprint::AlgorithmIdentifier(
13251341
case CertificateFingerprint::Algorithm::Sha512:
13261342
return "sha-512";
13271343
default:
1328-
return "unknown";
1344+
return "unknown";
13291345
}
13301346
}
13311347

‎test/track.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ void test_track() {
9797

9898
const auto mediaSdp1 = string(media);
9999
const auto mediaSdp2 = string(Description::Media(mediaSdp1));
100-
if (mediaSdp2 != mediaSdp1)
100+
if (mediaSdp2 != mediaSdp1) {
101+
cout << mediaSdp2 << endl;
101102
throw runtime_error("Media description parsing test failed");
103+
}
102104

103105
auto t1 = pc1.addTrack(media);
104106

0 commit comments

Comments
 (0)