chore: cherry-pick 6584528aeb0f0 from webrtc and 36e370cf4db9a from chromium (#31358)

* chore: cherry-pick 6584528aeb0f0 from webrtc and 36e370cf4db9a from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
Pedro Pontes
2021-10-10 08:25:09 +02:00
committed by GitHub
parent 3b1127ccbd
commit dfc9cb6e7f
4 changed files with 412 additions and 0 deletions

View File

@@ -108,3 +108,4 @@ cherry-pick-f8a74d72f328.patch
cherry-pick-f2fd53c6d706.patch
cherry-pick-096afc1c5428.patch
cherry-pick-4e528a5a8d83.patch
check_direction_of_rtcencodedframes.patch

View File

@@ -0,0 +1,188 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tony Herre <toprice@chromium.org>
Date: Fri, 1 Oct 2021 19:18:45 +0000
Subject: Check direction of RTCEncodedFrames
Add a check to RTCEncodedVideoUnderlyingSink of the direction of the
underlying webrtc frame, to make sure a web app doesn't take a received
encoded frame and pass it into a sender insertable stream, which is as
yet unsupported in WebRTC.
Bug: 1247260
Change-Id: I9ed5bd8b2bd5e5ee461f3b553f8a91f6cc2e9ed7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3190473
Commit-Queue: Tony Herre <toprice@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/main@{#927323}
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc
index c390ab72418194cb10c3b0bc5a83b95de8dd19f6..775b837fee46836fd292b17ac8d80e4c83bd08a8 100644
--- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc
+++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc
@@ -14,8 +14,10 @@ namespace blink {
RTCEncodedVideoUnderlyingSink::RTCEncodedVideoUnderlyingSink(
ScriptState* script_state,
- TransformerCallback transformer_callback)
- : transformer_callback_(std::move(transformer_callback)) {
+ TransformerCallback transformer_callback,
+ webrtc::TransformableFrameInterface::Direction expected_direction)
+ : transformer_callback_(std::move(transformer_callback)),
+ expected_direction_(expected_direction) {
DCHECK(transformer_callback_);
}
@@ -53,6 +55,12 @@ ScriptPromise RTCEncodedVideoUnderlyingSink::write(
return ScriptPromise();
}
+ if (webrtc_frame->GetDirection() != expected_direction_) {
+ exception_state.ThrowDOMException(DOMExceptionCode::kOperationError,
+ "Invalid frame");
+ return ScriptPromise();
+ }
+
RTCEncodedVideoStreamTransformer* transformer = transformer_callback_.Run();
if (!transformer) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h
index dd1cad227eb7947dd0bf2ec7ba217956cb7a8787..8591fcc6eb1c78d0e107e4f097d3133d111ab959 100644
--- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h
+++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h
@@ -7,6 +7,7 @@
#include "third_party/blink/renderer/core/streams/underlying_sink_base.h"
#include "third_party/blink/renderer/modules/modules_export.h"
+#include "third_party/webrtc/api/frame_transformer_interface.h"
namespace blink {
@@ -18,7 +19,9 @@ class MODULES_EXPORT RTCEncodedVideoUnderlyingSink final
public:
using TransformerCallback =
base::RepeatingCallback<RTCEncodedVideoStreamTransformer*()>;
- RTCEncodedVideoUnderlyingSink(ScriptState*, TransformerCallback);
+ RTCEncodedVideoUnderlyingSink(ScriptState*,
+ TransformerCallback,
+ webrtc::TransformableFrameInterface::Direction);
// UnderlyingSinkBase
ScriptPromise start(ScriptState*,
@@ -37,6 +40,7 @@ class MODULES_EXPORT RTCEncodedVideoUnderlyingSink final
private:
TransformerCallback transformer_callback_;
+ webrtc::TransformableFrameInterface::Direction expected_direction_;
};
} // namespace blink
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc
index d06c1a735767c936d25e68e23c15a4aca7ef901a..8238bd13f9a7c3276468c76bd3aee4c56345e83a 100644
--- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc
+++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc
@@ -75,11 +75,15 @@ class RTCEncodedVideoUnderlyingSinkTest : public testing::Test {
EXPECT_FALSE(transformer_.HasTransformedFrameSinkCallback(kSSRC));
}
- RTCEncodedVideoUnderlyingSink* CreateSink(ScriptState* script_state) {
+ RTCEncodedVideoUnderlyingSink* CreateSink(
+ ScriptState* script_state,
+ webrtc::TransformableFrameInterface::Direction expected_direction =
+ webrtc::TransformableFrameInterface::Direction::kSender) {
return MakeGarbageCollected<RTCEncodedVideoUnderlyingSink>(
script_state,
WTF::BindRepeating(&RTCEncodedVideoUnderlyingSinkTest::GetTransformer,
- WTF::Unretained(this)));
+ WTF::Unretained(this)),
+ expected_direction);
}
RTCEncodedVideoUnderlyingSink* CreateNullCallbackSink(
@@ -87,14 +91,20 @@ class RTCEncodedVideoUnderlyingSinkTest : public testing::Test {
return MakeGarbageCollected<RTCEncodedVideoUnderlyingSink>(
script_state,
WTF::BindRepeating(
- []() -> RTCEncodedVideoStreamTransformer* { return nullptr; }));
+ []() -> RTCEncodedVideoStreamTransformer* { return nullptr; }),
+ webrtc::TransformableFrameInterface::Direction::kSender);
}
RTCEncodedVideoStreamTransformer* GetTransformer() { return &transformer_; }
- ScriptValue CreateEncodedVideoFrameChunk(ScriptState* script_state) {
+ ScriptValue CreateEncodedVideoFrameChunk(
+ ScriptState* script_state,
+ webrtc::TransformableFrameInterface::Direction direction =
+ webrtc::TransformableFrameInterface::Direction::kSender) {
auto mock_frame = std::make_unique<NiceMock<MockTransformableVideoFrame>>();
+
ON_CALL(*mock_frame.get(), GetSsrc).WillByDefault(Return(kSSRC));
+ ON_CALL(*mock_frame.get(), GetDirection).WillByDefault(Return(direction));
RTCEncodedVideoFrame* frame =
MakeGarbageCollected<RTCEncodedVideoFrame>(std::move(mock_frame));
return ScriptValue(script_state->GetIsolate(),
@@ -175,4 +185,21 @@ TEST_F(RTCEncodedVideoUnderlyingSinkTest, WriteToNullCallbackSinkFails) {
DOMExceptionCode::kInvalidStateError));
}
+TEST_F(RTCEncodedVideoUnderlyingSinkTest, WriteInvalidDirectionFails) {
+ V8TestingScope v8_scope;
+ ScriptState* script_state = v8_scope.GetScriptState();
+ auto* sink = CreateSink(
+ script_state, webrtc::TransformableFrameInterface::Direction::kSender);
+
+ // Write an encoded chunk with direction set to Receiver should fail as it
+ // doesn't match the expected direction of our sink.
+ DummyExceptionStateForTesting dummy_exception_state;
+ sink->write(script_state,
+ CreateEncodedVideoFrameChunk(
+ script_state,
+ webrtc::TransformableFrameInterface::Direction::kReceiver),
+ nullptr, dummy_exception_state);
+ EXPECT_TRUE(dummy_exception_state.HadException());
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
index 9291c4f8a10f6d822bbda0ed47c2e206eb7955eb..1838055f829337c5f9c302ad0b0703899693be25 100644
--- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
+++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc
@@ -595,7 +595,8 @@ void RTCRtpReceiver::InitializeEncodedVideoStreams(ScriptState* script_state) {
->GetEncodedVideoStreamTransformer()
: nullptr;
},
- WrapWeakPersistent(this)));
+ WrapWeakPersistent(this)),
+ webrtc::TransformableFrameInterface::Direction::kReceiver);
// The high water mark for the stream is set to 1 so that the stream seems
// ready to write, but without queuing frames.
WritableStream* writable_stream =
diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc
index 9adbd0b2ba67af5cb416e95685e0993200a4bbbc..293714421b4f00b8561bf637313b5041f15d0875 100644
--- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc
+++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc
@@ -950,7 +950,8 @@ void RTCRtpSender::InitializeEncodedVideoStreams(ScriptState* script_state) {
->GetEncodedVideoStreamTransformer()
: nullptr;
},
- WrapWeakPersistent(this)));
+ WrapWeakPersistent(this)),
+ webrtc::TransformableFrameInterface::Direction::kSender);
// The high water mark for the stream is set to 1 so that the stream is
// ready to write, but without queuing frames.
WritableStream* writable_stream =
diff --git a/third_party/blink/renderer/modules/peerconnection/testing/mock_transformable_video_frame.h b/third_party/blink/renderer/modules/peerconnection/testing/mock_transformable_video_frame.h
index 4780d43ca327c3b65125ccb855cdad618c3fc6ec..1e0b12eac7bfd70247b1a74c237a63c805303c16 100644
--- a/third_party/blink/renderer/modules/peerconnection/testing/mock_transformable_video_frame.h
+++ b/third_party/blink/renderer/modules/peerconnection/testing/mock_transformable_video_frame.h
@@ -27,6 +27,10 @@ class MockTransformableVideoFrame
GetMetadata,
(),
(const, override));
+ MOCK_METHOD(webrtc::TransformableFrameInterface::Direction,
+ GetDirection,
+ (),
+ (const, override));
};
} // namespace blink

