Camera communication is on a separate thread. UI threads takes latest frame explicitly on timer.

BUG=chromiumos:7085
TEST=Verify that video capturing doesn't flicker and is smooth while UI is responsible enough.

Review URL: http://codereview.chromium.org/3874002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63757 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 154 insertions, 106 deletions

diff --git a/chrome/browser/chromeos/login/camera.cc b/chrome/browser/chromeos/login/camera.cc
index 6de6075..18d6ce3 100644
--- a/chrome/browser/chromeos/login/camera.cc
+++ b/chrome/browser/chromeos/login/camera.cc
@@ -113,6 +113,8 @@ gfx::Size get_best_frame_size(int fd,
 
 // Default camera device name.
 const char kDeviceName[] = "/dev/video0";
+// Name for camera thread.
+const char kCameraThreadName[] = "Chrome_CameraThread";
 // Default width of each frame received from the camera.
 const int kFrameWidth = 640;
 // Default height of each frame received from the camera.
@@ -124,11 +126,18 @@ const long kSelectTimeout = 1 * base::Time::kMicrosecondsPerSecond;
 
 }  // namespace
 
+// static
+Lock Camera::image_lock_;
+
+// static
+Lock Camera::thread_lock_;
+
 ///////////////////////////////////////////////////////////////////////////////
 // Camera, public members:
 
 Camera::Camera(Delegate* delegate, bool mirrored)
     : delegate_(delegate),
+      camera_thread_(kCameraThreadName),
       device_name_(kDeviceName),
       device_descriptor_(-1),
       is_capturing_(false),
@@ -144,7 +153,7 @@ Camera::~Camera() {
 }
 
 void Camera::ReportFailure() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   if (device_descriptor_ == -1) {
     BrowserThread::PostTask(
         BrowserThread::UI,
@@ -168,9 +177,7 @@ void Camera::ReportFailure() {
 
 void Camera::Initialize(int desired_width, int desired_height) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
+  PostCameraTask(
       NewRunnableMethod(this,
                         &Camera::DoInitialize,
                         desired_width,
@@ -178,7 +185,7 @@ void Camera::Initialize(int desired_width, int desired_height) {
 }
 
 void Camera::DoInitialize(int desired_width, int desired_height) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   DCHECK(delegate_);
 
   if (device_descriptor_ != -1) {
@@ -246,15 +253,11 @@ void Camera::DoInitialize(int desired_width, int desired_height) {
 
 void Camera::Uninitialize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::DoUninitialize));
+  PostCameraTask(NewRunnableMethod(this, &Camera::DoUninitialize));
 }
 
 void Camera::DoUninitialize() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   if (device_descriptor_ == -1) {
     LOG(WARNING) << "Calling uninitialize for uninitialized camera.";
     return;
@@ -266,18 +269,13 @@ void Camera::DoUninitialize() {
   device_descriptor_ = -1;
 }
 
-void Camera::StartCapturing(int64 rate) {
+void Camera::StartCapturing() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::DoStartCapturing,
-                        rate));
+  PostCameraTask(NewRunnableMethod(this, &Camera::DoStartCapturing));
 }
 
-void Camera::DoStartCapturing(int64 rate) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+void Camera::DoStartCapturing() {
+  DCHECK(IsOnCameraThread());
   if (is_capturing_) {
     LOG(WARNING) << "Capturing is already started.";
     return;
@@ -306,25 +304,16 @@ void Camera::DoStartCapturing(int64 rate) {
       NewRunnableMethod(this,
                         &Camera::OnStartCapturingSuccess));
   is_capturing_ = true;
-  capturing_rate_ = rate;
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::OnCapture));
+  PostCameraTask(NewRunnableMethod(this, &Camera::OnCapture));
 }
 
 void Camera::StopCapturing() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::DoStopCapturing));
