Revert "Implement RtpParameters.transaction_id for PC RtpSenderInterface"

This reverts commit 5faf36ef3c.

Reason for revert: fast/peerconnection/RTCRtpSender-setParameters.html
 failing in webrtc roll, probably this CL? https://chromium-review.googlesource.com/c/chromium/src/+/1045889.

Original change's description:
> Implement RtpParameters.transaction_id for PC RtpSenderInterface
> 
> The transaction_id field should be refreshed for every getParameters()
> call and checked at each setParameters() call.
> This also checks that getParameters() was ever called to return a proper
> error code.
> 
> Bug: webrtc:7580
> Change-Id: I6c6fe289542e486fc422cdc61577982b0529d4c1
> Reviewed-on: https://webrtc-review.googlesource.com/70820
> Commit-Queue: Florent Castelli <orphis@webrtc.org>
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23120}

TBR=steveanton@webrtc.org,deadbeef@webrtc.org,sakal@webrtc.org,kthelgason@webrtc.org,orphis@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:7580
Change-Id: I86da108227f8fc8d235bb2e9559377c800595b8c
Reviewed-on: https://webrtc-review.googlesource.com/74740
Reviewed-by: Max Morin <maxmorin@webrtc.org>
Commit-Queue: Max Morin <maxmorin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23134}

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.

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<std::string> stream_ids() const = 0;
 
-  // TODO(orphis): Transitional implementation
+  virtual RtpParameters GetParameters() const = 0;
-  // 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<RtpSenderInterface*>(this)->GetParameters();
-  }
   // 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<std::string>, stream_ids)
-  PROXY_METHOD0(RtpParameters, GetParameters);
+  PROXY_CONSTMETHOD0(RtpParameters, GetParameters);
   PROXY_METHOD1(RTCError, SetParameters, const RtpParameters&)
   PROXY_CONSTMETHOD0(rtc::scoped_refptr<DtmfSenderInterface>, GetDtmfSender);
   END_PROXY_MAP()

api/test/mock_rtpsender.h

@@ -27,7 +27,7 @@ class MockRtpSender : public rtc::RefCountedObject<RtpSenderInterface> {
   MOCK_CONST_METHOD0(media_type, cricket::MediaType());
   MOCK_CONST_METHOD0(id, std::string());
   MOCK_CONST_METHOD0(stream_ids, std::vector<std::string>());
-  MOCK_METHOD0(GetParameters, RtpParameters());
+  MOCK_CONST_METHOD0(GetParameters, RtpParameters());
   MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&));
   MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr<DtmfSenderInterface>());
 };

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<RtpParameters>(RTC_FROM_HERE, [&] {
-    RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_);
+    return media_channel_->GetRtpSendParameters(ssrc_);
-    last_transaction_id_ = rtc::CreateRandomUuid();
-    result.transaction_id = last_transaction_id_.value();
-    return result;
   });
 }
 
@@ -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<RTCError>(RTC_FROM_HERE, [&] {
-    RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    return media_channel_->SetRtpSendParameters(ssrc_, parameters);
-    last_transaction_id_.reset();
-    return result;
   });
 }
 
@@ -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<RtpParameters>(RTC_FROM_HERE, [&] {
-    RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_);
+    return media_channel_->GetRtpSendParameters(ssrc_);
-    last_transaction_id_ = rtc::CreateRandomUuid();
-    result.transaction_id = last_transaction_id_.value();
-    return result;
   });
 }
 
@@ -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<RTCError>(RTC_FROM_HERE, [&] {
-    RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    return media_channel_->SetRtpSendParameters(ssrc_, parameters);
-    last_transaction_id_.reset();
-    return result;
   });
 }
 

pc/rtpsender.h

@@ -128,7 +128,7 @@ class AudioRtpSender : public DtmfProviderInterface,
 
   std::vector<std::string> stream_ids() const override { return stream_ids_; }
 
-  RtpParameters GetParameters() override;
+  RtpParameters GetParameters() const override;
   RTCError SetParameters(const RtpParameters& parameters) override;
 
   rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
@@ -172,7 +172,6 @@ class AudioRtpSender : public DtmfProviderInterface,
   StatsCollector* stats_;
   rtc::scoped_refptr<AudioTrackInterface> track_;
   rtc::scoped_refptr<DtmfSenderInterface> dtmf_sender_proxy_;
-  rtc::Optional<std::string> 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<std::string> stream_ids() const override { return stream_ids_; }
 
-  RtpParameters GetParameters() override;
+  RtpParameters GetParameters() const override;
   RTCError SetParameters(const RtpParameters& parameters) override;
 
   rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
@@ -254,7 +253,6 @@ class VideoRtpSender : public ObserverInterface,
   std::vector<std::string> stream_ids_;
   cricket::VideoMediaChannel* media_channel_ = nullptr;
   rtc::scoped_refptr<VideoTrackInterface> track_;
-  rtc::Optional<std::string> last_transaction_id_;
   uint32_t ssrc_ = 0;
   VideoTrackInterface::ContentHint cached_track_content_hint_ =
       VideoTrackInterface::ContentHint::kNone;

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();
 

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<std::string>());
-  MOCK_METHOD0(GetParameters, RtpParameters());
+  MOCK_CONST_METHOD0(GetParameters, RtpParameters());
   MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&));
   MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr<DtmfSenderInterface>());
 

sdk/android/api/org/webrtc/RtpParameters.java

@@ -109,24 +109,21 @@ public class RtpParameters {
     }
   }
 
-  public final String transactionId;
-
   public final List<Encoding> 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<Codec> codecs;
 
-  @CalledByNative
+  public RtpParameters() {
-  RtpParameters(String transactionId, List<Encoding> encodings, List<Codec> codecs) {
+    this.encodings = new ArrayList<>();
-    this.transactionId = transactionId;
+    this.codecs = new ArrayList<>();
-    this.encodings = encodings;
-    this.codecs = codecs;
   }
 
   @CalledByNative
-  String getTransactionId() {
+  RtpParameters(List<Encoding> encodings, List<Codec> codecs) {
-    return transactionId;
+    this.encodings = encodings;
+    this.codecs = codecs;
   }
 
   @CalledByNative

sdk/android/src/jni/pc/rtpparameters.cc

@@ -59,10 +59,6 @@ RtpParameters JavaToNativeRtpParameters(JNIEnv* jni,
                                         const JavaRef<jobject>& j_parameters) {
   RtpParameters parameters;
 
-  ScopedJavaLocalRef<jstring> j_transaction_id =
-      Java_RtpParameters_getTransactionId(jni, j_parameters);
-  parameters.transaction_id = JavaToNativeString(jni, j_transaction_id);
-
   // Convert encodings.
   ScopedJavaLocalRef<jobject> j_encodings =
       Java_RtpParameters_getEncodings(jni, j_parameters);
@@ -94,7 +90,7 @@ ScopedJavaLocalRef<jobject> 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));

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]
@@ -47,7 +44,6 @@
 
 - (webrtc::RtpParameters)nativeParameters {
     webrtc::RtpParameters parameters;
-  parameters.transaction_id = [NSString stdStringForString:_transactionId];
   for (RTCRtpEncodingParameters *encoding in _encodings) {
     parameters.encodings.push_back(encoding.nativeParameters);
   }

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<RTCRtpEncodingParameters *> *encodings;