View File

@@ -1 +1,2 @@
add_thread_local_to_x_error_trap_cc.patch
merge_to_94_add_direction_indicator_to_transformableframes.patch

View File

@@ -0,0 +1,222 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tony Herre <toprice@chromium.org>
Date: Mon, 4 Oct 2021 10:02:51 +0000
Subject: Add Direction indicator to TransformableFrames
Currently the implementation of FrameTransformers uses distinct,
incompatible types for recevied vs about-to-be-sent frames. This adds a
flag in the interface so we can at least check that we are being given
the correct type. crbug.com/1250638 tracks removing the need for this.
Chrome will be updated after this to check the direction flag and provide
a javascript error if the wrong type of frame is written into the
encoded insertable streams writable stream, rather than crashing.
(cherry picked from commit 8fb41a39e1a2d151d1c00c409630dcee80adeb76)
Bug: chromium:1247260
Change-Id: I9cbb66962ea0718ed47c5e5dba19a8ff9635b0b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232301
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tony Herre <toprice@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#35100}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/233943
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/branch-heads/4606@{#4}
Cr-Branched-From: 8b18304e66524060eca390f143033ba51322b3a2-refs/heads/master@{#34737}
diff --git a/api/frame_transformer_interface.h b/api/frame_transformer_interface.h
index 2cfe6edb884b53b0b3fa87afad264aa411d112bd..d04acc098cb564c161f87c87d68511429895757c 100644
--- a/api/frame_transformer_interface.h
+++ b/api/frame_transformer_interface.h
@@ -35,6 +35,16 @@ class TransformableFrameInterface {
virtual uint32_t GetTimestamp() const = 0;
virtual uint32_t GetSsrc() const = 0;
+
+ enum class Direction {
+ kUnknown,
+ kReceiver,
+ kSender,
+ };
+ // TODO(crbug.com/1250638): Remove this distinction between receiver and
+ // sender frames to allow received frames to be directly re-transmitted on
+ // other PeerConnectionss.
+ virtual Direction GetDirection() const { return Direction::kUnknown; }
};
class TransformableVideoFrameInterface : public TransformableFrameInterface {
diff --git a/audio/channel_receive_frame_transformer_delegate.cc b/audio/channel_receive_frame_transformer_delegate.cc
index 7e617df78018ece3f619c7fb3288aa180468135d..d00e7176c5205be4a4faac36e8364c070fe7c014 100644
--- a/audio/channel_receive_frame_transformer_delegate.cc
+++ b/audio/channel_receive_frame_transformer_delegate.cc
@@ -18,15 +18,16 @@
namespace webrtc {
namespace {
-class TransformableAudioFrame : public TransformableAudioFrameInterface {
+class TransformableIncomingAudioFrame
+ : public TransformableAudioFrameInterface {
public:
- TransformableAudioFrame(rtc::ArrayView<const uint8_t> payload,
- const RTPHeader& header,
- uint32_t ssrc)
+ TransformableIncomingAudioFrame(rtc::ArrayView<const uint8_t> payload,
+ const RTPHeader& header,
+ uint32_t ssrc)
: payload_(payload.data(), payload.size()),
header_(header),
ssrc_(ssrc) {}
- ~TransformableAudioFrame() override = default;
+ ~TransformableIncomingAudioFrame() override = default;
rtc::ArrayView<const uint8_t> GetData() const override { return payload_; }
void SetData(rtc::ArrayView<const uint8_t> data) override {
@@ -36,6 +37,7 @@ class TransformableAudioFrame : public TransformableAudioFrameInterface {
uint32_t GetTimestamp() const override { return header_.timestamp; }
uint32_t GetSsrc() const override { return ssrc_; }
const RTPHeader& GetHeader() const override { return header_; }
+ Direction GetDirection() const override { return Direction::kReceiver; }
private:
rtc::Buffer payload_;
@@ -71,7 +73,7 @@ void ChannelReceiveFrameTransformerDelegate::Transform(
uint32_t ssrc) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
frame_transformer_->Transform(
- std::make_unique<TransformableAudioFrame>(packet, header, ssrc));
+ std::make_unique<TransformableIncomingAudioFrame>(packet, header, ssrc));
}
void ChannelReceiveFrameTransformerDelegate::OnTransformedFrame(
@@ -88,7 +90,10 @@ void ChannelReceiveFrameTransformerDelegate::ReceiveFrame(
RTC_DCHECK_RUN_ON(&sequence_checker_);
if (!receive_frame_callback_)
return;
- auto* transformed_frame = static_cast<TransformableAudioFrame*>(frame.get());
+ RTC_CHECK_EQ(frame->GetDirection(),
+ TransformableFrameInterface::Direction::kReceiver);
+ auto* transformed_frame =
+ static_cast<TransformableIncomingAudioFrame*>(frame.get());
receive_frame_callback_(transformed_frame->GetData(),
transformed_frame->GetHeader());
}
diff --git a/audio/channel_send_frame_transformer_delegate.cc b/audio/channel_send_frame_transformer_delegate.cc
index 72a459d89783f9b7bc498aabc15cd7c7b45f3783..5597e7553e956c9dc20e311f5e16d163d9a7119e 100644
--- a/audio/channel_send_frame_transformer_delegate.cc
+++ b/audio/channel_send_frame_transformer_delegate.cc
@@ -15,16 +15,16 @@
namespace webrtc {
namespace {
-class TransformableAudioFrame : public TransformableFrameInterface {
+class TransformableOutgoingAudioFrame : public TransformableFrameInterface {
public:
- TransformableAudioFrame(AudioFrameType frame_type,
- uint8_t payload_type,
- uint32_t rtp_timestamp,
- uint32_t rtp_start_timestamp,
- const uint8_t* payload_data,
- size_t payload_size,
- int64_t absolute_capture_timestamp_ms,
- uint32_t ssrc)
+ TransformableOutgoingAudioFrame(AudioFrameType frame_type,
+ uint8_t payload_type,
+ uint32_t rtp_timestamp,
+ uint32_t rtp_start_timestamp,
+ const uint8_t* payload_data,
+ size_t payload_size,
+ int64_t absolute_capture_timestamp_ms,
+ uint32_t ssrc)
: frame_type_(frame_type),
payload_type_(payload_type),
rtp_timestamp_(rtp_timestamp),
@@ -32,7 +32,7 @@ class TransformableAudioFrame : public TransformableFrameInterface {
payload_(payload_data, payload_size),
absolute_capture_timestamp_ms_(absolute_capture_timestamp_ms),
ssrc_(ssrc) {}
- ~TransformableAudioFrame() override = default;
+ ~TransformableOutgoingAudioFrame() override = default;
rtc::ArrayView<const uint8_t> GetData() const override { return payload_; }
void SetData(rtc::ArrayView<const uint8_t> data) override {
payload_.SetData(data.data(), data.size());
@@ -48,6 +48,7 @@ class TransformableAudioFrame : public TransformableFrameInterface {
int64_t GetAbsoluteCaptureTimestampMs() const {
return absolute_capture_timestamp_ms_;
}
+ Direction GetDirection() const override { return Direction::kSender; }
private:
AudioFrameType frame_type_;
@@ -90,9 +91,10 @@ void ChannelSendFrameTransformerDelegate::Transform(
size_t payload_size,
int64_t absolute_capture_timestamp_ms,
uint32_t ssrc) {
- frame_transformer_->Transform(std::make_unique<TransformableAudioFrame>(
- frame_type, payload_type, rtp_timestamp, rtp_start_timestamp,
- payload_data, payload_size, absolute_capture_timestamp_ms, ssrc));
+ frame_transformer_->Transform(
+ std::make_unique<TransformableOutgoingAudioFrame>(
+ frame_type, payload_type, rtp_timestamp, rtp_start_timestamp,
+ payload_data, payload_size, absolute_capture_timestamp_ms, ssrc));
}
void ChannelSendFrameTransformerDelegate::OnTransformedFrame(
@@ -111,9 +113,12 @@ void ChannelSendFrameTransformerDelegate::SendFrame(
std::unique_ptr<TransformableFrameInterface> frame) const {
MutexLock lock(&send_lock_);
RTC_DCHECK_RUN_ON(encoder_queue_);
+ RTC_CHECK_EQ(frame->GetDirection(),
+ TransformableFrameInterface::Direction::kSender);
if (!send_frame_callback_)
return;
- auto* transformed_frame = static_cast<TransformableAudioFrame*>(frame.get());
+ auto* transformed_frame =
+ static_cast<TransformableOutgoingAudioFrame*>(frame.get());
send_frame_callback_(transformed_frame->GetFrameType(),
transformed_frame->GetPayloadType(),
transformed_frame->GetTimestamp() -
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
index 23e66bf75744c49b5ee057263d989e58f36b39d3..77a4ca5c35ebe51d13267e5dccce7ff360a29a33 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
@@ -75,6 +75,8 @@ class TransformableVideoSenderFrame : public TransformableVideoFrameInterface {
return expected_retransmission_time_ms_;
}
+ Direction GetDirection() const override { return Direction::kSender; }
+
private:
rtc::scoped_refptr<EncodedImageBufferInterface> encoded_data_;
const RTPVideoHeader header_;
@@ -144,6 +146,8 @@ void RTPSenderVideoFrameTransformerDelegate::OnTransformedFrame(
void RTPSenderVideoFrameTransformerDelegate::SendVideo(
std::unique_ptr<TransformableFrameInterface> transformed_frame) const {
RTC_CHECK(encoder_queue_->IsCurrent());
+ RTC_CHECK_EQ(transformed_frame->GetDirection(),
+ TransformableFrameInterface::Direction::kSender);
MutexLock lock(&sender_lock_);
if (!sender_)
return;
diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc
index f2f81df3ee76603c24ccc8ce93beaedb7d1eeaac..d6c6944e0efe999ea2e4b1a74184e8a74de7312a 100644
--- a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc
+++ b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc
@@ -58,6 +58,8 @@ class TransformableVideoReceiverFrame
return std::move(frame_);
}
+ Direction GetDirection() const override { return Direction::kReceiver; }
+
private:
std::unique_ptr<RtpFrameObject> frame_;
const VideoFrameMetadata metadata_;
@@ -110,6 +112,8 @@ void RtpVideoStreamReceiverFrameTransformerDelegate::OnTransformedFrame(
void RtpVideoStreamReceiverFrameTransformerDelegate::ManageFrame(
std::unique_ptr<TransformableFrameInterface> frame) {
RTC_DCHECK_RUN_ON(&network_sequence_checker_);
+ RTC_CHECK_EQ(frame->GetDirection(),
+ TransformableFrameInterface::Direction::kReceiver);
if (!receiver_)
return;
auto transformed_frame = absl::WrapUnique(