+  PostCameraTask(NewRunnableMethod(this, &Camera::DoStopCapturing));
 }
 
 void Camera::DoStopCapturing() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   if (!is_capturing_) {
     LOG(WARNING) << "Calling StopCapturing when capturing is not started.";
     return;
@@ -336,11 +325,16 @@ void Camera::DoStopCapturing() {
     log_errno("VIDIOC_STREAMOFF failed.");
 }
 
+void Camera::GetFrame(SkBitmap* frame) {
+  AutoLock lock(image_lock_);
+  frame->swap(frame_image_);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Camera, private members:
 
 int Camera::OpenDevice(const char* device_name) const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   struct stat st;
   if (stat(device_name, &st) == -1) {
     log_errno(base::StringPrintf("Cannot identify %s", device_name));
@@ -359,7 +353,7 @@ int Camera::OpenDevice(const char* device_name) const {
 }
 
 bool Camera::InitializeReadingMode(int fd) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   v4l2_requestbuffers req;
   req.count = kRequestBuffersCount;
   req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
@@ -404,7 +398,7 @@ bool Camera::InitializeReadingMode(int fd) {
 }
 
 void Camera::UnmapVideoBuffers() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   for (size_t i = 0; i < buffers_.size(); ++i) {
     if (munmap(buffers_[i].start, buffers_[i].length) == -1)
       log_errno("munmap failed.");
@@ -412,7 +406,7 @@ void Camera::UnmapVideoBuffers() {
 }
 
 void Camera::OnCapture() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   if (!is_capturing_)
     return;
 
@@ -441,16 +435,11 @@ void Camera::OnCapture() {
     // EAGAIN - continue select loop.
   } while (!ReadFrame());
 
-  BrowserThread::PostDelayedTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::OnCapture),
-      capturing_rate_);
+  PostCameraTask(NewRunnableMethod(this, &Camera::OnCapture));
 }
 
 bool Camera::ReadFrame() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   v4l2_buffer buffer = {};
   buffer.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   buffer.memory = V4L2_MEMORY_MMAP;
@@ -475,7 +464,7 @@ bool Camera::ReadFrame() {
 }
 
 void Camera::ProcessImage(void* data) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(IsOnCameraThread());
   // If desired resolution is higher than available, we crop the available
   // image to get the same aspect ratio and scale the result.
   int desired_width = desired_width_;
@@ -538,12 +527,14 @@ void Camera::ProcessImage(void* data) {
         desired_height_);
   }
   image.setIsOpaque(true);
