Reland "Add PT lookup function to JsepTransportController"
This reverts commit 0e3a3266afc50218747134bec7c40f1c6e82ab19.
Reason for revert: Ancestor CL fixed
Original change's description:
> Revert "Add PT lookup function to JsepTransportController"
>
> This reverts commit d178532ff9416f8b4272b9b8622afa9bab2ed558.
>
> Reason for revert: break pw-answer
>
> Original change's description:
> > Add PT lookup function to JsepTransportController
> >
> > Bug: webrtc:360058654
> > Change-Id: I9db58bf872f8659622e9f626fc21ce84993cfdfb
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360143
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Florent Castelli <orphis@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#42829}
>
> Bug: webrtc:360058654
> Change-Id: Ic082dd3e86ed11d05b65710463fa9e57715bf07a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360360
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Jonas Oreland <jonaso@google.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#42832}
Bug: webrtc:360058654
Change-Id: Ice9c118f9a5d4e0fa2cff89f504a25b80ec625ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360662
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#42853}
diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h
index b2c7a75..3c14f13 100644
--- a/pc/jsep_transport.h
+++ b/pc/jsep_transport.h
@@ -242,6 +242,18 @@
webrtc::SdpType type,
const ContentInfo& content);
+ const webrtc::PayloadTypeRecorder& remote_payload_types() const {
+ return remote_payload_types_;
+ }
+ const webrtc::PayloadTypeRecorder& local_payload_types() const {
+ return local_payload_types_;
+ }
+ void CommitPayloadTypes() {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ local_payload_types_.Commit();
+ remote_payload_types_.Commit();
+ }
+
private:
bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source);
diff --git a/pc/jsep_transport_collection.cc b/pc/jsep_transport_collection.cc
index b50d303..08ae513 100644
--- a/pc/jsep_transport_collection.cc
+++ b/pc/jsep_transport_collection.cc
@@ -306,6 +306,9 @@
RTC_DCHECK_RUN_ON(&sequence_checker_);
stable_mid_to_transport_ = mid_to_transport_;
DestroyUnusedTransports();
+ for (auto& transport : jsep_transports_by_name_) {
+ transport.second->CommitPayloadTypes();
+ }
RTC_DCHECK(IsConsistent());
}
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 5d6ac95..7ad8e2d 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -220,6 +220,29 @@
return t->GetDtlsRole();
}
+RTCErrorOr<webrtc::PayloadType> JsepTransportController::SuggestPayloadType(
+ const std::string& mid,
+ cricket::Codec codec) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ const cricket::JsepTransport* transport = GetJsepTransportForMid(mid);
+ if (transport) {
+ auto local_result =
+ transport->local_payload_types().LookupPayloadType(codec);
+ if (local_result.ok()) {
+ return local_result;
+ }
+ auto remote_result =
+ transport->remote_payload_types().LookupPayloadType(codec);
+ if (remote_result.ok()) {
+ return remote_result;
+ }
+ return payload_type_picker_.SuggestMapping(
+ codec, &transport->local_payload_types());
+ }
+ // If there is no transport, there are no exclusions.
+ return payload_type_picker_.SuggestMapping(codec, nullptr);
+}
+
bool JsepTransportController::SetLocalCertificate(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
if (!network_thread_->IsCurrent()) {
diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index 749fadd..c4275bd 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -235,6 +235,13 @@
// Get negotiated role, if one has been negotiated.
absl::optional<rtc::SSLRole> GetDtlsRole(const std::string& mid) const;
+ // Suggest a payload type for a given codec on a given media section.
+ // Media section is indicated by MID.
+ // The function will either return a PT already in use on the connection
+ // or a newly suggested one.
+ RTCErrorOr<PayloadType> SuggestPayloadType(const std::string& mid,
+ cricket::Codec codec);
+
// TODO(deadbeef): GetStats isn't const because all the way down to
// OpenSSLStreamAdapter, GetSslCipherSuite and GetDtlsSrtpCryptoSuite are not
// const. Fix this.
diff --git a/pc/payload_type_picker.cc b/pc/payload_type_picker.cc
index a06e42c..8720b20 100644
--- a/pc/payload_type_picker.cc
+++ b/pc/payload_type_picker.cc
@@ -130,7 +130,7 @@
RTCErrorOr<PayloadType> PayloadTypePicker::SuggestMapping(
cricket::Codec codec,
- PayloadTypeRecorder* excluder) {
+ const PayloadTypeRecorder* excluder) {
// The first matching entry is returned, unless excluder
// maps it to something different.
for (auto entry : entries_) {
@@ -193,12 +193,12 @@
}
std::vector<std::pair<PayloadType, cricket::Codec>>
-PayloadTypeRecorder::GetMappings() {
+PayloadTypeRecorder::GetMappings() const {
return std::vector<std::pair<PayloadType, cricket::Codec>>{};
}
RTCErrorOr<PayloadType> PayloadTypeRecorder::LookupPayloadType(
- cricket::Codec codec) {
+ cricket::Codec codec) const {
// Note that having multiple PTs mapping to the same codec is NOT an error.
// In this case, we return the first found (not deterministic).
auto result = std::find_if(
@@ -212,7 +212,7 @@
}
RTCErrorOr<cricket::Codec> PayloadTypeRecorder::LookupCodec(
- PayloadType payload_type) {
+ PayloadType payload_type) const {
auto result = payload_type_to_codec_.find(payload_type);
if (result == payload_type_to_codec_.end()) {
return RTCError(RTCErrorType::INVALID_PARAMETER, "No such payload type");
@@ -220,7 +220,7 @@
return result->second;
}
-void PayloadTypeRecorder::Checkpoint() {
+void PayloadTypeRecorder::Commit() {
checkpoint_payload_type_to_codec_ = payload_type_to_codec_;
}
void PayloadTypeRecorder::Rollback() {
diff --git a/pc/payload_type_picker.h b/pc/payload_type_picker.h
index f299797..4726a31 100644
--- a/pc/payload_type_picker.h
+++ b/pc/payload_type_picker.h
@@ -42,7 +42,7 @@
// Suggest a payload type for the codec.
// If the excluder maps it to something different, don't suggest it.
RTCErrorOr<PayloadType> SuggestMapping(cricket::Codec codec,
- PayloadTypeRecorder* excluder);
+ const PayloadTypeRecorder* excluder);
RTCError AddMapping(PayloadType payload_type, cricket::Codec codec);
private:
@@ -67,12 +67,12 @@
: suggester_(suggester) {}
RTCError AddMapping(PayloadType payload_type, cricket::Codec codec);
- std::vector<std::pair<PayloadType, cricket::Codec>> GetMappings();
- RTCErrorOr<PayloadType> LookupPayloadType(cricket::Codec codec);
- RTCErrorOr<cricket::Codec> LookupCodec(PayloadType payload_type);
+ std::vector<std::pair<PayloadType, cricket::Codec>> GetMappings() const;
+ RTCErrorOr<PayloadType> LookupPayloadType(cricket::Codec codec) const;
+ RTCErrorOr<cricket::Codec> LookupCodec(PayloadType payload_type) const;
// Transaction support.
- // Checkpoint() commits previous changes.
- void Checkpoint();
+ // Commit() commits previous changes.
+ void Commit();
// Rollback() rolls back to the previous checkpoint.
void Rollback();
diff --git a/pc/payload_type_picker_unittest.cc b/pc/payload_type_picker_unittest.cc
index e97a2b0..71a3383 100644
--- a/pc/payload_type_picker_unittest.cc
+++ b/pc/payload_type_picker_unittest.cc
@@ -75,7 +75,7 @@
cricket::Codec b_codec = cricket::CreateVideoCodec(0, "vp9");
auto error = recorder.AddMapping(a_payload_type, a_codec);
ASSERT_TRUE(error.ok());
- recorder.Checkpoint();
+ recorder.Commit();
ASSERT_TRUE(recorder.AddMapping(b_payload_type, b_codec).ok());
{
auto result = recorder.LookupCodec(a_payload_type);
@@ -99,7 +99,7 @@
}
ASSERT_TRUE(recorder.AddMapping(b_payload_type, b_codec).ok());
// Rollback after a new checkpoint has no effect.
- recorder.Checkpoint();
+ recorder.Commit();
recorder.Rollback();
{
auto result = recorder.LookupCodec(b_payload_type);
