Move codecs() to MediaContentDescription
allowing for a lot of de-templating
BUG=webrtc:15214
Change-Id: Ibe1a5f5d704564566f24c496822a4308ba23c4dd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319160
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40774}
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index ed2dfca..fe7093d 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -438,18 +438,9 @@
continue;
}
const auto type = media_description->type();
- if (type == cricket::MEDIA_TYPE_AUDIO) {
- RTC_DCHECK(media_description->as_audio());
- for (const auto& c : media_description->as_audio()->codecs()) {
- auto error = FindDuplicateCodecParameters(
- c.ToCodecParameters(), payload_to_codec_parameters);
- if (!error.ok()) {
- return error;
- }
- }
- } else if (type == cricket::MEDIA_TYPE_VIDEO) {
- RTC_DCHECK(media_description->as_video());
- for (const auto& c : media_description->as_video()->codecs()) {
+ if (type == cricket::MEDIA_TYPE_AUDIO ||
+ type == cricket::MEDIA_TYPE_VIDEO) {
+ for (const auto& c : media_description->codecs()) {
auto error = FindDuplicateCodecParameters(
c.ToCodecParameters(), payload_to_codec_parameters);
if (!error.ok()) {
diff --git a/pc/session_description.h b/pc/session_description.h
index 31992be..52e891e 100644
--- a/pc/session_description.h
+++ b/pc/session_description.h
@@ -83,8 +83,6 @@
return nullptr;
}
- virtual bool has_codecs() const = 0;
-
// Copy operator that returns an unique_ptr.
// Not a virtual function.
// If a type-specific variant of Clone() is desired, override it, or
@@ -234,54 +232,14 @@
receive_rids_ = rids;
}
- protected:
- bool rtcp_mux_ = false;
- bool rtcp_reduced_size_ = false;
- bool remote_estimate_ = false;
- int bandwidth_ = kAutoBandwidth;
- std::string bandwidth_type_ = kApplicationSpecificBandwidth;
- std::string protocol_;
- std::vector<CryptoParams> cryptos_;
- std::vector<webrtc::RtpExtension> rtp_header_extensions_;
- bool rtp_header_extensions_set_ = false;
- StreamParamsVec send_streams_;
- bool conference_mode_ = false;
- webrtc::RtpTransceiverDirection direction_ =
- webrtc::RtpTransceiverDirection::kSendRecv;
- rtc::SocketAddress connection_address_;
- ExtmapAllowMixed extmap_allow_mixed_enum_ = kMedia;
-
- SimulcastDescription simulcast_;
- std::vector<RidDescription> receive_rids_;
-
- private:
- // Copy function that returns a raw pointer. Caller will assert ownership.
- // Should only be called by the Clone() function. Must be implemented
- // by each final subclass.
- virtual MediaContentDescription* CloneInternal() const = 0;
-};
-
-template <class C>
-class MediaContentDescriptionImpl : public MediaContentDescription {
- public:
- void set_protocol(absl::string_view protocol) override {
- RTC_DCHECK(IsRtpProtocol(protocol));
- protocol_ = std::string(protocol);
- }
-
// Codecs should be in preference order (most preferred codec first).
const std::vector<Codec>& codecs() const { return codecs_; }
void set_codecs(const std::vector<Codec>& codecs) { codecs_ = codecs; }
- bool has_codecs() const override { return !codecs_.empty(); }
+ virtual bool has_codecs() const { return !codecs_.empty(); }
bool HasCodec(int id) {
- bool found = false;
- for (auto it = codecs_.begin(); it != codecs_.end(); ++it) {
- if (it->id == id) {
- found = true;
- break;
- }
- }
- return found;
+ return absl::c_find_if(codecs_, [id](const cricket::Codec codec) {
+ return codec.id == id;
+ }) != codecs_.end();
}
void AddCodec(const Codec& codec) { codecs_.push_back(codec); }
void AddOrReplaceCodec(const Codec& codec) {
@@ -299,10 +257,48 @@
}
}
+ protected:
+ // TODO(bugs.webrtc.org/15214): move all RTP related things to a subclass that
+ // the SCTP content description does not inherit from.
+ std::string protocol_;
+
private:
+ bool rtcp_mux_ = false;
+ bool rtcp_reduced_size_ = false;
+ bool remote_estimate_ = false;
+ int bandwidth_ = kAutoBandwidth;
+ std::string bandwidth_type_ = kApplicationSpecificBandwidth;
+
+ std::vector<CryptoParams> cryptos_;
+ std::vector<webrtc::RtpExtension> rtp_header_extensions_;
+ bool rtp_header_extensions_set_ = false;
+ StreamParamsVec send_streams_;
+ bool conference_mode_ = false;
+ webrtc::RtpTransceiverDirection direction_ =
+ webrtc::RtpTransceiverDirection::kSendRecv;
+ rtc::SocketAddress connection_address_;
+ ExtmapAllowMixed extmap_allow_mixed_enum_ = kMedia;
+
+ SimulcastDescription simulcast_;
+ std::vector<RidDescription> receive_rids_;
+
+ // Copy function that returns a raw pointer. Caller will assert ownership.
+ // Should only be called by the Clone() function. Must be implemented
+ // by each final subclass.
+ virtual MediaContentDescription* CloneInternal() const = 0;
+
std::vector<Codec> codecs_;
};
+template <class C>
+class MediaContentDescriptionImpl : public MediaContentDescription {
+ public:
+ void set_protocol(absl::string_view protocol) override {
+ RTC_DCHECK(IsRtpProtocol(protocol));
+ protocol_ = std::string(protocol);
+ }
+};
+
class AudioContentDescription : public MediaContentDescriptionImpl<Codec> {
public:
AudioContentDescription() {}
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 9ed8ca6..80fef12 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -1438,17 +1438,11 @@
// fmt is a list of payload type numbers that MAY be used in the session.
std::string type;
std::string fmt;
- if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- type = kMediaTypeVideo;
- const VideoContentDescription* video_desc = media_desc->as_video();
- for (const cricket::VideoCodec& codec : video_desc->codecs()) {
- fmt.append(" ");
- fmt.append(rtc::ToString(codec.id));
- }
- } else if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- type = kMediaTypeAudio;
- const AudioContentDescription* audio_desc = media_desc->as_audio();
- for (const cricket::AudioCodec& codec : audio_desc->codecs()) {
+ if (media_type == cricket::MEDIA_TYPE_AUDIO ||
+ media_type == cricket::MEDIA_TYPE_VIDEO) {
+ type = media_type == cricket::MEDIA_TYPE_AUDIO ? kMediaTypeAudio
+ : kMediaTypeVideo;
+ for (const cricket::VideoCodec& codec : media_desc->codecs()) {
fmt.append(" ");
fmt.append(rtc::ToString(codec.id));
}
@@ -1867,8 +1861,7 @@
return !empty;
}
-template <class T>
-void AddFmtpLine(const T& codec, std::string* message) {
+void AddFmtpLine(const cricket::Codec& codec, std::string* message) {
rtc::StringBuilder os;
WriteFmtpHeader(codec.id, &os);
os << kSdpDelimiterSpace;
@@ -1879,8 +1872,7 @@
return;
}
-template <class T>
-void AddPacketizationLine(const T& codec, std::string* message) {
+void AddPacketizationLine(const cricket::Codec& codec, std::string* message) {
if (!codec.packetization) {
return;
}
@@ -1890,8 +1882,7 @@
AddLine(os.str(), message);
}
-template <class T>
-void AddRtcpFbLines(const T& codec, std::string* message) {
+void AddRtcpFbLines(const cricket::Codec& codec, std::string* message) {
for (const cricket::FeedbackParam& param : codec.feedback_params.params()) {
rtc::StringBuilder os;
WriteRtcpFbHeader(codec.id, &os);
@@ -1932,7 +1923,7 @@
RTC_DCHECK(media_desc != NULL);
rtc::StringBuilder os;
if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- for (const cricket::VideoCodec& codec : media_desc->as_video()->codecs()) {
+ for (const cricket::Codec& codec : media_desc->codecs()) {
// RFC 4566
// a=rtpmap:<payload type> <encoding name>/<clock rate>
// [/<encodingparameters>]
@@ -1950,7 +1941,7 @@
std::vector<int> ptimes;
std::vector<int> maxptimes;
int max_minptime = 0;
- for (const cricket::AudioCodec& codec : media_desc->as_audio()->codecs()) {
+ for (const cricket::Codec& codec : media_desc->codecs()) {
RTC_DCHECK(!codec.name.empty());
// RFC 4566
// a=rtpmap:<payload type> <encoding name>/<clock rate>
@@ -2857,21 +2848,6 @@
return true;
}
-bool VerifyCodec(const cricket::Codec& codec) {
- // Codec has not been populated correctly unless the name has been set. This
- // can happen if an SDP has an fmtp or rtcp-fb with a payload type but doesn't
- // have a corresponding "rtpmap" line.
- return !codec.name.empty();
-}
-
-bool VerifyAudioCodecs(const AudioContentDescription* audio_desc) {
- return absl::c_all_of(audio_desc->codecs(), &VerifyCodec);
-}
-
-bool VerifyVideoCodecs(const VideoContentDescription* video_desc) {
- return absl::c_all_of(video_desc->codecs(), &VerifyCodec);
-}
-
void AddParameters(const cricket::CodecParameterMap& parameters,
cricket::Codec* codec) {
for (const auto& entry : parameters) {
@@ -2896,11 +2872,11 @@
// Gets the current codec setting associated with `payload_type`. If there
// is no Codec associated with that payload type it returns an empty codec
// with that payload type.
-template <class T>
-T GetCodecWithPayloadType(cricket::MediaType type,
- const std::vector<T>& codecs,
- int payload_type) {
- const T* codec = FindCodecById(codecs, payload_type);
+cricket::Codec GetCodecWithPayloadType(
+ cricket::MediaType type,
+ const std::vector<cricket::Codec>& codecs,
+ int payload_type) {
+ const cricket::Codec* codec = FindCodecById(codecs, payload_type);
if (codec)
return *codec;
// Return empty codec with `payload_type`.
@@ -2912,12 +2888,11 @@
}
// Updates or creates a new codec entry in the media description.
-template <class T, class U>
-void AddOrReplaceCodec(MediaContentDescription* content_desc, const U& codec) {
- T* desc = static_cast<T*>(content_desc);
- std::vector<U> codecs = desc->codecs();
+void AddOrReplaceCodec(MediaContentDescription* content_desc,
+ const cricket::Codec& codec) {
+ std::vector<cricket::Codec> codecs = content_desc->codecs();
bool found = false;
- for (U& existing_codec : codecs) {
+ for (cricket::Codec& existing_codec : codecs) {
if (codec.id == existing_codec.id) {
// Overwrite existing codec with the new codec.
existing_codec = codec;
@@ -2926,38 +2901,34 @@
}
}
if (!found) {
- desc->AddCodec(codec);
+ content_desc->AddCodec(codec);
return;
}
- desc->set_codecs(codecs);
+ content_desc->set_codecs(codecs);
}
// Adds or updates existing codec corresponding to `payload_type` according
// to `parameters`.
-template <class T, class U>
void UpdateCodec(MediaContentDescription* content_desc,
int payload_type,
const cricket::CodecParameterMap& parameters) {
// Codec might already have been populated (from rtpmap).
- U new_codec = GetCodecWithPayloadType(content_desc->type(),
- static_cast<T*>(content_desc)->codecs(),
- payload_type);
+ cricket::Codec new_codec = GetCodecWithPayloadType(
+ content_desc->type(), content_desc->codecs(), payload_type);
AddParameters(parameters, &new_codec);
- AddOrReplaceCodec<T, U>(content_desc, new_codec);
+ AddOrReplaceCodec(content_desc, new_codec);
}
// Adds or updates existing codec corresponding to `payload_type` according
// to `feedback_param`.
-template <class T, class U>
void UpdateCodec(MediaContentDescription* content_desc,
int payload_type,
const cricket::FeedbackParam& feedback_param) {
// Codec might already have been populated (from rtpmap).
- U new_codec = GetCodecWithPayloadType(content_desc->type(),
- static_cast<T*>(content_desc)->codecs(),
- payload_type);
+ cricket::Codec new_codec = GetCodecWithPayloadType(
+ content_desc->type(), content_desc->codecs(), payload_type);
AddFeedbackParameter(feedback_param, &new_codec);
- AddOrReplaceCodec<T, U>(content_desc, new_codec);
+ AddOrReplaceCodec(content_desc, new_codec);
}
// Adds or updates existing video codec corresponding to `payload_type`
@@ -2974,15 +2945,15 @@
cricket::VideoCodec codec = GetCodecWithPayloadType(
video_desc->type(), video_desc->codecs(), payload_type);
codec.packetization = std::string(packetization);
- AddOrReplaceCodec<VideoContentDescription, cricket::VideoCodec>(video_desc,
- codec);
+ AddOrReplaceCodec(video_desc, codec);
}
-template <class T>
-absl::optional<T> PopWildcardCodec(std::vector<T>* codecs) {
+absl::optional<cricket::Codec> PopWildcardCodec(
+ std::vector<cricket::Codec>* codecs) {
+ RTC_DCHECK(codecs);
for (auto iter = codecs->begin(); iter != codecs->end(); ++iter) {
if (iter->id == kWildcardPayloadType) {
- T wildcard_codec = *iter;
+ cricket::Codec wildcard_codec = *iter;
codecs->erase(iter);
return wildcard_codec;
}
@@ -2990,10 +2961,10 @@
return absl::nullopt;
}
-template <class T>
-void UpdateFromWildcardCodecs(cricket::MediaContentDescriptionImpl<T>* desc) {
+void UpdateFromWildcardCodecs(cricket::MediaContentDescription* desc) {
+ RTC_DCHECK(desc);
auto codecs = desc->codecs();
- absl::optional<T> wildcard_codec = PopWildcardCodec(&codecs);
+ absl::optional<cricket::Codec> wildcard_codec = PopWildcardCodec(&codecs);
if (!wildcard_codec) {
return;
}
@@ -3006,6 +2977,7 @@
void AddAudioAttribute(const std::string& name,
absl::string_view value,
AudioContentDescription* audio_desc) {
+ RTC_DCHECK(audio_desc);
if (value.empty()) {
return;
}
@@ -3415,27 +3387,20 @@
media_desc->AddStream(track);
}
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- AudioContentDescription* audio_desc = media_desc->as_audio();
- UpdateFromWildcardCodecs(audio_desc);
-
- // Verify audio codec ensures that no audio codec has been populated with
- // only fmtp.
- if (!VerifyAudioCodecs(audio_desc)) {
- return ParseFailed("Failed to parse audio codecs correctly.", error);
- }
- AddAudioAttribute(kCodecParamMaxPTime, maxptime_as_string, audio_desc);
- AddAudioAttribute(kCodecParamPTime, ptime_as_string, audio_desc);
+ UpdateFromWildcardCodecs(media_desc);
+ // Codec has not been populated correctly unless the name has been set. This
+ // can happen if an SDP has an fmtp or rtcp-fb with a payload type but doesn't
+ // have a corresponding "rtpmap" line. This should lead to a parse error.
+ if (!absl::c_all_of(media_desc->codecs(), [](const cricket::Codec codec) {
+ return !codec.name.empty();
+ })) {
+ return ParseFailed("Failed to parse codecs correctly.", error);
}
-
- if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- VideoContentDescription* video_desc = media_desc->as_video();
- UpdateFromWildcardCodecs(video_desc);
- // Verify video codec ensures that no video codec has been populated with
- // only rtcp-fb.
- if (!VerifyVideoCodecs(video_desc)) {
- return ParseFailed("Failed to parse video codecs correctly.", error);
- }
+ if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+ AddAudioAttribute(kCodecParamMaxPTime, maxptime_as_string,
+ media_desc->as_audio());
+ AddAudioAttribute(kCodecParamPTime, ptime_as_string,
+ media_desc->as_audio());
}
// RFC 5245
@@ -3601,14 +3566,13 @@
AudioContentDescription* audio_desc) {
// Codec may already be populated with (only) optional parameters
// (from an fmtp).
- cricket::AudioCodec codec = GetCodecWithPayloadType(
+ cricket::Codec codec = GetCodecWithPayloadType(
audio_desc->type(), audio_desc->codecs(), payload_type);
codec.name = std::string(name);
codec.clockrate = clockrate;
codec.bitrate = bitrate;
codec.channels = channels;
- AddOrReplaceCodec<AudioContentDescription, cricket::AudioCodec>(audio_desc,
- codec);
+ AddOrReplaceCodec(audio_desc, codec);
}
// Updates or creates a new codec entry in the video description according to
@@ -3618,11 +3582,10 @@
VideoContentDescription* video_desc) {
// Codec may already be populated with (only) optional parameters
// (from an fmtp).
- cricket::VideoCodec codec = GetCodecWithPayloadType(
+ cricket::Codec codec = GetCodecWithPayloadType(
video_desc->type(), video_desc->codecs(), payload_type);
codec.name = std::string(name);
- AddOrReplaceCodec<VideoContentDescription, cricket::VideoCodec>(video_desc,
- codec);
+ AddOrReplaceCodec(video_desc, codec);
}
bool ParseRtpmapAttribute(absl::string_view line,
@@ -3671,8 +3634,7 @@
}
if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- VideoContentDescription* video_desc = media_desc->as_video();
- for (const cricket::VideoCodec& existing_codec : video_desc->codecs()) {
+ for (const cricket::VideoCodec& existing_codec : media_desc->codecs()) {
if (!existing_codec.name.empty() && payload_type == existing_codec.id &&
(!absl::EqualsIgnoreCase(encoding_name, existing_codec.name) ||
clock_rate != existing_codec.clockrate)) {
@@ -3686,7 +3648,7 @@
return ParseFailed(line, description.Release(), error);
}
}
- UpdateCodec(payload_type, encoding_name, video_desc);
+ UpdateCodec(payload_type, encoding_name, media_desc->as_video());
} else if (media_type == cricket::MEDIA_TYPE_AUDIO) {
// RFC 4566
// For audio streams, <encoding parameters> indicates the number
@@ -3703,8 +3665,7 @@
return ParseFailed(line, "At most 24 channels are supported.", error);
}
- AudioContentDescription* audio_desc = media_desc->as_audio();
- for (const cricket::AudioCodec& existing_codec : audio_desc->codecs()) {
+ for (const cricket::AudioCodec& existing_codec : media_desc->codecs()) {
// TODO(crbug.com/1338902) re-add checks for clockrate and number of
// channels.
if (!existing_codec.name.empty() && payload_type == existing_codec.id &&
@@ -3720,7 +3681,7 @@
}
}
UpdateCodec(payload_type, encoding_name, clock_rate, 0, channels,
- audio_desc);
+ media_desc->as_audio());
}
return true;
}
@@ -3791,12 +3752,9 @@
codec_params[name] = value;
}
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- UpdateCodec<AudioContentDescription, cricket::AudioCodec>(
- media_desc, payload_type, codec_params);
- } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- UpdateCodec<VideoContentDescription, cricket::VideoCodec>(
- media_desc, payload_type, codec_params);
+ if (media_type == cricket::MEDIA_TYPE_AUDIO ||
+ media_type == cricket::MEDIA_TYPE_VIDEO) {
+ UpdateCodec(media_desc, payload_type, codec_params);
}
return true;
}
@@ -3862,12 +3820,9 @@
}
const cricket::FeedbackParam feedback_param(id, param);
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- UpdateCodec<AudioContentDescription, cricket::AudioCodec>(
- media_desc, payload_type, feedback_param);
- } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- UpdateCodec<VideoContentDescription, cricket::VideoCodec>(
- media_desc, payload_type, feedback_param);
+ if (media_type == cricket::MEDIA_TYPE_AUDIO ||
+ media_type == cricket::MEDIA_TYPE_VIDEO) {
+ UpdateCodec(media_desc, payload_type, feedback_param);
}
return true;
}