+  {
+    AutoLock lock(image_lock_);
+    frame_image_.swap(image);
+  }
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
-      NewRunnableMethod(this,
-                        &Camera::OnCaptureSuccess,
-                        image));
+      NewRunnableMethod(this, &Camera::OnCaptureSuccess));
 }
 
 void Camera::OnInitializeSuccess() {
@@ -570,10 +561,10 @@ void Camera::OnStartCapturingFailure() {
     delegate_->OnStartCapturingFailure();
 }
 
-void Camera::OnCaptureSuccess(const SkBitmap& frame) {
+void Camera::OnCaptureSuccess() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (delegate_)
-    delegate_->OnCaptureSuccess(frame);
+    delegate_->OnCaptureSuccess();
 }
 
 void Camera::OnCaptureFailure() {
@@ -582,4 +573,16 @@ void Camera::OnCaptureFailure() {
     delegate_->OnCaptureFailure();
 }
 
+bool Camera::IsOnCameraThread() const {
+  AutoLock lock(thread_lock_);
+  return MessageLoop::current() == camera_thread_.message_loop();
+}
+
+void Camera::PostCameraTask(Task* task) {
+  AutoLock lock(thread_lock_);
+  if (!camera_thread_.IsRunning())
+    camera_thread_.Start();
+  camera_thread_.message_loop()->PostTask(FROM_HERE, task);
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/camera.h b/chrome/browser/chromeos/login/camera.h
index 667d9d6..38e7e19 100644
--- a/chrome/browser/chromeos/login/camera.h
+++ b/chrome/browser/chromeos/login/camera.h
@@ -9,11 +9,12 @@
 #include <string>
 #include <vector>
 
+#include "base/lock.h"
 #include "base/ref_counted.h"
-#include "base/timer.h"
-
-class SkBitmap;
+#include "base/thread.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
+class Task;
 namespace base {
 class TimeDelta;
 }  // namespace base
@@ -22,8 +23,8 @@ namespace chromeos {
 
 // Class that wraps interaction with video capturing device. Returns
 // frames captured with specified intervals of time via delegate interface.
-// All communication with camera driver is performed on IO thread.
-// Delegate's callback are called on UI thread.
+// All communication with camera driver is performed on a separate camera
+// thread. Delegate's callback are called on UI thread.
 class Camera : public base::RefCountedThreadSafe<Camera> {
  public:
   class Delegate {
@@ -39,9 +40,11 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
     virtual void OnStartCapturingSuccess() = 0;
     virtual void OnStartCapturingFailure() = 0;
 
-    // Called if video frame was captured successfully.
-    virtual void OnCaptureSuccess(const SkBitmap& frame) = 0;
-    // Called if capturing the current frame failed.
+    // Notifies the delegate that new frame was captured.
+    // The frame can be obtained via GetFrame() method.
+    virtual void OnCaptureSuccess() = 0;
+
+    // Notifies the delegate that we failed to capture the next frame.
     virtual void OnCaptureFailure() = 0;
   };
 
@@ -50,20 +53,19 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // determines if the returned video image is mirrored horizontally.
   Camera(Delegate* delegate, bool mirrored);
 
-  // Initializes camera device on IO thread. Corresponding delegate's callback
-  // is called on UI thread to notify about success or failure. Does nothing if
-  // camera is successfully initialized already. Sets the desired width and
-  // height of the frame to receive from camera.
+  // Initializes camera device on camera thread. Corresponding delegate's
+  // callback is called on UI thread to notify about success or failure. Does
+  // nothing if camera is successfully initialized already. Sets the desired
+  // width and height of the frame to receive from camera.
   void Initialize(int desired_width, int desired_height);
 
-  // Uninitializes the camera on IO thread. Can be called anytime, any number
-  // of times.
+  // Uninitializes the camera on camera thread. Can be called anytime, any
+  // number of times.
   void Uninitialize();
 
-  // Starts capturing video frames with specified interval, in ms. Calls the
-  // corresponding method of delegate to report about success or failure. If
-  // succeeded, subsequent call doesn't do anything.
-  void StartCapturing(int64 rate);
+  // Starts capturing video frames on camera thread. Frames can be retrieved
+  // by calling GetFrame method.
+  void StartCapturing();
 
   // Stops capturing video frames. Can be called anytime, any number of
   // times.
@@ -73,6 +75,9 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // be destroyed.
   void set_delegate(Delegate* delegate) { delegate_ = delegate; }
 
+  // Returns the last successful frame in the member passed.
+  void GetFrame(SkBitmap* frame);
+
  private:
   // Destructor is private so only its base class can delete Camera objects.
   ~Camera();
@@ -89,9 +94,9 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // Unmaps video buffers stored in |buffers_|.
   void UnmapVideoBuffers();
 
-  // Task for IO thread that queries camera about the next frame and sends it to
-  // |delegate_| via its method or reports failure. Schedules the next task
-  // for itself if capturing still takes place.
+  // Task for camera thread that queries camera about the next frame and
+  // saves it to |frame_image| buffer for UI thread to pick up. Schedules the
+  // next task for itself if capturing still takes place.
   void OnCapture();
 
   // Reads a frame from the video device. If retry is needed, returns false.
@@ -102,10 +107,10 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // size and notifies the delegate that the image is ready.
   void ProcessImage(void* data);
 
-  // Actual routines that run on IO thread and call delegate's callbacks. See
-  // the corresponding methods without Do prefix for details.
+  // Actual routines that run on camera thread and call delegate's callbacks.
+  // See the corresponding methods without Do prefix for details.
   void DoInitialize(int desired_width, int desired_height);
-  void DoStartCapturing(int64 rate);
+  void DoStartCapturing();
 
   // Helper method that reports failure to the delegate via method
   // corresponding to the current state of the object.
@@ -116,13 +121,20 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   void OnInitializeFailure();
   void OnStartCapturingSuccess();
   void OnStartCapturingFailure();
-  void OnCaptureSuccess(const SkBitmap& frame);
+  void OnCaptureSuccess();
   void OnCaptureFailure();
 
-  // IO thread routines that implement the corresponding public methods.
+  // Camera thread routines that implement the corresponding public methods.
   void DoUninitialize();
   void DoStopCapturing();
 
+  // Returns true if the code is executed on camera thread right now, false
+  // otherwise.
+  bool IsOnCameraThread() const;
+
+  // Posts task to camera thread.
+  void PostCameraTask(Task* task);
+
   // Defines a buffer in memory where one frame from the camera is stored.
   struct VideoBuffer {
     void* start;
@@ -133,7 +145,10 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // Delegate is accessed only on UI thread.
   Delegate* delegate_;
 
-  // All the members below are accessed only on IO thread.
+  // Thread where all work with the device is going on.
+  base::Thread camera_thread_;
+
+  // All the members below are accessed only on camera thread.
   // Name of the device file, i.e. "/dev/video0".
   std::string device_name_;
 
@@ -146,9 +161,6 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // Indicates if capturing has been started.
   bool is_capturing_;
 
-  // Rate at which camera device should be queried about new image, in ms.
-  int64 capturing_rate_;
-
   // Desired size of the frame to get from camera. If it doesn't match
   // camera's supported resolution, higher resolution is selected (if
   // available) and frame is cropped. If higher resolution is not available,
@@ -165,6 +177,15 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   // mimic mirror behavior.
   bool mirrored_;
 
+  // Image where camera frames are stored for UI thread to pick up.
+  SkBitmap frame_image_;
+
+  // Lock that guards references to |frame_image_|.
+  static Lock image_lock_;
+
+  // Lock that guards references to |camera_thread_|.
+  static Lock thread_lock_;
+
   DISALLOW_COPY_AND_ASSIGN(Camera);
 };
 
diff --git a/chrome/browser/chromeos/login/user_image_screen.cc b/chrome/browser/chromeos/login/user_image_screen.cc
index b54c47d..3aa68b0 100644
--- a/chrome/browser/chromeos/login/user_image_screen.cc
+++ b/chrome/browser/chromeos/login/user_image_screen.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/login/user_image_screen.h"
 
+#include "app/resource_bundle.h"
 #include "base/compiler_specific.h"
 #include "base/time.h"
 #include "chrome/browser/chromeos/login/login_utils.h"
@@ -13,6 +14,7 @@
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
+#include "grit/theme_resources.h"
 
 namespace chromeos {
 
@@ -22,16 +24,20 @@ namespace {
 const int kFrameWidth = 480;
 const int kFrameHeight = 480;
 
-// Frame rate in milliseconds for video capturing.
-// We want 25 FPS.
-const int64 kFrameRate = 40;
+// Maximum number of capture failures we ignore before we try to initialize
+// the camera again.
+const int kMaxCaptureFailureCounter = 5;
+
+// Maximum number of camera initialization retries.
+const int kMaxCameraInitFailureCounter = 3;
 
 }  // namespace
 
 UserImageScreen::UserImageScreen(WizardScreenDelegate* delegate)
     : ViewScreen<UserImageView>(delegate),
       ALLOW_THIS_IN_INITIALIZER_LIST(camera_(new Camera(this, true))),
-      camera_initialized_(false) {
+      capture_failure_counter_(0),
+      camera_init_failure_counter_(0) {
   registrar_.Add(
       this,
       NotificationType::SCREEN_LOCK_STATE_CHANGED,
@@ -45,12 +51,10 @@ UserImageScreen::~UserImageScreen() {
 }
 
 void UserImageScreen::Refresh() {
-  if (camera_.get() && camera_initialized_)
-    camera_->StartCapturing(kFrameRate);
 }
 
 void UserImageScreen::Hide() {
-  if (camera_.get() && camera_initialized_)
+  if (camera_.get())
     camera_->StopCapturing();
   ViewScreen<UserImageView>::Hide();
 }
@@ -60,36 +64,50 @@ UserImageView* UserImageScreen::AllocateView() {
 }
 
 void UserImageScreen::OnInitializeSuccess() {
-  camera_initialized_ = true;
   if (camera_.get())
-    camera_->StartCapturing(kFrameRate);
+    camera_->StartCapturing();
 }
 
 void UserImageScreen::OnInitializeFailure() {
-  if (view())
-    view()->ShowCameraError();
-  camera_initialized_ = false;
+  ++camera_init_failure_counter_;
+  if (camera_init_failure_counter_ > kMaxCameraInitFailureCounter) {
+    if (view())
+      view()->ShowCameraError();
+    return;
+  }
+  // Retry initializing the camera.
+  if (camera_.get()) {
+    camera_->Uninitialize();
+    camera_->Initialize(kFrameWidth, kFrameHeight);
+  }
 }
 
 void UserImageScreen::OnStartCapturingSuccess() {
 }
 
 void UserImageScreen::OnStartCapturingFailure() {
-  if (view())
-    view()->ShowCameraError();
+  // Try to reinitialize camera.
+  OnInitializeFailure();
 }
 
-void UserImageScreen::OnCaptureSuccess(const SkBitmap& frame) {
-  if (view())
-    view()->UpdateVideoFrame(frame);
+void UserImageScreen::OnCaptureSuccess() {
+  capture_failure_counter_ = 0;
+  camera_init_failure_counter_ = 0;
+  if (view() && camera_.get()) {
+    SkBitmap frame;
+    camera_->GetFrame(&frame);
+    if (!frame.isNull())
+      view()->UpdateVideoFrame(frame);
+  }
 }
 
 void UserImageScreen::OnCaptureFailure() {
-  // If camera failed to provide a picture we don't want to show broken
-  // camera image since it may lead to flicker if capturing fails and then
-  // works again.
-  // TODO(avayvod): Find a better way to handle such cases.
-  VLOG(1) << "Capturing image failed.";
+  ++capture_failure_counter_;
+  if (capture_failure_counter_ < kMaxCaptureFailureCounter)
+    return;
+
+  capture_failure_counter_ = 0;
+  OnInitializeFailure();
 }
 
 void UserImageScreen::OnOK(const SkBitmap& image) {
@@ -129,9 +147,9 @@ void UserImageScreen::Observe(NotificationType type,
 
   bool is_screen_locked = *Details<bool>(details).ptr();
   if (is_screen_locked)
-    camera_->StopCapturing();
+    camera_->Uninitialize();
   else
-    camera_->StartCapturing(kFrameRate);
+    camera_->Initialize(kFrameWidth, kFrameHeight);
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/user_image_screen.h b/chrome/browser/chromeos/login/user_image_screen.h
index c867023..bc4f984 100644
--- a/chrome/browser/chromeos/login/user_image_screen.h
+++ b/chrome/browser/chromeos/login/user_image_screen.h
@@ -33,7 +33,7 @@ class UserImageScreen: public ViewScreen<UserImageView>,
   virtual void OnInitializeFailure();
   virtual void OnStartCapturingSuccess();
   virtual void OnStartCapturingFailure();
-  virtual void OnCaptureSuccess(const SkBitmap& frame);
+  virtual void OnCaptureSuccess();
   virtual void OnCaptureFailure();
 
   // UserImageView::Delegate implementation:
@@ -46,11 +46,17 @@ class UserImageScreen: public ViewScreen<UserImageView>,
                        const NotificationDetails& details);
 
  private:
+  // Capturing timer callback that updates image from camera.
+  void OnCaptureTimer();
+
   // Object that handles video capturing.
   scoped_refptr<Camera> camera_;
 
-  // Indicates if camera is initialized.
-  bool camera_initialized_;
+  // Counts how many times in a row capture failed.
+  int capture_failure_counter_;
+
+  // Counts how many times camera initialization failed.
+  int camera_init_failure_counter_;
 
   NotificationRegistrar registrar_;