Make VideoTrack and VideoTrackRenderers implement rtc::VideoSourceInterface.
This patch tries to only change the interface to VideoTrack, with
minimal changes to the implementation. Some points worth noting:
VideoTrackRenderers should ultimately be deleted, but it is kept for
now since we need an object implementing webrtc::VideoRenderer, and
that shouldn't be VideoTrack.
BUG=webrtc:5426
TBR=glaznev@webrtc.org // please look at examples
Review URL: https://codereview.webrtc.org/1684423002
Cr-Commit-Position: refs/heads/master@{#11775}
diff --git a/webrtc/api/mediastreaminterface.h b/webrtc/api/mediastreaminterface.h
index d4fe2bf..100db08 100644
--- a/webrtc/api/mediastreaminterface.h
+++ b/webrtc/api/mediastreaminterface.h
@@ -24,6 +24,7 @@
#include "webrtc/base/refcount.h"
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/media/base/videosinkinterface.h"
+#include "webrtc/media/base/videosourceinterface.h"
namespace cricket {
@@ -121,12 +122,30 @@
class VideoSourceInterface;
-class VideoTrackInterface : public MediaStreamTrackInterface {
+class VideoTrackInterface
+ : public MediaStreamTrackInterface,
+ public rtc::VideoSourceInterface<cricket::VideoFrame> {
public:
- // Register a renderer that will render all frames received on this track.
- virtual void AddRenderer(VideoRendererInterface* renderer) = 0;
- // Deregister a renderer.
- virtual void RemoveRenderer(VideoRendererInterface* renderer) = 0;
+ // Make an unqualified VideoSourceInterface resolve to
+ // webrtc::VideoSourceInterface, not our base class
+ // rtc::VideoSourceInterface<cricket::VideoFrame>.
+ using VideoSourceInterface = webrtc::VideoSourceInterface;
+
+ // AddRenderer and RemoveRenderer are for backwards compatibility
+ // only. They are obsoleted by the methods of
+ // rtc::VideoSourceInterface.
+ virtual void AddRenderer(VideoRendererInterface* renderer) {
+ AddOrUpdateSink(renderer, rtc::VideoSinkWants());
+ }
+ virtual void RemoveRenderer(VideoRendererInterface* renderer) {
+ RemoveSink(renderer);
+ }
+
+ // Register a video sink for this track.
+ void AddOrUpdateSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) override{};
+ void RemoveSink(
+ rtc::VideoSinkInterface<cricket::VideoFrame>* sink) override{};
virtual VideoSourceInterface* GetSource() const = 0;
diff --git a/webrtc/api/mediastreamtrackproxy.h b/webrtc/api/mediastreamtrackproxy.h
index 5b29a0e..dca2d15 100644
--- a/webrtc/api/mediastreamtrackproxy.h
+++ b/webrtc/api/mediastreamtrackproxy.h
@@ -48,6 +48,11 @@
PROXY_METHOD1(void, AddRenderer, VideoRendererInterface*)
PROXY_METHOD1(void, RemoveRenderer, VideoRendererInterface*)
+ PROXY_METHOD2(void,
+ AddOrUpdateSink,
+ rtc::VideoSinkInterface<cricket::VideoFrame>*,
+ const rtc::VideoSinkWants&)
+ PROXY_METHOD1(void, RemoveSink, rtc::VideoSinkInterface<cricket::VideoFrame>*)
PROXY_CONSTMETHOD0(VideoSourceInterface*, GetSource)
PROXY_METHOD0(rtc::VideoSinkInterface<cricket::VideoFrame>*, GetSink)
diff --git a/webrtc/api/test/fakevideotrackrenderer.h b/webrtc/api/test/fakevideotrackrenderer.h
index 816b447..387ce51 100644
--- a/webrtc/api/test/fakevideotrackrenderer.h
+++ b/webrtc/api/test/fakevideotrackrenderer.h
@@ -16,18 +16,43 @@
namespace webrtc {
-class FakeVideoTrackRenderer : public VideoRendererInterface {
+class FakeVideoTrackRenderer
+ : public rtc::VideoSinkInterface<cricket::VideoFrame> {
public:
FakeVideoTrackRenderer(VideoTrackInterface* video_track)
- : video_track_(video_track), last_frame_(NULL) {
- video_track_->AddRenderer(this);
+ : video_track_(video_track) {
+ video_track_->AddOrUpdateSink(this, rtc::VideoSinkWants());
}
- ~FakeVideoTrackRenderer() {
- video_track_->RemoveRenderer(this);
+ ~FakeVideoTrackRenderer() { video_track_->RemoveSink(this); }
+
+ virtual void OnFrame(const cricket::VideoFrame& video_frame) override {
+ fake_renderer_.RenderFrame(&video_frame);
}
+ int errors() const { return fake_renderer_.errors(); }
+ int width() const { return fake_renderer_.width(); }
+ int height() const { return fake_renderer_.height(); }
+ bool black_frame() const { return fake_renderer_.black_frame(); }
+
+ int num_rendered_frames() const {
+ return fake_renderer_.num_rendered_frames();
+ }
+
+ private:
+ cricket::FakeVideoRenderer fake_renderer_;
+ rtc::scoped_refptr<VideoTrackInterface> video_track_;
+};
+
+// Similar class, testing the deprecated AddRenderer/RemoveRenderer methods.
+class FakeVideoTrackRendererOld : public VideoRendererInterface {
+ public:
+ FakeVideoTrackRendererOld(VideoTrackInterface* video_track)
+ : video_track_(video_track) {
+ video_track_->AddRenderer(this);
+ }
+ ~FakeVideoTrackRendererOld() { video_track_->RemoveRenderer(this); }
+
virtual void RenderFrame(const cricket::VideoFrame* video_frame) override {
- last_frame_ = const_cast<cricket::VideoFrame*>(video_frame);
fake_renderer_.RenderFrame(video_frame);
}
@@ -39,14 +64,10 @@
int num_rendered_frames() const {
return fake_renderer_.num_rendered_frames();
}
- const cricket::VideoFrame* last_frame() const { return last_frame_; }
private:
cricket::FakeVideoRenderer fake_renderer_;
rtc::scoped_refptr<VideoTrackInterface> video_track_;
-
- // Weak reference for frame pointer comparison only.
- cricket::VideoFrame* last_frame_;
};
} // namespace webrtc
diff --git a/webrtc/api/videotrack.cc b/webrtc/api/videotrack.cc
index 577ac68..11a0177 100644
--- a/webrtc/api/videotrack.cc
+++ b/webrtc/api/videotrack.cc
@@ -33,12 +33,15 @@
return kVideoKind;
}
-void VideoTrack::AddRenderer(VideoRendererInterface* renderer) {
- renderers_.AddRenderer(renderer);
+void VideoTrack::AddOrUpdateSink(
+ rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) {
+ renderers_.AddOrUpdateSink(sink, wants);
}
-void VideoTrack::RemoveRenderer(VideoRendererInterface* renderer) {
- renderers_.RemoveRenderer(renderer);
+void VideoTrack::RemoveSink(
+ rtc::VideoSinkInterface<cricket::VideoFrame>* sink) {
+ renderers_.RemoveSink(sink);
}
rtc::VideoSinkInterface<cricket::VideoFrame>* VideoTrack::GetSink() {
@@ -51,7 +54,8 @@
}
rtc::scoped_refptr<VideoTrack> VideoTrack::Create(
- const std::string& id, VideoSourceInterface* source) {
+ const std::string& id,
+ VideoSourceInterface* source) {
rtc::RefCountedObject<VideoTrack>* track =
new rtc::RefCountedObject<VideoTrack>(id, source);
return track;
diff --git a/webrtc/api/videotrack.h b/webrtc/api/videotrack.h
index f2b8f19..1964cb4 100644
--- a/webrtc/api/videotrack.h
+++ b/webrtc/api/videotrack.h
@@ -22,11 +22,13 @@
class VideoTrack : public MediaStreamTrack<VideoTrackInterface> {
public:
- static rtc::scoped_refptr<VideoTrack> Create(
- const std::string& label, VideoSourceInterface* source);
+ static rtc::scoped_refptr<VideoTrack> Create(const std::string& label,
+ VideoSourceInterface* source);
- virtual void AddRenderer(VideoRendererInterface* renderer);
- virtual void RemoveRenderer(VideoRendererInterface* renderer);
+ void AddOrUpdateSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) override;
+ void RemoveSink(rtc::VideoSinkInterface<cricket::VideoFrame>* sink) override;
+
virtual VideoSourceInterface* GetSource() const {
return video_source_.get();
}
diff --git a/webrtc/api/videotrack_unittest.cc b/webrtc/api/videotrack_unittest.cc
index 0f82e18..df41258 100644
--- a/webrtc/api/videotrack_unittest.cc
+++ b/webrtc/api/videotrack_unittest.cc
@@ -21,6 +21,7 @@
#include "webrtc/pc/channelmanager.h"
using webrtc::FakeVideoTrackRenderer;
+using webrtc::FakeVideoTrackRendererOld;
using webrtc::VideoSource;
using webrtc::VideoTrack;
using webrtc::VideoTrackInterface;
@@ -86,6 +87,47 @@
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);
+
+ EXPECT_EQ(2, renderer_1->num_rendered_frames());
+ EXPECT_EQ(2, renderer_2->num_rendered_frames());
+}
+
+// Test adding renderers to a video track and render to them by
+// providing frames to the source. Uses the old VideoTrack interface
+// with AddRenderer and RemoveRenderer.
+TEST_F(VideoTrackTest, RenderVideoOld) {
+ // FakeVideoTrackRenderer register itself to |video_track_|
+ 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);
+ 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());
+
+ 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);
diff --git a/webrtc/api/videotrackrenderers.cc b/webrtc/api/videotrackrenderers.cc
index 4800d3b..9e928ef 100644
--- a/webrtc/api/videotrackrenderers.cc
+++ b/webrtc/api/videotrackrenderers.cc
@@ -19,17 +19,21 @@
VideoTrackRenderers::~VideoTrackRenderers() {
}
-void VideoTrackRenderers::AddRenderer(VideoRendererInterface* renderer) {
- if (!renderer) {
- return;
- }
+void VideoTrackRenderers::AddOrUpdateSink(
+ VideoSinkInterface<cricket::VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) {
+ // TODO(nisse): Currently ignores wants. We should somehow use
+ // VideoBroadcaster, but we need to sort out its threading issues
+ // first.
rtc::CritScope cs(&critical_section_);
- renderers_.insert(renderer);
+ if (std::find(sinks_.begin(), sinks_.end(), sink) == sinks_.end())
+ sinks_.push_back(sink);
}
-void VideoTrackRenderers::RemoveRenderer(VideoRendererInterface* renderer) {
+void VideoTrackRenderers::RemoveSink(
+ VideoSinkInterface<cricket::VideoFrame>* sink) {
rtc::CritScope cs(&critical_section_);
- renderers_.erase(renderer);
+ sinks_.erase(std::remove(sinks_.begin(), sinks_.end(), sink), sinks_.end());
}
void VideoTrackRenderers::SetEnabled(bool enable) {
@@ -41,7 +45,7 @@
{
rtc::CritScope cs(&critical_section_);
if (enabled_) {
- RenderFrameToRenderers(frame);
+ RenderFrameToSinks(*frame);
return true;
}
}
@@ -64,17 +68,16 @@
// enabled while we generated the black frame. I think the
// enabled-ness ought to be applied at the track output, and hence
// an enabled track shouldn't send any blacked out frames.
- RenderFrameToRenderers(enabled_ ? frame : &black);
+ RenderFrameToSinks(enabled_ ? *frame : black);
return true;
}
}
// Called with critical_section_ already locked
-void VideoTrackRenderers::RenderFrameToRenderers(
- const cricket::VideoFrame* frame) {
- for (VideoRendererInterface* renderer : renderers_) {
- renderer->RenderFrame(frame);
+void VideoTrackRenderers::RenderFrameToSinks(const cricket::VideoFrame& frame) {
+ for (auto sink : sinks_) {
+ sink->OnFrame(frame);
}
}
diff --git a/webrtc/api/videotrackrenderers.h b/webrtc/api/videotrackrenderers.h
index 46f0c2f..04e1ad1 100644
--- a/webrtc/api/videotrackrenderers.h
+++ b/webrtc/api/videotrackrenderers.h
@@ -25,7 +25,9 @@
// Each VideoTrack owns a VideoTrackRenderers instance.
// The class is thread safe. Rendering to the added VideoRendererInterfaces is
// done on the same thread as the cricket::VideoRenderer.
-class VideoTrackRenderers : public cricket::VideoRenderer {
+class VideoTrackRenderers
+ : public cricket::VideoRenderer,
+ public rtc::VideoSourceInterface<cricket::VideoFrame> {
public:
VideoTrackRenderers();
~VideoTrackRenderers();
@@ -34,18 +36,18 @@
// incoming frames are replaced by black frames.
virtual bool RenderFrame(const cricket::VideoFrame* frame);
- void AddRenderer(VideoRendererInterface* renderer);
- void RemoveRenderer(VideoRendererInterface* renderer);
+ void AddOrUpdateSink(VideoSinkInterface<cricket::VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) override;
+ void RemoveSink(VideoSinkInterface<cricket::VideoFrame>* sink) override;
void SetEnabled(bool enable);
private:
// Pass the frame on to to each registered renderer. Requires
// critical_section_ already locked.
- void RenderFrameToRenderers(const cricket::VideoFrame* frame);
+ void RenderFrameToSinks(const cricket::VideoFrame& frame);
bool enabled_;
- std::set<VideoRendererInterface*> renderers_;
-
+ std::vector<VideoSinkInterface<cricket::VideoFrame>*> sinks_;
rtc::CriticalSection critical_section_; // Protects the above variables
};
diff --git a/webrtc/examples/peerconnection/client/linux/main_wnd.cc b/webrtc/examples/peerconnection/client/linux/main_wnd.cc
index cf98c1c..5a9c268 100644
--- a/webrtc/examples/peerconnection/client/linux/main_wnd.cc
+++ b/webrtc/examples/peerconnection/client/linux/main_wnd.cc
@@ -460,11 +460,11 @@
height_(0),
main_wnd_(main_wnd),
rendered_track_(track_to_render) {
- rendered_track_->AddRenderer(this);
+ rendered_track_->AddOrUpdateSink(this, rtc::VideoSinkWants());
}
GtkMainWnd::VideoRenderer::~VideoRenderer() {
- rendered_track_->RemoveRenderer(this);
+ rendered_track_->RemoveSink(this);
}
void GtkMainWnd::VideoRenderer::SetSize(int width, int height) {
@@ -480,11 +480,11 @@
gdk_threads_leave();
}
-void GtkMainWnd::VideoRenderer::RenderFrame(
- const cricket::VideoFrame* video_frame) {
+void GtkMainWnd::VideoRenderer::OnFrame(
+ const cricket::VideoFrame& video_frame) {
gdk_threads_enter();
- const cricket::VideoFrame* frame = video_frame->GetCopyWithRotationApplied();
+ const cricket::VideoFrame* frame = video_frame.GetCopyWithRotationApplied();
SetSize(static_cast<int>(frame->GetWidth()),
static_cast<int>(frame->GetHeight()));
@@ -511,5 +511,3 @@
g_idle_add(Redraw, main_wnd_);
}
-
-
diff --git a/webrtc/examples/peerconnection/client/linux/main_wnd.h b/webrtc/examples/peerconnection/client/linux/main_wnd.h
index e35d4dd..a7eb25e 100644
--- a/webrtc/examples/peerconnection/client/linux/main_wnd.h
+++ b/webrtc/examples/peerconnection/client/linux/main_wnd.h
@@ -71,7 +71,7 @@
void OnRedraw();
protected:
- class VideoRenderer : public webrtc::VideoRendererInterface {
+ class VideoRenderer : public rtc::VideoSinkInterface<cricket::VideoFrame> {
public:
VideoRenderer(GtkMainWnd* main_wnd,
webrtc::VideoTrackInterface* track_to_render);
@@ -79,7 +79,7 @@
// VideoRendererInterface implementation
virtual void SetSize(int width, int height);
- virtual void RenderFrame(const cricket::VideoFrame* frame);
+ virtual void OnFrame(const cricket::VideoFrame& frame);
const uint8_t* image() const { return image_.get(); }