diff --git a/api/rtpparameters.h b/api/rtpparameters.h index e8ff11623c..12e041939f 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -546,6 +546,7 @@ struct RtpParameters { // Used when calling getParameters/setParameters with a PeerConnection // RtpSender, to ensure that outdated parameters are not unintentionally // applied successfully. + // TODO(deadbeef): Not implemented. std::string transaction_id; // Value to use for MID RTP header extension. diff --git a/api/rtpsenderinterface.h b/api/rtpsenderinterface.h index 8c7e751392..01279a55a0 100644 --- a/api/rtpsenderinterface.h +++ b/api/rtpsenderinterface.h @@ -23,7 +23,6 @@ #include "api/proxy.h" #include "api/rtcerror.h" #include "api/rtpparameters.h" -#include "rtc_base/deprecation.h" #include "rtc_base/refcount.h" #include "rtc_base/scoped_ref_ptr.h" @@ -54,13 +53,7 @@ class RtpSenderInterface : public rtc::RefCountInterface { // tracks. virtual std::vector stream_ids() const = 0; - // TODO(orphis): Transitional implementation - // Remove the const implementation and make the non-const pure virtual once - // when external code depending on this has updated - virtual RtpParameters GetParameters() { return RtpParameters(); } - RTC_DEPRECATED virtual RtpParameters GetParameters() const { - return const_cast(this)->GetParameters(); - } + virtual RtpParameters GetParameters() const = 0; // Note that only a subset of the parameters can currently be changed. See // rtpparameters.h virtual RTCError SetParameters(const RtpParameters& parameters) = 0; @@ -83,7 +76,7 @@ BEGIN_SIGNALING_PROXY_MAP(RtpSender) PROXY_CONSTMETHOD0(cricket::MediaType, media_type) PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(std::vector, stream_ids) - PROXY_METHOD0(RtpParameters, GetParameters); + PROXY_CONSTMETHOD0(RtpParameters, GetParameters); PROXY_METHOD1(RTCError, SetParameters, const RtpParameters&) PROXY_CONSTMETHOD0(rtc::scoped_refptr, GetDtmfSender); END_PROXY_MAP() diff --git a/api/test/mock_rtpsender.h b/api/test/mock_rtpsender.h index 22f391b86c..dda5f45675 100644 --- a/api/test/mock_rtpsender.h +++ b/api/test/mock_rtpsender.h @@ -27,7 +27,7 @@ class MockRtpSender : public rtc::RefCountedObject { MOCK_CONST_METHOD0(media_type, cricket::MediaType()); MOCK_CONST_METHOD0(id, std::string()); MOCK_CONST_METHOD0(stream_ids, std::vector()); - MOCK_METHOD0(GetParameters, RtpParameters()); + MOCK_CONST_METHOD0(GetParameters, RtpParameters()); MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&)); MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr()); }; diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index 3b0bbf886a..5cfbb13637 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -187,15 +187,12 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) { return true; } -RtpParameters AudioRtpSender::GetParameters() { +RtpParameters AudioRtpSender::GetParameters() const { if (!media_channel_ || stopped_) { return RtpParameters(); } return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_); - last_transaction_id_ = rtc::CreateRandomUuid(); - result.transaction_id = last_transaction_id_.value(); - return result; + return media_channel_->GetRtpSendParameters(ssrc_); }); } @@ -204,23 +201,8 @@ RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) { if (!media_channel_ || stopped_) { return RTCError(RTCErrorType::INVALID_STATE); } - if (!last_transaction_id_) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_STATE, - "Failed to set parameters since getParameters() has never been called" - " on this sender"); - } - if (last_transaction_id_ != parameters.transaction_id) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - "Failed to set parameters since the transaction_id doesn't match" - " the last value returned from getParameters()"); - } - return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters); - last_transaction_id_.reset(); - return result; + return media_channel_->SetRtpSendParameters(ssrc_, parameters); }); } @@ -391,15 +373,12 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) { return true; } -RtpParameters VideoRtpSender::GetParameters() { +RtpParameters VideoRtpSender::GetParameters() const { if (!media_channel_ || stopped_) { return RtpParameters(); } return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_); - last_transaction_id_ = rtc::CreateRandomUuid(); - result.transaction_id = last_transaction_id_.value(); - return result; + return media_channel_->GetRtpSendParameters(ssrc_); }); } @@ -408,23 +387,8 @@ RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) { if (!media_channel_ || stopped_) { return RTCError(RTCErrorType::INVALID_STATE); } - if (!last_transaction_id_) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_STATE, - "Failed to set parameters since getParameters() has never been called" - " on this sender"); - } - if (last_transaction_id_ != parameters.transaction_id) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - "Failed to set parameters since the transaction_id doesn't match" - " the last value returned from getParameters()"); - } - return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters); - last_transaction_id_.reset(); - return result; + return media_channel_->SetRtpSendParameters(ssrc_, parameters); }); } diff --git a/pc/rtpsender.h b/pc/rtpsender.h index 4077b9ca1f..ef41485898 100644 --- a/pc/rtpsender.h +++ b/pc/rtpsender.h @@ -128,7 +128,7 @@ class AudioRtpSender : public DtmfProviderInterface, std::vector stream_ids() const override { return stream_ids_; } - RtpParameters GetParameters() override; + RtpParameters GetParameters() const override; RTCError SetParameters(const RtpParameters& parameters) override; rtc::scoped_refptr GetDtmfSender() const override; @@ -172,7 +172,6 @@ class AudioRtpSender : public DtmfProviderInterface, StatsCollector* stats_; rtc::scoped_refptr track_; rtc::scoped_refptr dtmf_sender_proxy_; - rtc::Optional last_transaction_id_; uint32_t ssrc_ = 0; bool cached_track_enabled_ = false; bool stopped_ = false; @@ -217,7 +216,7 @@ class VideoRtpSender : public ObserverInterface, std::vector stream_ids() const override { return stream_ids_; } - RtpParameters GetParameters() override; + RtpParameters GetParameters() const override; RTCError SetParameters(const RtpParameters& parameters) override; rtc::scoped_refptr GetDtmfSender() const override; @@ -254,7 +253,6 @@ class VideoRtpSender : public ObserverInterface, std::vector stream_ids_; cricket::VideoMediaChannel* media_channel_ = nullptr; rtc::scoped_refptr track_; - rtc::Optional last_transaction_id_; uint32_t ssrc_ = 0; VideoTrackInterface::ContentHint cached_track_content_hint_ = VideoTrackInterface::ContentHint::kNone; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index b084b01e1a..d9b543b8c2 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -606,65 +606,6 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParameters) { DestroyAudioRtpSender(); } -TEST_F(RtpSenderReceiverTest, - AudioSenderMustCallGetParametersBeforeSetParameters) { - CreateAudioRtpSender(); - - RtpParameters params; - RTCError result = audio_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type()); - - DestroyAudioRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, - AudioSenderSetParametersInvalidatesTransactionId) { - CreateAudioRtpSender(); - - RtpParameters params = audio_rtp_sender_->GetParameters(); - EXPECT_EQ(1u, params.encodings.size()); - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); - RTCError result = audio_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type()); - - DestroyAudioRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, AudioSenderDetectTransactionIdModification) { - CreateAudioRtpSender(); - - RtpParameters params = audio_rtp_sender_->GetParameters(); - params.transaction_id = ""; - RTCError result = audio_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type()); - - DestroyAudioRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, AudioSenderCheckTransactionIdRefresh) { - CreateAudioRtpSender(); - - RtpParameters params = audio_rtp_sender_->GetParameters(); - EXPECT_NE(params.transaction_id.size(), 0); - auto saved_transaction_id = params.transaction_id; - params = audio_rtp_sender_->GetParameters(); - EXPECT_NE(saved_transaction_id, params.transaction_id); - - DestroyAudioRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, AudioSenderSetParametersOldValueFail) { - CreateAudioRtpSender(); - - RtpParameters params = audio_rtp_sender_->GetParameters(); - RtpParameters second_params = audio_rtp_sender_->GetParameters(); - - RTCError result = audio_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type()); - - DestroyAudioRtpSender(); -} - TEST_F(RtpSenderReceiverTest, SetAudioMaxSendBitrate) { CreateAudioRtpSender(); @@ -723,65 +664,6 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParameters) { DestroyVideoRtpSender(); } -TEST_F(RtpSenderReceiverTest, - VideoSenderMustCallGetParametersBeforeSetParameters) { - CreateVideoRtpSender(); - - RtpParameters params; - RTCError result = video_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type()); - - DestroyVideoRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, - VideoSenderSetParametersInvalidatesTransactionId) { - CreateVideoRtpSender(); - - RtpParameters params = video_rtp_sender_->GetParameters(); - EXPECT_EQ(1u, params.encodings.size()); - EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); - RTCError result = video_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type()); - - DestroyVideoRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, VideoSenderDetectTransactionIdModification) { - CreateVideoRtpSender(); - - RtpParameters params = video_rtp_sender_->GetParameters(); - params.transaction_id = ""; - RTCError result = video_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type()); - - DestroyVideoRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, VideoSenderCheckTransactionIdRefresh) { - CreateVideoRtpSender(); - - RtpParameters params = video_rtp_sender_->GetParameters(); - EXPECT_NE(params.transaction_id.size(), 0); - auto saved_transaction_id = params.transaction_id; - params = video_rtp_sender_->GetParameters(); - EXPECT_NE(saved_transaction_id, params.transaction_id); - - DestroyVideoRtpSender(); -} - -TEST_F(RtpSenderReceiverTest, VideoSenderSetParametersOldValueFail) { - CreateVideoRtpSender(); - - RtpParameters params = video_rtp_sender_->GetParameters(); - RtpParameters second_params = video_rtp_sender_->GetParameters(); - - RTCError result = video_rtp_sender_->SetParameters(params); - EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type()); - - DestroyVideoRtpSender(); -} - TEST_F(RtpSenderReceiverTest, SetVideoMaxSendBitrate) { CreateVideoRtpSender(); diff --git a/pc/test/mock_rtpsenderinternal.h b/pc/test/mock_rtpsenderinternal.h index 3ddffec7a1..5988c7b3c1 100644 --- a/pc/test/mock_rtpsenderinternal.h +++ b/pc/test/mock_rtpsenderinternal.h @@ -29,7 +29,7 @@ class MockRtpSenderInternal : public RtpSenderInternal { MOCK_CONST_METHOD0(media_type, cricket::MediaType()); MOCK_CONST_METHOD0(id, std::string()); MOCK_CONST_METHOD0(stream_ids, std::vector()); - MOCK_METHOD0(GetParameters, RtpParameters()); + MOCK_CONST_METHOD0(GetParameters, RtpParameters()); MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&)); MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr()); diff --git a/sdk/android/api/org/webrtc/RtpParameters.java b/sdk/android/api/org/webrtc/RtpParameters.java index f2227ae53a..634f3b3017 100644 --- a/sdk/android/api/org/webrtc/RtpParameters.java +++ b/sdk/android/api/org/webrtc/RtpParameters.java @@ -109,24 +109,21 @@ public class RtpParameters { } } - public final String transactionId; - public final List encodings; // Codec parameters can't currently be changed between getParameters and // setParameters. Though in the future it will be possible to reorder them or // remove them. public final List codecs; - @CalledByNative - RtpParameters(String transactionId, List encodings, List codecs) { - this.transactionId = transactionId; - this.encodings = encodings; - this.codecs = codecs; + public RtpParameters() { + this.encodings = new ArrayList<>(); + this.codecs = new ArrayList<>(); } @CalledByNative - String getTransactionId() { - return transactionId; + RtpParameters(List encodings, List codecs) { + this.encodings = encodings; + this.codecs = codecs; } @CalledByNative diff --git a/sdk/android/src/jni/pc/rtpparameters.cc b/sdk/android/src/jni/pc/rtpparameters.cc index a990934b01..6a312b4865 100644 --- a/sdk/android/src/jni/pc/rtpparameters.cc +++ b/sdk/android/src/jni/pc/rtpparameters.cc @@ -59,10 +59,6 @@ RtpParameters JavaToNativeRtpParameters(JNIEnv* jni, const JavaRef& j_parameters) { RtpParameters parameters; - ScopedJavaLocalRef j_transaction_id = - Java_RtpParameters_getTransactionId(jni, j_parameters); - parameters.transaction_id = JavaToNativeString(jni, j_transaction_id); - // Convert encodings. ScopedJavaLocalRef j_encodings = Java_RtpParameters_getEncodings(jni, j_parameters); @@ -94,7 +90,7 @@ ScopedJavaLocalRef NativeToJavaRtpParameters( JNIEnv* env, const RtpParameters& parameters) { return Java_RtpParameters_Constructor( - env, NativeToJavaString(env, parameters.transaction_id), + env, NativeToJavaList(env, parameters.encodings, &NativeToJavaRtpEncodingParameter), NativeToJavaList(env, parameters.codecs, &NativeToJavaRtpCodecParameter)); diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpParameters.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpParameters.mm index d18eba6a3d..5e791066df 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpParameters.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpParameters.mm @@ -10,13 +10,11 @@ #import "RTCRtpParameters+Private.h" -#import "NSString+StdString.h" #import "RTCRtpCodecParameters+Private.h" #import "RTCRtpEncodingParameters+Private.h" @implementation RTCRtpParameters -@synthesize transactionId = _transactionId; @synthesize encodings = _encodings; @synthesize codecs = _codecs; @@ -27,7 +25,6 @@ - (instancetype)initWithNativeParameters: (const webrtc::RtpParameters &)nativeParameters { if (self = [self init]) { - _transactionId = [NSString stringForStdString:nativeParameters.transaction_id]; NSMutableArray *encodings = [[NSMutableArray alloc] init]; for (const auto &encoding : nativeParameters.encodings) { [encodings addObject:[[RTCRtpEncodingParameters alloc] @@ -46,8 +43,7 @@ } - (webrtc::RtpParameters)nativeParameters { - webrtc::RtpParameters parameters; - parameters.transaction_id = [NSString stdStringForString:_transactionId]; + webrtc::RtpParameters parameters; for (RTCRtpEncodingParameters *encoding in _encodings) { parameters.encodings.push_back(encoding.nativeParameters); } diff --git a/sdk/objc/Framework/Headers/WebRTC/RTCRtpParameters.h b/sdk/objc/Framework/Headers/WebRTC/RTCRtpParameters.h index b6e1f01a23..bdebf84884 100644 --- a/sdk/objc/Framework/Headers/WebRTC/RTCRtpParameters.h +++ b/sdk/objc/Framework/Headers/WebRTC/RTCRtpParameters.h @@ -19,9 +19,6 @@ NS_ASSUME_NONNULL_BEGIN RTC_EXPORT @interface RTCRtpParameters : NSObject -/** A unique identifier for the last set of parameters applied. */ -@property(nonatomic, copy) NSString *transactionId; - /** The currently active encodings in the order of preference. */ @property(nonatomic, copy) NSArray *encodings;