Add VideoSource::Stop and Restart methods.
The purpose is to make sure that start and stop is called on the correct thread on Android. It also cleans up the Java VideoSource implementation.
BUG=4303
R=glaznev@webrtc.org, magjed@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/39989004
Cr-Commit-Position: refs/heads/master@{#8389}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8389 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
index 72a592e..109c294 100644
--- a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
+++ b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
@@ -122,6 +122,8 @@
RendererCallbacks callbacks = new RendererCallbacks();
track.addRenderer(new VideoRenderer(callbacks));
assertTrue(callbacks.WaitForNextFrameToRender() > 0);
+ track.dispose();
+ source.dispose();
}
@Override
@@ -195,6 +197,32 @@
RendererCallbacks callbacks = new RendererCallbacks();
track.addRenderer(new VideoRenderer(callbacks));
assertTrue(callbacks.WaitForNextFrameToRender() > 0);
+ track.dispose();
+ source.dispose();
+ }
+
+ @SmallTest
+ // This test that the VideoSource that the VideoCapturer is connected to can
+ // be stopped and restarted. It tests both the Java and the C++ layer.
+ public void testStopRestartVideoSource() throws Exception {
+ PeerConnectionFactory factory = new PeerConnectionFactory();
+ VideoCapturerAndroid capturer = VideoCapturerAndroid.create("");
+ VideoSource source =
+ factory.createVideoSource(capturer, new MediaConstraints());
+ VideoTrack track = factory.createVideoTrack("dummy", source);
+ RendererCallbacks callbacks = new RendererCallbacks();
+ track.addRenderer(new VideoRenderer(callbacks));
+ assertTrue(callbacks.WaitForNextFrameToRender() > 0);
+ assertEquals(MediaSource.State.LIVE, source.state());
+
+ source.stop();
+ assertEquals(MediaSource.State.ENDED, source.state());
+
+ source.restart();
+ assertTrue(callbacks.WaitForNextFrameToRender() > 0);
+ assertEquals(MediaSource.State.LIVE, source.state());
+ track.dispose();
+ source.dispose();
}
@SmallTest
@@ -218,5 +246,6 @@
assertEquals((format.width*format.height*3)/2, observer.frameSize());
assertTrue(capturer.stopCapture());
}
+ capturer.dispose();
}
}
diff --git a/talk/app/webrtc/androidvideocapturer.cc b/talk/app/webrtc/androidvideocapturer.cc
index fa9d925..3264421 100644
--- a/talk/app/webrtc/androidvideocapturer.cc
+++ b/talk/app/webrtc/androidvideocapturer.cc
@@ -100,7 +100,8 @@
: running_(false),
delegate_(delegate.Pass()),
worker_thread_(NULL),
- frame_factory_(NULL) {
+ frame_factory_(NULL),
+ current_state_(cricket::CS_STOPPED){
std::string json_string = delegate_->GetSupportedFormats();
LOG(LS_INFO) << json_string;
@@ -132,7 +133,7 @@
cricket::CaptureState AndroidVideoCapturer::Start(
const cricket::VideoFormat& capture_format) {
DCHECK(!running_);
- DCHECK(worker_thread_ == nullptr);
+ DCHECK(worker_thread_ == nullptr || worker_thread_ == rtc::Thread::Current());
// TODO(perkj): Better way to get a handle to the worker thread?
worker_thread_ = rtc::Thread::Current();
@@ -146,7 +147,9 @@
delegate_->Start(
capture_format.width, capture_format.height,
cricket::VideoFormat::IntervalToFps(capture_format.interval), this);
- return cricket::CS_STARTING;
+ SetCaptureFormat(&capture_format);
+ current_state_ = cricket::CS_STARTING;
+ return current_state_;
}
void AndroidVideoCapturer::Stop() {
@@ -157,7 +160,8 @@
SetCaptureFormat(NULL);
delegate_->Stop();
- SignalStateChange(this, cricket::CS_STOPPED);
+ current_state_ = cricket::CS_STOPPED;
+ SignalStateChange(this, current_state_);
}
bool AndroidVideoCapturer::IsRunning() {
@@ -180,7 +184,14 @@
DCHECK(worker_thread_->IsCurrent());
cricket::CaptureState new_state =
success ? cricket::CS_RUNNING : cricket::CS_FAILED;
- SetCaptureState(new_state);
+ if (new_state == current_state_)
+ return;
+ current_state_ = new_state;
+
+ // TODO(perkj): SetCaptureState can not be used since it posts to |thread_|.
+ // But |thread_ | is currently just the thread that happened to create the
+ // cricket::VideoCapturer.
+ SignalStateChange(this, new_state);
}
void AndroidVideoCapturer::OnIncomingFrame(signed char* videoFrame,
diff --git a/talk/app/webrtc/androidvideocapturer.h b/talk/app/webrtc/androidvideocapturer.h
index efbe059..ef72cd6 100644
--- a/talk/app/webrtc/androidvideocapturer.h
+++ b/talk/app/webrtc/androidvideocapturer.h
@@ -102,6 +102,8 @@
class FrameFactory;
FrameFactory* frame_factory_; // Owned by cricket::VideoCapturer.
+
+ cricket::CaptureState current_state_;
};
} // namespace webrtc
diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc
index bddd417..1851c391 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -2950,28 +2950,13 @@
}
}
-JOW(jlong, VideoSource_stop)(JNIEnv* jni, jclass, jlong j_p) {
- cricket::VideoCapturer* capturer =
- reinterpret_cast<VideoSourceInterface*>(j_p)->GetVideoCapturer();
- scoped_ptr<cricket::VideoFormatPod> format(
- new cricket::VideoFormatPod(*capturer->GetCaptureFormat()));
- capturer->Stop();
- return jlongFromPointer(format.release());
+JOW(void, VideoSource_stop)(JNIEnv* jni, jclass, jlong j_p) {
+ reinterpret_cast<VideoSourceInterface*>(j_p)->Stop();
}
JOW(void, VideoSource_restart)(
JNIEnv* jni, jclass, jlong j_p_source, jlong j_p_format) {
- CHECK(j_p_source);
- CHECK(j_p_format);
- scoped_ptr<cricket::VideoFormatPod> format(
- reinterpret_cast<cricket::VideoFormatPod*>(j_p_format));
- reinterpret_cast<VideoSourceInterface*>(j_p_source)->GetVideoCapturer()->
- StartCapturing(cricket::VideoFormat(*format));
-}
-
-JOW(void, VideoSource_freeNativeVideoFormat)(
- JNIEnv* jni, jclass, jlong j_p) {
- delete reinterpret_cast<cricket::VideoFormatPod*>(j_p);
+ reinterpret_cast<VideoSourceInterface*>(j_p_source)->Restart();
}
JOW(jstring, MediaStreamTrack_nativeId)(JNIEnv* jni, jclass, jlong j_p) {
diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoCapturerAndroid.java b/talk/app/webrtc/java/src/org/webrtc/VideoCapturerAndroid.java
index f35b629..2650e71 100644
--- a/talk/app/webrtc/java/src/org/webrtc/VideoCapturerAndroid.java
+++ b/talk/app/webrtc/java/src/org/webrtc/VideoCapturerAndroid.java
@@ -422,8 +422,9 @@
@Override public void run() {
stopCaptureOnCameraThread(result);
}
- });
+ });
boolean status = exchange(result, false); // |false| is a dummy value here.
+ Log.d(TAG, "stopCapture wait");
try {
cameraThread.join();
} catch (InterruptedException e) {
diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoSource.java b/talk/app/webrtc/java/src/org/webrtc/VideoSource.java
index 9df236ce..7151748 100644
--- a/talk/app/webrtc/java/src/org/webrtc/VideoSource.java
+++ b/talk/app/webrtc/java/src/org/webrtc/VideoSource.java
@@ -36,7 +36,6 @@
* its output to the encoder) can be too high to bear.
*/
public class VideoSource extends MediaSource {
- private long nativeVideoFormatAtStop;
public VideoSource(long nativeSource) {
super(nativeSource);
@@ -44,30 +43,21 @@
// Stop capture feeding this source.
public void stop() {
- nativeVideoFormatAtStop = stop(nativeSource);
+ stop(nativeSource);
}
// Restart capture feeding this source. stop() must have been called since
// the last call to restart() (if any). Note that this isn't "start()";
// sources are started by default at birth.
public void restart() {
- restart(nativeSource, nativeVideoFormatAtStop);
- nativeVideoFormatAtStop = 0;
+ restart(nativeSource);
}
@Override
public void dispose() {
- if (nativeVideoFormatAtStop != 0) {
- freeNativeVideoFormat(nativeVideoFormatAtStop);
- nativeVideoFormatAtStop = 0;
- }
super.dispose();
}
- // This stop() returns an owned C++ VideoFormat pointer for use in restart()
- // and dispose().
- private static native long stop(long nativeSource);
- private static native void restart(
- long nativeSource, long nativeVideoFormatAtStop);
- private static native void freeNativeVideoFormat(long nativeVideoFormat);
+ private static native void stop(long nativeSource);
+ private static native void restart(long nativeSource);
}
diff --git a/talk/app/webrtc/mediastreamhandler_unittest.cc b/talk/app/webrtc/mediastreamhandler_unittest.cc
index d41a933..6246643 100644
--- a/talk/app/webrtc/mediastreamhandler_unittest.cc
+++ b/talk/app/webrtc/mediastreamhandler_unittest.cc
@@ -85,6 +85,8 @@
virtual cricket::VideoCapturer* GetVideoCapturer() {
return &fake_capturer_;
}
+ virtual void Stop() {}
+ virtual void Restart() {}
virtual void AddSink(cricket::VideoRenderer* output) {}
virtual void RemoveSink(cricket::VideoRenderer* output) {}
virtual SourceState state() const { return state_; }
diff --git a/talk/app/webrtc/videosource.cc b/talk/app/webrtc/videosource.cc
index ea670f5..44f3e22 100644
--- a/talk/app/webrtc/videosource.cc
+++ b/talk/app/webrtc/videosource.cc
@@ -432,11 +432,27 @@
return frame_input_.get();
}
+void VideoSource::Stop() {
+ channel_manager_->StopVideoCapture(video_capturer_.get(), format_);
+}
+
+void VideoSource::Restart() {
+ if (!channel_manager_->StartVideoCapture(video_capturer_.get(), format_)) {
+ SetState(kEnded);
+ return;
+ }
+ for(cricket::VideoRenderer* sink : sinks_) {
+ channel_manager_->AddVideoRenderer(video_capturer_.get(), sink);
+ }
+}
+
void VideoSource::AddSink(cricket::VideoRenderer* output) {
+ sinks_.push_back(output);
channel_manager_->AddVideoRenderer(video_capturer_.get(), output);
}
void VideoSource::RemoveSink(cricket::VideoRenderer* output) {
+ sinks_.remove(output);
channel_manager_->RemoveVideoRenderer(video_capturer_.get(), output);
}
diff --git a/talk/app/webrtc/videosource.h b/talk/app/webrtc/videosource.h
index d845ebf..8253cba 100644
--- a/talk/app/webrtc/videosource.h
+++ b/talk/app/webrtc/videosource.h
@@ -28,6 +28,8 @@
#ifndef TALK_APP_WEBRTC_VIDEOSOURCE_H_
#define TALK_APP_WEBRTC_VIDEOSOURCE_H_
+#include <list>
+
#include "talk/app/webrtc/mediastreaminterface.h"
#include "talk/app/webrtc/notifier.h"
#include "talk/app/webrtc/videosourceinterface.h"
@@ -73,6 +75,10 @@
virtual cricket::VideoCapturer* GetVideoCapturer() {
return video_capturer_.get();
}
+
+ void Stop() override;
+ void Restart() override;
+
// |output| will be served video frames as long as the underlying capturer
// is running video frames.
virtual void AddSink(cricket::VideoRenderer* output);
@@ -93,6 +99,8 @@
rtc::scoped_ptr<cricket::VideoCapturer> video_capturer_;
rtc::scoped_ptr<cricket::VideoRenderer> frame_input_;
+ std::list<cricket::VideoRenderer*> sinks_;
+
cricket::VideoFormat format_;
cricket::VideoOptions options_;
SourceState state_;
diff --git a/talk/app/webrtc/videosource_unittest.cc b/talk/app/webrtc/videosource_unittest.cc
index e83e500..ac072bd 100644
--- a/talk/app/webrtc/videosource_unittest.cc
+++ b/talk/app/webrtc/videosource_unittest.cc
@@ -169,7 +169,7 @@
// Test that a VideoSource transition to kLive state when the capture
// device have started and kEnded if it is stopped.
// It also test that an output can receive video frames.
-TEST_F(VideoSourceTest, StartStop) {
+TEST_F(VideoSourceTest, CapturerStartStop) {
// Initialize without constraints.
CreateVideoSource();
EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
@@ -183,6 +183,30 @@
kMaxWaitMs);
}
+// Test that a VideoSource can be stopped and restarted.
+TEST_F(VideoSourceTest, StopRestart) {
+ // Initialize without constraints.
+ CreateVideoSource();
+ EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
+ kMaxWaitMs);
+
+ ASSERT_TRUE(capturer_->CaptureFrame());
+ EXPECT_EQ(1, renderer_.num_rendered_frames());
+
+ source_->Stop();
+ EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(),
+ kMaxWaitMs);
+
+ source_->Restart();
+ EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(),
+ kMaxWaitMs);
+
+ ASSERT_TRUE(capturer_->CaptureFrame());
+ EXPECT_EQ(2, renderer_.num_rendered_frames());
+
+ source_->Stop();
+}
+
// Test start stop with a remote VideoSource - the video source that has a
// RemoteVideoCapturer and takes video frames from FrameInput.
TEST_F(VideoSourceTest, StartStopRemote) {
diff --git a/talk/app/webrtc/videosourceinterface.h b/talk/app/webrtc/videosourceinterface.h
index 6f176aa..a90e3d5 100644
--- a/talk/app/webrtc/videosourceinterface.h
+++ b/talk/app/webrtc/videosourceinterface.h
@@ -43,6 +43,11 @@
// This can be used for receiving frames and state notifications.
// But it should not be used for starting or stopping capturing.
virtual cricket::VideoCapturer* GetVideoCapturer() = 0;
+
+ // Stop the video capturer.
+ virtual void Stop() = 0;
+ virtual void Restart() = 0;
+
// Adds |output| to the source to receive frames.
virtual void AddSink(cricket::VideoRenderer* output) = 0;
virtual void RemoveSink(cricket::VideoRenderer* output) = 0;
diff --git a/talk/app/webrtc/videosourceproxy.h b/talk/app/webrtc/videosourceproxy.h
index 3f6513b..677fa9c 100644
--- a/talk/app/webrtc/videosourceproxy.h
+++ b/talk/app/webrtc/videosourceproxy.h
@@ -39,6 +39,8 @@
BEGIN_PROXY_MAP(VideoSource)
PROXY_CONSTMETHOD0(SourceState, state)
PROXY_METHOD0(cricket::VideoCapturer*, GetVideoCapturer)
+ PROXY_METHOD0(void, Stop)
+ PROXY_METHOD0(void, Restart)
PROXY_METHOD1(void, AddSink, cricket::VideoRenderer*)
PROXY_METHOD1(void, RemoveSink, cricket::VideoRenderer*)
PROXY_CONSTMETHOD0(const cricket::VideoOptions*, options)
