Change VideoRtpReceiver to create remote VideoTrack and VideoTrackSource.
This enabled us to be able to remove VideoTrack::GetSink and RemoteVideoCapturer.
Since video frames from the decoder is delivered on a media engine internal thread, VideoBroadCaster must be made thread safe.
BUG=webrtc:5426
R=deadbeef@webrtc.org, pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/1765423005 .
Cr-Commit-Position: refs/heads/master@{#11944}
diff --git a/webrtc/api/api_tests.gyp b/webrtc/api/api_tests.gyp
index 53285f9..bb706dc 100644
--- a/webrtc/api/api_tests.gyp
+++ b/webrtc/api/api_tests.gyp
@@ -37,14 +37,13 @@
'fakemetricsobserver.h',
'jsepsessiondescription_unittest.cc',
'localaudiosource_unittest.cc',
- 'mediaconstraintsinterface_unittest.cc',
+ 'mediaconstraintsinterface_unittest.cc',
'mediastream_unittest.cc',
'peerconnection_unittest.cc',
'peerconnectionendtoend_unittest.cc',
'peerconnectionfactory_unittest.cc',
'peerconnectioninterface_unittest.cc',
# 'peerconnectionproxy_unittest.cc',
- 'remotevideocapturer_unittest.cc',
'rtpsenderreceiver_unittest.cc',
'statscollector_unittest.cc',
'test/fakeaudiocapturemodule.cc',
diff --git a/webrtc/api/mediastreaminterface.h b/webrtc/api/mediastreaminterface.h
index 54d3fd4..69a50c9 100644
--- a/webrtc/api/mediastreaminterface.h
+++ b/webrtc/api/mediastreaminterface.h
@@ -177,26 +177,6 @@
virtual VideoTrackSourceInterface* GetSource() const = 0;
- // Return the track input sink. I.e., frames sent to this sink are
- // propagated to all renderers registered with the track. The
- // returned sink must not change between calls. Currently, this
- // method is used for remote tracks (VideoRtpReceiver); further
- // refactoring is planned for this path, it's unclear if this method
- // belongs here long term.
-
- // We do this instead of simply implementing the
- // VideoSourceInterface directly, because if we did the latter, we'd
- // need an OnFrame method in VideoTrackProxy, with a thread jump on
- // each call.
-
- // TODO(nisse): It has a default implementation so that mock
- // objects, in particular, chrome's MockWebRtcVideoTrack, doesn't
- // need to know about it. Consider removing the implementation (or
- // this comment) after refactoring dust settles.
- virtual rtc::VideoSinkInterface<cricket::VideoFrame>* GetSink() {
- return nullptr;
- };
-
protected:
virtual ~VideoTrackInterface() {}
};
diff --git a/webrtc/api/mediastreamtrackproxy.h b/webrtc/api/mediastreamtrackproxy.h
index ada8291..676ba70 100644
--- a/webrtc/api/mediastreamtrackproxy.h
+++ b/webrtc/api/mediastreamtrackproxy.h
@@ -54,7 +54,6 @@
const rtc::VideoSinkWants&)
PROXY_METHOD1(void, RemoveSink, rtc::VideoSinkInterface<cricket::VideoFrame>*)
PROXY_CONSTMETHOD0(VideoTrackSourceInterface*, GetSource)
- PROXY_METHOD0(rtc::VideoSinkInterface<cricket::VideoFrame>*, GetSink)
PROXY_METHOD1(void, RegisterObserver, ObserverInterface*)
PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*)
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index c2039ae..700714a 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -381,10 +381,8 @@
// Factory class for creating remote MediaStreams and MediaStreamTracks.
class RemoteMediaStreamFactory {
public:
- explicit RemoteMediaStreamFactory(rtc::Thread* signaling_thread,
- rtc::Thread* worker_thread)
- : signaling_thread_(signaling_thread),
- worker_thread_(worker_thread) {}
+ explicit RemoteMediaStreamFactory(rtc::Thread* signaling_thread)
+ : signaling_thread_(signaling_thread) {}
rtc::scoped_refptr<MediaStreamInterface> CreateMediaStream(
const std::string& stream_label) {
@@ -400,15 +398,6 @@
stream, track_id, RemoteAudioSource::Create(ssrc, provider));
}
- VideoTrackInterface* AddVideoTrack(webrtc::MediaStreamInterface* stream,
- const std::string& track_id) {
- return AddTrack<VideoTrackInterface, VideoTrack, VideoTrackProxy>(
- stream, track_id,
- VideoCapturerTrackSource::Create(
- worker_thread_, new RemoteVideoCapturer(), nullptr, true)
- .get());
- }
-
private:
template <typename TI, typename T, typename TP, typename S>
TI* AddTrack(MediaStreamInterface* stream,
@@ -424,7 +413,6 @@
}
rtc::Thread* signaling_thread_;
- rtc::Thread* worker_thread_;
};
bool ExtractMediaSessionOptions(
@@ -611,8 +599,8 @@
media_controller_.reset(factory_->CreateMediaController(media_config));
- remote_stream_factory_.reset(new RemoteMediaStreamFactory(
- factory_->signaling_thread(), factory_->worker_thread()));
+ remote_stream_factory_.reset(
+ new RemoteMediaStreamFactory(factory_->signaling_thread()));
session_.reset(
new WebRtcSession(media_controller_.get(), factory_->signaling_thread(),
@@ -1325,11 +1313,12 @@
}
void PeerConnection::CreateVideoReceiver(MediaStreamInterface* stream,
- VideoTrackInterface* video_track,
+ const std::string& track_id,
uint32_t ssrc) {
- receivers_.push_back(RtpReceiverProxy::Create(
- signaling_thread(),
- new VideoRtpReceiver(video_track, ssrc, session_.get())));
+ VideoRtpReceiver* video_receiver = new VideoRtpReceiver(
+ stream, track_id, factory_->worker_thread(), ssrc, session_.get());
+ receivers_.push_back(
+ RtpReceiverProxy::Create(signaling_thread(), video_receiver));
}
// TODO(deadbeef): Keep RtpReceivers around even if track goes away in remote
@@ -1677,9 +1666,7 @@
ssrc, session_.get(), stream, track_id);
CreateAudioReceiver(stream, audio_track, ssrc);
} else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- VideoTrackInterface* video_track =
- remote_stream_factory_->AddVideoTrack(stream, track_id);
- CreateVideoReceiver(stream, video_track, ssrc);
+ CreateVideoReceiver(stream, track_id, ssrc);
} else {
RTC_DCHECK(false && "Invalid media type");
}
@@ -1702,8 +1689,9 @@
rtc::scoped_refptr<VideoTrackInterface> video_track =
stream->FindVideoTrack(track_id);
if (video_track) {
- video_track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
stream->RemoveTrack(video_track);
+ // Stopping or destroying a VideoRtpReceiver will end the
+ // VideoRtpReceiver::track().
DestroyVideoReceiver(stream, video_track);
}
} else {
diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h
index e4fcdf0..2e9d9b2 100644
--- a/webrtc/api/peerconnection.h
+++ b/webrtc/api/peerconnection.h
@@ -27,6 +27,7 @@
class MediaStreamObserver;
class RemoteMediaStreamFactory;
+class VideoRtpReceiver;
// Populates |session_options| from |rtc_options|, and returns true if options
// are valid.
@@ -167,8 +168,9 @@
void CreateAudioReceiver(MediaStreamInterface* stream,
AudioTrackInterface* audio_track,
uint32_t ssrc);
+
void CreateVideoReceiver(MediaStreamInterface* stream,
- VideoTrackInterface* video_track,
+ const std::string& track_id,
uint32_t ssrc);
void DestroyAudioReceiver(MediaStreamInterface* stream,
AudioTrackInterface* audio_track);
diff --git a/webrtc/api/remotevideocapturer.cc b/webrtc/api/remotevideocapturer.cc
index 8c1ffcc..177ba54 100644
--- a/webrtc/api/remotevideocapturer.cc
+++ b/webrtc/api/remotevideocapturer.cc
@@ -10,69 +10,4 @@
#include "webrtc/api/remotevideocapturer.h"
-#include "webrtc/base/logging.h"
-#include "webrtc/media/base/videoframe.h"
-
-namespace webrtc {
-
-RemoteVideoCapturer::RemoteVideoCapturer() {}
-
-RemoteVideoCapturer::~RemoteVideoCapturer() {}
-
-cricket::CaptureState RemoteVideoCapturer::Start(
- const cricket::VideoFormat& capture_format) {
- if (capture_state() == cricket::CS_RUNNING) {
- LOG(LS_WARNING)
- << "RemoteVideoCapturer::Start called when it's already started.";
- return capture_state();
- }
-
- LOG(LS_INFO) << "RemoteVideoCapturer::Start";
- SetCaptureFormat(&capture_format);
- return cricket::CS_RUNNING;
-}
-
-void RemoteVideoCapturer::Stop() {
- if (capture_state() == cricket::CS_STOPPED) {
- LOG(LS_WARNING)
- << "RemoteVideoCapturer::Stop called when it's already stopped.";
- return;
- }
-
- LOG(LS_INFO) << "RemoteVideoCapturer::Stop";
- SetCaptureFormat(NULL);
- SetCaptureState(cricket::CS_STOPPED);
-}
-
-bool RemoteVideoCapturer::IsRunning() {
- return capture_state() == cricket::CS_RUNNING;
-}
-
-bool RemoteVideoCapturer::GetPreferredFourccs(std::vector<uint32_t>* fourccs) {
- if (!fourccs)
- return false;
- fourccs->push_back(cricket::FOURCC_I420);
- return true;
-}
-
-bool RemoteVideoCapturer::GetBestCaptureFormat(
- const cricket::VideoFormat& desired, cricket::VideoFormat* best_format) {
- if (!best_format) {
- return false;
- }
-
- // RemoteVideoCapturer does not support capability enumeration.
- // Use the desired format as the best format.
- best_format->width = desired.width;
- best_format->height = desired.height;
- best_format->fourcc = cricket::FOURCC_I420;
- best_format->interval = desired.interval;
- return true;
-}
-
-bool RemoteVideoCapturer::IsScreencast() const {
- // TODO(ronghuawu): what about remote screencast stream.
- return false;
-}
-
-} // namespace webrtc
+// TODO(perkj): Remove this file once Chrome gyp file doesn't depend on it.
diff --git a/webrtc/api/remotevideocapturer.h b/webrtc/api/remotevideocapturer.h
index a0fd4b7..774cb5a 100644
--- a/webrtc/api/remotevideocapturer.h
+++ b/webrtc/api/remotevideocapturer.h
@@ -11,38 +11,6 @@
#ifndef WEBRTC_API_REMOTEVIDEOCAPTURER_H_
#define WEBRTC_API_REMOTEVIDEOCAPTURER_H_
-#include <vector>
-
-#include "webrtc/api/mediastreaminterface.h"
-#include "webrtc/media/base/videocapturer.h"
-#include "webrtc/media/base/videorenderer.h"
-
-namespace webrtc {
-
-// RemoteVideoCapturer implements a simple cricket::VideoCapturer which
-// gets decoded remote video frames from media channel.
-// It's used as the remote video source's VideoCapturer so that the remote video
-// can be used as a cricket::VideoCapturer and in that way a remote video stream
-// can implement the MediaStreamSourceInterface.
-class RemoteVideoCapturer : public cricket::VideoCapturer {
- public:
- RemoteVideoCapturer();
- virtual ~RemoteVideoCapturer();
-
- // cricket::VideoCapturer implementation.
- cricket::CaptureState Start(
- const cricket::VideoFormat& capture_format) override;
- void Stop() override;
- bool IsRunning() override;
- bool GetPreferredFourccs(std::vector<uint32_t>* fourccs) override;
- bool GetBestCaptureFormat(const cricket::VideoFormat& desired,
- cricket::VideoFormat* best_format) override;
- bool IsScreencast() const override;
-
- private:
- RTC_DISALLOW_COPY_AND_ASSIGN(RemoteVideoCapturer);
-};
-
-} // namespace webrtc
+// TODO(perkj): Remove this file once Chrome gyp file doesn't depend on it.
#endif // WEBRTC_API_REMOTEVIDEOCAPTURER_H_
diff --git a/webrtc/api/remotevideocapturer_unittest.cc b/webrtc/api/remotevideocapturer_unittest.cc
deleted file mode 100644
index 6478d21..0000000
--- a/webrtc/api/remotevideocapturer_unittest.cc
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Copyright 2013 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#include <string>
-
-#include "webrtc/api/remotevideocapturer.h"
-#include "webrtc/base/gunit.h"
-#include "webrtc/media/engine/webrtcvideoframe.h"
-
-using cricket::CaptureState;
-using cricket::VideoCapturer;
-using cricket::VideoFormat;
-using cricket::VideoFormatPod;
-using cricket::VideoFrame;
-
-static const int kMaxWaitMs = 1000;
-static const VideoFormatPod kTestFormat =
- {640, 480, FPS_TO_INTERVAL(30), cricket::FOURCC_ANY};
-
-class RemoteVideoCapturerTest : public testing::Test,
- public sigslot::has_slots<> {
- protected:
- RemoteVideoCapturerTest()
- : captured_frame_num_(0),
- capture_state_(cricket::CS_STOPPED) {}
-
- virtual void SetUp() {
- capturer_.SignalStateChange.connect(
- this, &RemoteVideoCapturerTest::OnStateChange);
- }
-
- ~RemoteVideoCapturerTest() {
- capturer_.SignalStateChange.disconnect(this);
- }
-
- int captured_frame_num() const {
- return captured_frame_num_;
- }
-
- CaptureState capture_state() const {
- return capture_state_;
- }
-
- webrtc::RemoteVideoCapturer capturer_;
-
- private:
- void OnStateChange(VideoCapturer* capturer,
- CaptureState capture_state) {
- EXPECT_EQ(&capturer_, capturer);
- capture_state_ = capture_state;
- }
-
- int captured_frame_num_;
- CaptureState capture_state_;
-};
-
-TEST_F(RemoteVideoCapturerTest, StartStop) {
- // Start
- EXPECT_TRUE(
- capturer_.StartCapturing(VideoFormat(kTestFormat)));
- EXPECT_TRUE_WAIT((cricket::CS_RUNNING == capture_state()), kMaxWaitMs);
- EXPECT_EQ(VideoFormat(kTestFormat),
- *capturer_.GetCaptureFormat());
- EXPECT_TRUE(capturer_.IsRunning());
-
- // Stop
- capturer_.Stop();
- EXPECT_TRUE_WAIT((cricket::CS_STOPPED == capture_state()), kMaxWaitMs);
- EXPECT_TRUE(NULL == capturer_.GetCaptureFormat());
-}
-
-TEST_F(RemoteVideoCapturerTest, GetPreferredFourccs) {
- EXPECT_FALSE(capturer_.GetPreferredFourccs(NULL));
-
- std::vector<uint32_t> fourccs;
- EXPECT_TRUE(capturer_.GetPreferredFourccs(&fourccs));
- EXPECT_EQ(1u, fourccs.size());
- EXPECT_EQ(cricket::FOURCC_I420, fourccs.at(0));
-}
-
-TEST_F(RemoteVideoCapturerTest, GetBestCaptureFormat) {
- VideoFormat desired = VideoFormat(kTestFormat);
- EXPECT_FALSE(capturer_.GetBestCaptureFormat(desired, NULL));
-
- VideoFormat expected_format = VideoFormat(kTestFormat);
- expected_format.fourcc = cricket::FOURCC_I420;
- VideoFormat best_format;
- EXPECT_TRUE(capturer_.GetBestCaptureFormat(desired, &best_format));
- EXPECT_EQ(expected_format, best_format);
-}
diff --git a/webrtc/api/rtpreceiver.cc b/webrtc/api/rtpreceiver.cc
index 0ad5218..1590612 100644
--- a/webrtc/api/rtpreceiver.cc
+++ b/webrtc/api/rtpreceiver.cc
@@ -10,7 +10,8 @@
#include "webrtc/api/rtpreceiver.h"
-#include "webrtc/api/videosourceinterface.h"
+#include "webrtc/api/mediastreamtrackproxy.h"
+#include "webrtc/api/videotrack.h"
namespace webrtc {
@@ -65,11 +66,27 @@
provider_->SetAudioPlayout(ssrc_, track_->enabled());
}
-VideoRtpReceiver::VideoRtpReceiver(VideoTrackInterface* track,
+VideoRtpReceiver::VideoRtpReceiver(MediaStreamInterface* stream,
+ const std::string& track_id,
+ rtc::Thread* worker_thread,
uint32_t ssrc,
VideoProviderInterface* provider)
- : id_(track->id()), track_(track), ssrc_(ssrc), provider_(provider) {
- provider_->SetVideoPlayout(ssrc_, true, track_->GetSink());
+ : id_(track_id),
+ ssrc_(ssrc),
+ provider_(provider),
+ source_(new RefCountedObject<VideoTrackSource>(&broadcaster_,
+ worker_thread,
+ true /* remote */)),
+ track_(VideoTrackProxy::Create(
+ rtc::Thread::Current(),
+ VideoTrack::Create(track_id, source_.get()))) {
+ source_->SetState(MediaSourceInterface::kLive);
+ // TODO(perkj): It should be enough to set the source state. All tracks
+ // belonging to the same source should get its state from the source.
+ // I.e. if a track has been cloned from a remote source.
+ track_->set_state(webrtc::MediaStreamTrackInterface::kLive);
+ provider_->SetVideoPlayout(ssrc_, true, &broadcaster_);
+ stream->AddTrack(track_);
}
VideoRtpReceiver::~VideoRtpReceiver() {
@@ -83,6 +100,11 @@
if (!provider_) {
return;
}
+ source_->SetState(MediaSourceInterface::kEnded);
+ source_->OnSourceDestroyed();
+ // TODO(perkj): It should be enough to set the source state. All tracks
+ // belonging to the same source should get its state from the source.
+ track_->set_state(MediaStreamTrackInterface::kEnded);
provider_->SetVideoPlayout(ssrc_, false, nullptr);
provider_ = nullptr;
}
diff --git a/webrtc/api/rtpreceiver.h b/webrtc/api/rtpreceiver.h
index 43adc72..a62ed19 100644
--- a/webrtc/api/rtpreceiver.h
+++ b/webrtc/api/rtpreceiver.h
@@ -19,7 +19,9 @@
#include "webrtc/api/mediastreamprovider.h"
#include "webrtc/api/rtpreceiverinterface.h"
+#include "webrtc/api/videotracksource.h"
#include "webrtc/base/basictypes.h"
+#include "webrtc/media/base/videobroadcaster.h"
namespace webrtc {
@@ -60,12 +62,18 @@
class VideoRtpReceiver : public rtc::RefCountedObject<RtpReceiverInterface> {
public:
- VideoRtpReceiver(VideoTrackInterface* track,
+ VideoRtpReceiver(MediaStreamInterface* stream,
+ const std::string& track_id,
+ rtc::Thread* worker_thread,
uint32_t ssrc,
VideoProviderInterface* provider);
virtual ~VideoRtpReceiver();
+ rtc::scoped_refptr<VideoTrackInterface> video_track() const {
+ return track_.get();
+ }
+
// RtpReceiverInterface implementation
rtc::scoped_refptr<MediaStreamTrackInterface> track() const override {
return track_.get();
@@ -77,9 +85,16 @@
private:
std::string id_;
- rtc::scoped_refptr<VideoTrackInterface> track_;
uint32_t ssrc_;
VideoProviderInterface* provider_;
+ // |broadcaster_| is needed since the decoder can only handle one sink.
+ // It might be better if the decoder can handle multiple sinks and consider
+ // the VideoSinkWants.
+ rtc::VideoBroadcaster broadcaster_;
+ // |source_| is held here to be able to change the state of the source when
+ // the VideoRtpReceiver is stopped.
+ rtc::scoped_refptr<VideoTrackSource> source_;
+ rtc::scoped_refptr<VideoTrackInterface> track_;
};
} // namespace webrtc
diff --git a/webrtc/api/rtpsenderreceiver_unittest.cc b/webrtc/api/rtpsenderreceiver_unittest.cc
index 0a7c408..22fa14f 100644
--- a/webrtc/api/rtpsenderreceiver_unittest.cc
+++ b/webrtc/api/rtpsenderreceiver_unittest.cc
@@ -150,12 +150,11 @@
}
void CreateVideoRtpReceiver() {
- AddVideoTrack(true);
- EXPECT_CALL(video_provider_,
- SetVideoPlayout(kVideoSsrc, true,
- video_track_->GetSink()));
- video_rtp_receiver_ = new VideoRtpReceiver(stream_->GetVideoTracks()[0],
- kVideoSsrc, &video_provider_);
+ EXPECT_CALL(video_provider_, SetVideoPlayout(kVideoSsrc, true, _));
+ video_rtp_receiver_ =
+ new VideoRtpReceiver(stream_, kVideoTrackId, rtc::Thread::Current(),
+ kVideoSsrc, &video_provider_);
+ video_track_ = video_rtp_receiver_->video_track();
}
void DestroyAudioRtpReceiver() {
@@ -244,6 +243,20 @@
DestroyVideoRtpSender();
}
+TEST_F(RtpSenderReceiverTest, RemoteVideoTrackState) {
+ CreateVideoRtpReceiver();
+
+ EXPECT_EQ(webrtc::MediaStreamTrackInterface::kLive, video_track_->state());
+ EXPECT_EQ(webrtc::MediaSourceInterface::kLive,
+ video_track_->GetSource()->state());
+
+ DestroyVideoRtpReceiver();
+
+ EXPECT_EQ(webrtc::MediaStreamTrackInterface::kEnded, video_track_->state());
+ EXPECT_EQ(webrtc::MediaSourceInterface::kEnded,
+ video_track_->GetSource()->state());
+}
+
TEST_F(RtpSenderReceiverTest, RemoteVideoTrackDisable) {
CreateVideoRtpReceiver();
diff --git a/webrtc/api/videocapturertracksource_unittest.cc b/webrtc/api/videocapturertracksource_unittest.cc
index b183421..0a06870 100644
--- a/webrtc/api/videocapturertracksource_unittest.cc
+++ b/webrtc/api/videocapturertracksource_unittest.cc
@@ -182,27 +182,6 @@
source_->Stop();
}
-// Test start stop with a remote VideoSource - the video source that has a
-// RemoteVideoCapturer and takes video frames from FrameInput.
-TEST_F(VideoCapturerTrackSourceTest, StartStopRemote) {
- source_ = VideoCapturerTrackSource::Create(
- rtc::Thread::Current(), new webrtc::RemoteVideoCapturer(), NULL, true);
-
- ASSERT_TRUE(source_.get() != NULL);
- EXPECT_TRUE(NULL != source_->GetVideoCapturer());
-
- state_observer_.reset(new StateObserver(source_));
- source_->RegisterObserver(state_observer_.get());
- source_->AddOrUpdateSink(&renderer_, rtc::VideoSinkWants());
-
- EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
- kMaxWaitMs);
-
- source_->GetVideoCapturer()->Stop();
- EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(),
- kMaxWaitMs);
-}
-
// Test that a VideoSource transition to kEnded if the capture device
// fails.
TEST_F(VideoCapturerTrackSourceTest, CameraFailed) {
diff --git a/webrtc/api/videotrack.cc b/webrtc/api/videotrack.cc
index 36d256f..b2ea1f9 100644
--- a/webrtc/api/videotrack.cc
+++ b/webrtc/api/videotrack.cc
@@ -50,10 +50,6 @@
renderers_.RemoveSink(sink);
}
-rtc::VideoSinkInterface<cricket::VideoFrame>* VideoTrack::GetSink() {
- return &renderers_;
-}
-
bool VideoTrack::set_enabled(bool enable) {
renderers_.SetEnabled(enable);
return MediaStreamTrack<VideoTrackInterface>::set_enabled(enable);
diff --git a/webrtc/api/videotrack.h b/webrtc/api/videotrack.h
index 066aa90..3a59504 100644
--- a/webrtc/api/videotrack.h
+++ b/webrtc/api/videotrack.h
@@ -33,7 +33,6 @@
virtual VideoTrackSourceInterface* GetSource() const {
return video_source_.get();
}
- rtc::VideoSinkInterface<cricket::VideoFrame>* GetSink() override;
virtual bool set_enabled(bool enable);
virtual std::string kind() const;
diff --git a/webrtc/api/videotrack_unittest.cc b/webrtc/api/videotrack_unittest.cc
index 9f44b81..008ba21 100644
--- a/webrtc/api/videotrack_unittest.cc
+++ b/webrtc/api/videotrack_unittest.cc
@@ -10,42 +10,37 @@
#include <string>
-#include "webrtc/api/remotevideocapturer.h"
#include "webrtc/api/test/fakevideotrackrenderer.h"
#include "webrtc/api/videocapturertracksource.h"
#include "webrtc/api/videotrack.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/media/base/fakevideocapturer.h"
#include "webrtc/media/base/fakemediaengine.h"
#include "webrtc/media/engine/webrtcvideoframe.h"
-#include "webrtc/pc/channelmanager.h"
using webrtc::FakeVideoTrackRenderer;
using webrtc::FakeVideoTrackRendererOld;
-using webrtc::VideoCapturerTrackSource;
+using webrtc::VideoTrackSource;
using webrtc::VideoTrack;
using webrtc::VideoTrackInterface;
-namespace {
-
-class WebRtcVideoTestFrame : public cricket::WebRtcVideoFrame {
- public:
- using cricket::WebRtcVideoFrame::SetRotation;
-};
-
-} // namespace
class VideoTrackTest : public testing::Test {
public:
VideoTrackTest() {
static const char kVideoTrackId[] = "track_id";
video_track_ = VideoTrack::Create(
- kVideoTrackId, VideoCapturerTrackSource::Create(
- rtc::Thread::Current(),
- new webrtc::RemoteVideoCapturer(), NULL, true));
+ kVideoTrackId,
+ new rtc::RefCountedObject<VideoTrackSource>(
+ &capturer_, rtc::Thread::Current(), true /* remote */));
+ capturer_.Start(
+ cricket::VideoFormat(640, 480, cricket::VideoFormat::FpsToInterval(30),
+ cricket::FOURCC_I420));
}
protected:
+ cricket::FakeVideoCapturer capturer_;
rtc::scoped_refptr<VideoTrackInterface> video_track_;
};
@@ -56,35 +51,18 @@
rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_1(
new FakeVideoTrackRenderer(video_track_.get()));
- rtc::VideoSinkInterface<cricket::VideoFrame>* renderer_input =
- video_track_->GetSink();
- ASSERT_FALSE(renderer_input == NULL);
-
- cricket::WebRtcVideoFrame frame;
- frame.InitToBlack(123, 123, 0);
- renderer_input->OnFrame(frame);
+ capturer_.CaptureFrame();
EXPECT_EQ(1, renderer_1->num_rendered_frames());
- EXPECT_EQ(123, renderer_1->width());
- EXPECT_EQ(123, renderer_1->height());
-
// FakeVideoTrackRenderer register itself to |video_track_|
rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_2(
new FakeVideoTrackRenderer(video_track_.get()));
-
- renderer_input->OnFrame(frame);
-
- EXPECT_EQ(123, renderer_1->width());
- EXPECT_EQ(123, renderer_1->height());
- EXPECT_EQ(123, renderer_2->width());
- EXPECT_EQ(123, renderer_2->height());
-
+ capturer_.CaptureFrame();
EXPECT_EQ(2, renderer_1->num_rendered_frames());
EXPECT_EQ(1, renderer_2->num_rendered_frames());
video_track_->RemoveSink(renderer_1.get());
- renderer_input->OnFrame(frame);
-
+ capturer_.CaptureFrame();
EXPECT_EQ(2, renderer_1->num_rendered_frames());
EXPECT_EQ(2, renderer_2->num_rendered_frames());
}
@@ -97,35 +75,19 @@
rtc::scoped_ptr<FakeVideoTrackRendererOld> renderer_1(
new FakeVideoTrackRendererOld(video_track_.get()));
- rtc::VideoSinkInterface<cricket::VideoFrame>* renderer_input =
- video_track_->GetSink();
- ASSERT_FALSE(renderer_input == NULL);
-
- cricket::WebRtcVideoFrame frame;
- frame.InitToBlack(123, 123, 0);
- renderer_input->OnFrame(frame);
+ capturer_.CaptureFrame();
EXPECT_EQ(1, renderer_1->num_rendered_frames());
- EXPECT_EQ(123, renderer_1->width());
- EXPECT_EQ(123, renderer_1->height());
-
// FakeVideoTrackRenderer register itself to |video_track_|
rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_2(
new FakeVideoTrackRenderer(video_track_.get()));
- renderer_input->OnFrame(frame);
-
- EXPECT_EQ(123, renderer_1->width());
- EXPECT_EQ(123, renderer_1->height());
- EXPECT_EQ(123, renderer_2->width());
- EXPECT_EQ(123, renderer_2->height());
-
+ capturer_.CaptureFrame();
EXPECT_EQ(2, renderer_1->num_rendered_frames());
EXPECT_EQ(1, renderer_2->num_rendered_frames());
video_track_->RemoveRenderer(renderer_1.get());
- renderer_input->OnFrame(frame);
-
+ capturer_.CaptureFrame();
EXPECT_EQ(2, renderer_1->num_rendered_frames());
EXPECT_EQ(2, renderer_2->num_rendered_frames());
}
@@ -135,32 +97,17 @@
rtc::scoped_ptr<FakeVideoTrackRenderer> renderer(
new FakeVideoTrackRenderer(video_track_.get()));
- rtc::VideoSinkInterface<cricket::VideoFrame>* renderer_input =
- video_track_->GetSink();
- ASSERT_FALSE(renderer_input == NULL);
-
- cricket::WebRtcVideoFrame frame;
- frame.InitToBlack(100, 200, 0);
- // Make it not all-black
- frame.GetUPlane()[0] = 0;
-
- renderer_input->OnFrame(frame);
+ capturer_.CaptureFrame();
EXPECT_EQ(1, renderer->num_rendered_frames());
EXPECT_FALSE(renderer->black_frame());
- EXPECT_EQ(100, renderer->width());
- EXPECT_EQ(200, renderer->height());
video_track_->set_enabled(false);
- renderer_input->OnFrame(frame);
+ capturer_.CaptureFrame();
EXPECT_EQ(2, renderer->num_rendered_frames());
EXPECT_TRUE(renderer->black_frame());
- EXPECT_EQ(100, renderer->width());
- EXPECT_EQ(200, renderer->height());
video_track_->set_enabled(true);
- renderer_input->OnFrame(frame);
+ capturer_.CaptureFrame();
EXPECT_EQ(3, renderer->num_rendered_frames());
EXPECT_FALSE(renderer->black_frame());
- EXPECT_EQ(100, renderer->width());
- EXPECT_EQ(200, renderer->height());
}
diff --git a/webrtc/api/videotracksource.cc b/webrtc/api/videotracksource.cc
index 3b5e19a..f8212d7 100644
--- a/webrtc/api/videotracksource.cc
+++ b/webrtc/api/videotracksource.cc
@@ -32,9 +32,16 @@
}
}
+void VideoTrackSource::OnSourceDestroyed() {
+ source_ = nullptr;
+}
+
void VideoTrackSource::AddOrUpdateSink(
rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
const rtc::VideoSinkWants& wants) {
+ if (!source_) {
+ return;
+ }
worker_thread_->Invoke<void>(rtc::Bind(
&rtc::VideoSourceInterface<cricket::VideoFrame>::AddOrUpdateSink, source_,
sink, wants));
@@ -42,6 +49,9 @@
void VideoTrackSource::RemoveSink(
rtc::VideoSinkInterface<cricket::VideoFrame>* sink) {
+ if (!source_) {
+ return;
+ }
worker_thread_->Invoke<void>(
rtc::Bind(&rtc::VideoSourceInterface<cricket::VideoFrame>::RemoveSink,
source_, sink));
diff --git a/webrtc/api/videotracksource.h b/webrtc/api/videotracksource.h
index 59dae42..5e2437c 100644
--- a/webrtc/api/videotracksource.h
+++ b/webrtc/api/videotracksource.h
@@ -24,21 +24,27 @@
VideoTrackSource(rtc::VideoSourceInterface<cricket::VideoFrame>* source,
rtc::Thread* worker_thread,
bool remote);
-
void SetState(SourceState new_state);
+ // OnSourceDestroyed clears this instance pointer to |source_|. It is useful
+ // when the underlying rtc::VideoSourceInterface is destroyed before the
+ // reference counted VideoTrackSource.
+ void OnSourceDestroyed();
+
SourceState state() const override { return state_; }
bool remote() const override { return remote_; }
void Stop() override{};
void Restart() override{};
- virtual bool is_screencast() const { return false; };
- virtual bool needs_denoising() const { return false; };
+ virtual bool is_screencast() const { return false; }
+ virtual bool needs_denoising() const { return false; }
void AddOrUpdateSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
const rtc::VideoSinkWants& wants) override;
void RemoveSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink) override;
+ cricket::VideoCapturer* GetVideoCapturer() override { return nullptr; }
+
protected:
rtc::Thread* worker_thread() { return worker_thread_; }
diff --git a/webrtc/media/base/videobroadcaster.cc b/webrtc/media/base/videobroadcaster.cc
index 5fa3db7..015f520 100644
--- a/webrtc/media/base/videobroadcaster.cc
+++ b/webrtc/media/base/videobroadcaster.cc
@@ -25,6 +25,7 @@
const VideoSinkWants& wants) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
+ rtc::CritScope cs(&sinks_and_wants_lock_);
SinkPair* sink_pair = FindSinkPair(sink);
if (!sink_pair) {
@@ -39,8 +40,8 @@
VideoSinkInterface<cricket::VideoFrame>* sink) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
+ rtc::CritScope cs(&sinks_and_wants_lock_);
RTC_DCHECK(FindSinkPair(sink));
-
sinks_.erase(std::remove_if(sinks_.begin(), sinks_.end(),
[sink](const SinkPair& sink_pair) {
return sink_pair.sink == sink;
@@ -51,16 +52,18 @@
bool VideoBroadcaster::frame_wanted() const {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ rtc::CritScope cs(&sinks_and_wants_lock_);
return !sinks_.empty();
}
VideoSinkWants VideoBroadcaster::wants() const {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ rtc::CritScope cs(&sinks_and_wants_lock_);
return current_wants_;
}
void VideoBroadcaster::OnFrame(const cricket::VideoFrame& frame) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ rtc::CritScope cs(&sinks_and_wants_lock_);
for (auto& sink_pair : sinks_) {
sink_pair.sink->OnFrame(frame);
}
diff --git a/webrtc/media/base/videobroadcaster.h b/webrtc/media/base/videobroadcaster.h
index 89c9f3d..b4227ea 100644
--- a/webrtc/media/base/videobroadcaster.h
+++ b/webrtc/media/base/videobroadcaster.h
@@ -14,6 +14,7 @@
#include <utility>
#include <vector>
+#include "webrtc/base/criticalsection.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/media/base/videoframe.h"
#include "webrtc/media/base/videosinkinterface.h"
@@ -21,6 +22,12 @@
namespace rtc {
+// VideoBroadcaster broadcast video frames to sinks and combines
+// VideoSinkWants from its sinks. It does that by implementing
+// rtc::VideoSourceInterface and rtc::VideoSinkInterface.
+// Sinks must be added and removed on one and only one thread.
+// Video frames can be broadcasted on any thread. I.e VideoBroadcaster::OnFrame
+// can be called on any thread.
class VideoBroadcaster : public VideoSourceInterface<cricket::VideoFrame>,
public VideoSinkInterface<cricket::VideoFrame> {
public:
@@ -46,13 +53,15 @@
VideoSinkInterface<cricket::VideoFrame>* sink;
VideoSinkWants wants;
};
- SinkPair* FindSinkPair(const VideoSinkInterface<cricket::VideoFrame>* sink);
- void UpdateWants();
+ SinkPair* FindSinkPair(const VideoSinkInterface<cricket::VideoFrame>* sink)
+ EXCLUSIVE_LOCKS_REQUIRED(sinks_and_wants_lock_);
+ void UpdateWants() EXCLUSIVE_LOCKS_REQUIRED(sinks_and_wants_lock_);
ThreadChecker thread_checker_;
+ rtc::CriticalSection sinks_and_wants_lock_;
- VideoSinkWants current_wants_;
- std::vector<SinkPair> sinks_;
+ VideoSinkWants current_wants_ GUARDED_BY(sinks_and_wants_lock_);
+ std::vector<SinkPair> sinks_ GUARDED_BY(sinks_and_wants_lock_);
};
} // namespace rtc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 02de6b8..daffc2f 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -1616,6 +1616,8 @@
}
}
capturer_ = capturer;
+ // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since
+ // that might cause a lock order inversion.
capturer_->AddOrUpdateSink(this, sink_wants_);
return true;
}
@@ -1631,6 +1633,8 @@
return false;
}
+ // |capturer_->RemoveSink| may not be called while holding |lock_| since
+ // that might cause a lock order inversion.
capturer_->RemoveSink(this);
capturer_ = NULL;
// Reset |cpu_restricted_counter_| if the capturer is changed. It is not
@@ -1755,46 +1759,54 @@
void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
const ChangedSendParameters& params) {
- rtc::CritScope cs(&lock_);
- // |recreate_stream| means construction-time parameters have changed and the
- // sending stream needs to be reset with the new config.
- bool recreate_stream = false;
- if (params.rtcp_mode) {
- parameters_.config.rtp.rtcp_mode = *params.rtcp_mode;
- recreate_stream = true;
- }
+ {
+ rtc::CritScope cs(&lock_);
+ // |recreate_stream| means construction-time parameters have changed and the
+ // sending stream needs to be reset with the new config.
+ bool recreate_stream = false;
+ if (params.rtcp_mode) {
+ parameters_.config.rtp.rtcp_mode = *params.rtcp_mode;
+ recreate_stream = true;
+ }
+ if (params.rtp_header_extensions) {
+ parameters_.config.rtp.extensions = *params.rtp_header_extensions;
+ recreate_stream = true;
+ }
+ if (params.max_bandwidth_bps) {
+ // Max bitrate has changed, reconfigure encoder settings on the next frame
+ // or stream recreation.
+ parameters_.max_bitrate_bps = *params.max_bandwidth_bps;
+ pending_encoder_reconfiguration_ = true;
+ }
+ if (params.conference_mode) {
+ parameters_.conference_mode = *params.conference_mode;
+ }
+ if (params.options)
+ SetOptions(*params.options);
+
+ // Set codecs and options.
+ if (params.codec) {
+ SetCodec(*params.codec);
+ return;
+ } else if (params.conference_mode && parameters_.codec_settings) {
+ SetCodec(*parameters_.codec_settings);
+ return;
+ }
+ if (recreate_stream) {
+ LOG(LS_INFO)
+ << "RecreateWebRtcStream (send) because of SetSendParameters";
+ RecreateWebRtcStream();
+ }
+ } // release |lock_|
+
+ // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since
+ // that might cause a lock order inversion.
if (params.rtp_header_extensions) {
- parameters_.config.rtp.extensions = *params.rtp_header_extensions;
sink_wants_.rotation_applied = !ContainsHeaderExtension(
*params.rtp_header_extensions, kRtpVideoRotationHeaderExtension);
if (capturer_) {
capturer_->AddOrUpdateSink(this, sink_wants_);
}
- recreate_stream = true;
- }
- if (params.max_bandwidth_bps) {
- // Max bitrate has changed, reconfigure encoder settings on the next frame
- // or stream recreation.
- parameters_.max_bitrate_bps = *params.max_bandwidth_bps;
- pending_encoder_reconfiguration_ = true;
- }
- if (params.conference_mode) {
- parameters_.conference_mode = *params.conference_mode;
- }
- if (params.options)
- SetOptions(*params.options);
-
- // Set codecs and options.
- if (params.codec) {
- SetCodec(*params.codec);
- return;
- } else if (params.conference_mode && parameters_.codec_settings) {
- SetCodec(*parameters_.codec_settings);
- return;
- }
- if (recreate_stream) {
- LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetSendParameters";
- RecreateWebRtcStream();
}
}
@@ -1961,6 +1973,8 @@
sink_wants_.max_pixel_count = max_pixel_count;
sink_wants_.max_pixel_count_step_up = max_pixel_count_step_up;
}
+ // |capturer_->AddOrUpdateSink| may not be called while holding |lock_| since
+ // that might cause a lock order inversion.
capturer_->AddOrUpdateSink(this, sink_wants_);
}
