Commit e0431952 authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

[RTCInsertableStreams] Ensure correct transformer type is set

This CL ensures that a sender or receiver gets at most one transformer
and that the transformer is of the correct type.
Prior to this CL, it was possible for an audio sender or receiver
to get a video transformer if insertable streams were enabled for
video. This resulted in data not flowing due to no transformer
being set, or in crashes due to the wrong type of transformer being
executed.

Tests added in
https://chromium-review.googlesource.com/c/chromium/src/+/2139213

Bug: 1068125,1068164
Change-Id: I68a701098b09e0dfc8edaec82bc7161ae686b915
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139231
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarHarald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757035}
parent 5ba2cd81
...@@ -54,9 +54,9 @@ RTCRtpReceiver::RTCRtpReceiver(RTCPeerConnection* pc, ...@@ -54,9 +54,9 @@ RTCRtpReceiver::RTCRtpReceiver(RTCPeerConnection* pc,
DCHECK(pc_); DCHECK(pc_);
DCHECK(receiver_); DCHECK(receiver_);
DCHECK(track_); DCHECK(track_);
if (force_encoded_audio_insertable_streams_) if (force_encoded_audio_insertable_streams_ && track_->kind() == "audio")
RegisterEncodedAudioStreamCallback(); RegisterEncodedAudioStreamCallback();
if (force_encoded_video_insertable_streams_) if (force_encoded_video_insertable_streams_ && track_->kind() == "video")
RegisterEncodedVideoStreamCallback(); RegisterEncodedVideoStreamCallback();
} }
...@@ -362,6 +362,7 @@ void RTCRtpReceiver::RegisterEncodedAudioStreamCallback() { ...@@ -362,6 +362,7 @@ void RTCRtpReceiver::RegisterEncodedAudioStreamCallback() {
DCHECK(!platform_receiver() DCHECK(!platform_receiver()
->GetEncodedAudioStreamTransformer() ->GetEncodedAudioStreamTransformer()
->HasTransformerCallback()); ->HasTransformerCallback());
DCHECK_EQ(track_->kind(), "audio");
platform_receiver() platform_receiver()
->GetEncodedAudioStreamTransformer() ->GetEncodedAudioStreamTransformer()
->SetTransformerCallback( ->SetTransformerCallback(
...@@ -370,9 +371,7 @@ void RTCRtpReceiver::RegisterEncodedAudioStreamCallback() { ...@@ -370,9 +371,7 @@ void RTCRtpReceiver::RegisterEncodedAudioStreamCallback() {
} }
void RTCRtpReceiver::UnregisterEncodedAudioStreamCallback() { void RTCRtpReceiver::UnregisterEncodedAudioStreamCallback() {
DCHECK(!platform_receiver() DCHECK_EQ(track_->kind(), "audio");
->GetEncodedAudioStreamTransformer()
->HasTransformerCallback());
platform_receiver() platform_receiver()
->GetEncodedAudioStreamTransformer() ->GetEncodedAudioStreamTransformer()
->ResetTransformerCallback(); ->ResetTransformerCallback();
...@@ -432,6 +431,7 @@ void RTCRtpReceiver::RegisterEncodedVideoStreamCallback() { ...@@ -432,6 +431,7 @@ void RTCRtpReceiver::RegisterEncodedVideoStreamCallback() {
DCHECK(!platform_receiver() DCHECK(!platform_receiver()
->GetEncodedVideoStreamTransformer() ->GetEncodedVideoStreamTransformer()
->HasTransformerCallback()); ->HasTransformerCallback());
DCHECK_EQ(track_->kind(), "video");
platform_receiver() platform_receiver()
->GetEncodedVideoStreamTransformer() ->GetEncodedVideoStreamTransformer()
->SetTransformerCallback( ->SetTransformerCallback(
...@@ -440,9 +440,7 @@ void RTCRtpReceiver::RegisterEncodedVideoStreamCallback() { ...@@ -440,9 +440,7 @@ void RTCRtpReceiver::RegisterEncodedVideoStreamCallback() {
} }
void RTCRtpReceiver::UnregisterEncodedVideoStreamCallback() { void RTCRtpReceiver::UnregisterEncodedVideoStreamCallback() {
DCHECK(!platform_receiver() DCHECK_EQ(track_->kind(), "video");
->GetEncodedVideoStreamTransformer()
->HasTransformerCallback());
platform_receiver() platform_receiver()
->GetEncodedVideoStreamTransformer() ->GetEncodedVideoStreamTransformer()
->ResetTransformerCallback(); ->ResetTransformerCallback();
......
...@@ -147,18 +147,21 @@ class RTCRtpReceiverImpl::RTCRtpReceiverInternal ...@@ -147,18 +147,21 @@ class RTCRtpReceiverImpl::RTCRtpReceiverInternal
state_(std::move(state)) { state_(std::move(state)) {
DCHECK(native_peer_connection_); DCHECK(native_peer_connection_);
DCHECK(state_.is_initialized()); DCHECK(state_.is_initialized());
if (force_encoded_audio_insertable_streams) { if (force_encoded_audio_insertable_streams &&
webrtc_receiver_->media_type() == cricket::MEDIA_TYPE_AUDIO) {
encoded_audio_transformer_ = encoded_audio_transformer_ =
std::make_unique<RTCEncodedAudioStreamTransformer>(main_task_runner_); std::make_unique<RTCEncodedAudioStreamTransformer>(main_task_runner_);
webrtc_receiver_->SetDepacketizerToDecoderFrameTransformer( webrtc_receiver_->SetDepacketizerToDecoderFrameTransformer(
encoded_audio_transformer_->Delegate()); encoded_audio_transformer_->Delegate());
} }
if (force_encoded_video_insertable_streams) { if (force_encoded_video_insertable_streams &&
webrtc_receiver_->media_type() == cricket::MEDIA_TYPE_VIDEO) {
encoded_video_transformer_ = encoded_video_transformer_ =
std::make_unique<RTCEncodedVideoStreamTransformer>(main_task_runner_); std::make_unique<RTCEncodedVideoStreamTransformer>(main_task_runner_);
webrtc_receiver_->SetDepacketizerToDecoderFrameTransformer( webrtc_receiver_->SetDepacketizerToDecoderFrameTransformer(
encoded_video_transformer_->Delegate()); encoded_video_transformer_->Delegate());
} }
DCHECK(!encoded_audio_transformer_ || !encoded_video_transformer_);
} }
const RtpReceiverState& state() const { const RtpReceiverState& state() const {
......
...@@ -178,7 +178,8 @@ TEST_F(RTCRtpReceiverImplTest, CreateReceiverWithInsertableStreams) { ...@@ -178,7 +178,8 @@ TEST_F(RTCRtpReceiverImplTest, CreateReceiverWithInsertableStreams) {
/*force_encoded_audio_insertable_streams=*/true, /*force_encoded_audio_insertable_streams=*/true,
/*force_encoded_video_insertable_streams=*/true); /*force_encoded_video_insertable_streams=*/true);
EXPECT_TRUE(receiver_->GetEncodedAudioStreamTransformer()); EXPECT_TRUE(receiver_->GetEncodedAudioStreamTransformer());
EXPECT_TRUE(receiver_->GetEncodedVideoStreamTransformer()); // There should be no video transformer in audio receivers.
EXPECT_FALSE(receiver_->GetEncodedVideoStreamTransformer());
} }
} // namespace blink } // namespace blink
...@@ -392,12 +392,14 @@ RTCRtpSender::RTCRtpSender(RTCPeerConnection* pc, ...@@ -392,12 +392,14 @@ RTCRtpSender::RTCRtpSender(RTCPeerConnection* pc,
track_(track), track_(track),
streams_(std::move(streams)), streams_(std::move(streams)),
force_encoded_audio_insertable_streams_( force_encoded_audio_insertable_streams_(
force_encoded_audio_insertable_streams), force_encoded_audio_insertable_streams && kind_ == "audio"),
force_encoded_video_insertable_streams_( force_encoded_video_insertable_streams_(
force_encoded_video_insertable_streams) { force_encoded_video_insertable_streams && kind_ == "video") {
DCHECK(pc_); DCHECK(pc_);
DCHECK(sender_); DCHECK(sender_);
DCHECK(!track || kind_ == track->kind()); DCHECK(!track || kind_ == track->kind());
DCHECK(!force_encoded_audio_insertable_streams_ ||
!force_encoded_video_insertable_streams_);
if (force_encoded_audio_insertable_streams_) if (force_encoded_audio_insertable_streams_)
RegisterEncodedAudioStreamCallback(); RegisterEncodedAudioStreamCallback();
if (force_encoded_video_insertable_streams_) if (force_encoded_video_insertable_streams_)
...@@ -757,6 +759,7 @@ void RTCRtpSender::RegisterEncodedAudioStreamCallback() { ...@@ -757,6 +759,7 @@ void RTCRtpSender::RegisterEncodedAudioStreamCallback() {
DCHECK(!web_sender() DCHECK(!web_sender()
->GetEncodedAudioStreamTransformer() ->GetEncodedAudioStreamTransformer()
->HasTransformerCallback()); ->HasTransformerCallback());
DCHECK_EQ(kind_, "audio");
web_sender()->GetEncodedAudioStreamTransformer()->SetTransformerCallback( web_sender()->GetEncodedAudioStreamTransformer()->SetTransformerCallback(
WTF::BindRepeating(&RTCRtpSender::OnAudioFrameFromEncoder, WTF::BindRepeating(&RTCRtpSender::OnAudioFrameFromEncoder,
WrapWeakPersistent(this))); WrapWeakPersistent(this)));
...@@ -818,6 +821,7 @@ void RTCRtpSender::RegisterEncodedVideoStreamCallback() { ...@@ -818,6 +821,7 @@ void RTCRtpSender::RegisterEncodedVideoStreamCallback() {
DCHECK(!web_sender() DCHECK(!web_sender()
->GetEncodedVideoStreamTransformer() ->GetEncodedVideoStreamTransformer()
->HasTransformerCallback()); ->HasTransformerCallback());
DCHECK_EQ(kind_, "video");
web_sender()->GetEncodedVideoStreamTransformer()->SetTransformerCallback( web_sender()->GetEncodedVideoStreamTransformer()->SetTransformerCallback(
WTF::BindRepeating(&RTCRtpSender::OnVideoFrameFromEncoder, WTF::BindRepeating(&RTCRtpSender::OnVideoFrameFromEncoder,
WrapWeakPersistent(this))); WrapWeakPersistent(this)));
......
...@@ -196,18 +196,21 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal ...@@ -196,18 +196,21 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal
state_(std::move(state)) { state_(std::move(state)) {
DCHECK(track_map_); DCHECK(track_map_);
DCHECK(state_.is_initialized()); DCHECK(state_.is_initialized());
if (force_encoded_audio_insertable_streams) { if (force_encoded_audio_insertable_streams &&
webrtc_sender_->media_type() == cricket::MEDIA_TYPE_AUDIO) {
encoded_audio_transformer_ = encoded_audio_transformer_ =
std::make_unique<RTCEncodedAudioStreamTransformer>(main_task_runner_); std::make_unique<RTCEncodedAudioStreamTransformer>(main_task_runner_);
webrtc_sender_->SetEncoderToPacketizerFrameTransformer( webrtc_sender_->SetEncoderToPacketizerFrameTransformer(
encoded_audio_transformer_->Delegate()); encoded_audio_transformer_->Delegate());
} }
if (force_encoded_video_insertable_streams) { if (force_encoded_video_insertable_streams &&
webrtc_sender_->media_type() == cricket::MEDIA_TYPE_VIDEO) {
encoded_video_transformer_ = encoded_video_transformer_ =
std::make_unique<RTCEncodedVideoStreamTransformer>(main_task_runner_); std::make_unique<RTCEncodedVideoStreamTransformer>(main_task_runner_);
webrtc_sender_->SetEncoderToPacketizerFrameTransformer( webrtc_sender_->SetEncoderToPacketizerFrameTransformer(
encoded_video_transformer_->Delegate()); encoded_video_transformer_->Delegate());
} }
DCHECK(!encoded_audio_transformer_ || !encoded_video_transformer_);
} }
const RtpSenderState& state() const { const RtpSenderState& state() const {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment