diff options
author | Max Morin <maxmorin@webrtc.org> | 2018-05-07 06:09:37 +0000 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2018-05-07 08:02:34 +0000 |
commit | 909338b0277af58dc38ea14a828e20e4e392a07a (patch) | |
tree | 14952d4bb82c8fd3df316ca2df0d082a4bf001f8 /pc | |
parent | b044d3a28f077af941c5d49a5fa90b7b7a408749 (diff) | |
download | webrtc-909338b0277af58dc38ea14a828e20e4e392a07a.tar.gz |
Revert "Implement RtpParameters.transaction_id for PC RtpSenderInterface"
This reverts commit 5faf36ef3c582350fba5ef97a3549e440d81a283.
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}
Diffstat (limited to 'pc')
-rw-r--r-- | pc/rtpsender.cc | 48 | ||||
-rw-r--r-- | pc/rtpsender.h | 6 | ||||
-rw-r--r-- | pc/rtpsenderreceiver_unittest.cc | 118 | ||||
-rw-r--r-- | pc/test/mock_rtpsenderinternal.h | 2 |
4 files changed, 9 insertions, 165 deletions
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<RtpParameters>(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<RTCError>(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<RtpParameters>(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<RTCError>(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<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; 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<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>()); |