From Video Capture, abolish OnFrameInfo and enable resolution changes

content/renderer/media/video_capture_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/video_capture_impl.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "content/child/child_process.h"
 #include "content/common/media/video_capture_messages.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/limits.h"
 
 namespace content {
 
 class VideoCaptureImpl::ClientBuffer
     : public base::RefCountedThreadSafe<ClientBuffer> {
  public:
   ClientBuffer(scoped_ptr<base::SharedMemory> buffer,
-               size_t buffer_size,
-               int frame_width,
-               int frame_height,
-               int frame_stride)
+               size_t buffer_size)
       : buffer(buffer.Pass()),
-        buffer_size(buffer_size),
-        frame_width(frame_width),
-        frame_height(frame_height),
-        frame_stride(frame_stride) {}
+        buffer_size(buffer_size) {}
   const scoped_ptr<base::SharedMemory> buffer;
   const size_t buffer_size;
-  const int frame_width;   // In pixels.
-  const int frame_height;  // In pixels.
-  const int frame_stride;  // In pixels.
 
  private:
   friend class base::RefCountedThreadSafe<ClientBuffer>;
 
   virtual ~ClientBuffer() {}
 
   DISALLOW_COPY_AND_ASSIGN(ClientBuffer);
 };
 
 bool VideoCaptureImpl::CaptureStarted() {
   return state_ == VIDEO_CAPTURE_STATE_STARTED;
 }
 
 int VideoCaptureImpl::CaptureWidth() {
-  return capture_format_.width;
+  return requested_params_.width;
 }
 
 int VideoCaptureImpl::CaptureHeight() {
-  return capture_format_.height;
+  return requested_params_.height;
 }
 
 int VideoCaptureImpl::CaptureFrameRate() {
-  return capture_format_.frame_rate;
+  return requested_params_.frame_rate;
 }
 
 VideoCaptureImpl::VideoCaptureImpl(
-    const media::VideoCaptureSessionId id,
+    const media::VideoCaptureSessionId session_id,
     base::MessageLoopProxy* capture_message_loop_proxy,
     VideoCaptureMessageFilter* filter)
     : VideoCapture(),
       message_filter_(filter),
       capture_message_loop_proxy_(capture_message_loop_proxy),
       io_message_loop_proxy_(ChildProcess::current()->io_message_loop_proxy()),
       device_id_(0),
+      session_id_(session_id),
       client_buffer_weak_this_factory_(this),
-      video_type_(media::PIXEL_FORMAT_I420),
-      device_info_available_(false),
       suspended_(false),
       state_(VIDEO_CAPTURE_STATE_STOPPED) {
   DCHECK(filter);
-  capture_format_.session_id = id;
 }
 
 VideoCaptureImpl::~VideoCaptureImpl() {}
 
 void VideoCaptureImpl::Init() {
   if (!io_message_loop_proxy_->BelongsToCurrentThread()) {
     io_message_loop_proxy_->PostTask(FROM_HERE,
         base::Bind(&VideoCaptureImpl::AddDelegateOnIOThread,
                    base::Unretained(this)));
   } else {
     AddDelegateOnIOThread();
   }
 }
 
 void VideoCaptureImpl::DeInit(base::Closure task) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoDeInitOnCaptureThread,
                  base::Unretained(this), task));
 }
 
 void VideoCaptureImpl::StartCapture(
     media::VideoCapture::EventHandler* handler,
-    const media::VideoCaptureCapability& capability) {
-  DCHECK_EQ(capability.color, media::PIXEL_FORMAT_I420);
-
+    const media::VideoCaptureParams& params) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoStartCaptureOnCaptureThread,
-                 base::Unretained(this), handler, capability));
+                 base::Unretained(this), handler, params));
 }
 
 void VideoCaptureImpl::StopCapture(media::VideoCapture::EventHandler* handler) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoStopCaptureOnCaptureThread,
                  base::Unretained(this), handler));
 }
 
 void VideoCaptureImpl::OnBufferCreated(
     base::SharedMemoryHandle handle,
     int length, int buffer_id) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoBufferCreatedOnCaptureThread,
                  base::Unretained(this), handle, length, buffer_id));
 }
 
-void VideoCaptureImpl::OnBufferReceived(int buffer_id, base::Time timestamp) {
+void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
+  capture_message_loop_proxy_->PostTask(FROM_HERE,
+      base::Bind(&VideoCaptureImpl::DoBufferDestroyedOnCaptureThread,
+                 base::Unretained(this), buffer_id));
+}
+
+void VideoCaptureImpl::OnBufferReceived(
+    int buffer_id,
+    base::Time timestamp,
+    const media::VideoCaptureFormat& format) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoBufferReceivedOnCaptureThread,
-                 base::Unretained(this), buffer_id, timestamp));
+                 base::Unretained(this), buffer_id, timestamp, format));
 }
 
 void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoStateChangedOnCaptureThread,
                  base::Unretained(this), state));
 }
 
-void VideoCaptureImpl::OnDeviceInfoReceived(
-    const media::VideoCaptureParams& device_info) {
-  capture_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::DoDeviceInfoReceivedOnCaptureThread,
-                 base::Unretained(this), device_info));
-}
-
-void VideoCaptureImpl::OnDeviceInfoChanged(
-    const media::VideoCaptureParams& device_info) {
-  capture_message_loop_proxy_->PostTask(FROM_HERE,
-      base::Bind(&VideoCaptureImpl::DoDeviceInfoChangedOnCaptureThread,
-                 base::Unretained(this), device_info));
-}
-
 void VideoCaptureImpl::OnDelegateAdded(int32 device_id) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoDelegateAddedOnCaptureThread,
                  base::Unretained(this), device_id));
 }
 
 void VideoCaptureImpl::SuspendCapture(bool suspend) {
   capture_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::DoSuspendCaptureOnCaptureThread,
                  base::Unretained(this), suspend));
 }
 
 void VideoCaptureImpl::DoDeInitOnCaptureThread(base::Closure task) {
   if (state_ == VIDEO_CAPTURE_STATE_STARTED)
     Send(new VideoCaptureHostMsg_Stop(device_id_));
 
   io_message_loop_proxy_->PostTask(FROM_HERE,
       base::Bind(&VideoCaptureImpl::RemoveDelegateOnIOThread,
                  base::Unretained(this), task));
 }
 
 void VideoCaptureImpl::DoStartCaptureOnCaptureThread(
     media::VideoCapture::EventHandler* handler,
-    const media::VideoCaptureCapability& capability) {
+    const media::VideoCaptureParams& params) {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
   if (state_ == VIDEO_CAPTURE_STATE_ERROR) {
     handler->OnError(this, 1);
     handler->OnRemoved(this);
   } else if ((clients_pending_on_filter_.find(handler) !=
               clients_pending_on_filter_.end()) ||
              (clients_pending_on_restart_.find(handler) !=
               clients_pending_on_restart_.end()) ||
              clients_.find(handler) != clients_.end() ) {
     // This client has started.
   } else if (!device_id_) {
-    clients_pending_on_filter_[handler] = capability;
+    clients_pending_on_filter_[handler] = params;
   } else {
     handler->OnStarted(this);
     if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
-      // TODO(wjia): Temporarily disable restarting till client supports
-      // resampling.
-#if 0
-      if (capability.width > capture_format_.width ||
-          capability.height > capture_format_.height) {
-        StopDevice();
-        DVLOG(1) << "StartCapture: Got client with higher resolution ("
-                 << capability.width << ", " << capability.height << ") "
-                 << "after started, try to restart.";
-        clients_pending_on_restart_[handler] = capability;
-      } else {
-#endif
-      {
-        if (device_info_available_) {
-          handler->OnDeviceInfoReceived(this, device_info_);
-        }
+      if (last_device_info_.IsValid()) {
+        handler->OnDeviceInfoReceived(this, last_device_info_);
+      }
 
-        clients_[handler] = capability;
-      }
+      clients_[handler] = params;
     } else if (state_ == VIDEO_CAPTURE_STATE_STOPPING) {
-      clients_pending_on_restart_[handler] = capability;
+      clients_pending_on_restart_[handler] = params;
       DVLOG(1) << "StartCapture: Got new resolution ("
-               << capability.width << ", " << capability.height << ") "
+               << params.width << ", " << params.height << ") "
                << ", during stopping.";
     } else {
-      clients_[handler] = capability;
+      DCHECK_EQ(params.session_id, 0);

[sheu, 2013/10/12 00:37:52]

If we expect session_id to be 0, can we replace the media::VideoCaptureParams
parameter with a media::VideoCaptureFormat?

[ncarter (slow), 2013/10/16 02:08:40]

I've been tempted to do this (and have tried/reverted it several times), but it
doesn't quite work out cleanly. Taking a step back, the Params object is really
a kind of capability indicator for the consumer. Right now we model a Params
object as a single Format, but I expect.

Essentially, the pattern I want to go with is to use Params (which happens to
have a format) to model a request, and a Format to model the response. If you
change |requested_params_| and |clients_pending_on_filter_| and |clients_| to
all be Formats, it becomes very easy (and somewhat dangerous) to use the wrong
one.

So the session_id == 0 seems acceptable to me.

[sheu, 2013/10/18 01:48:43]

Have you thought about making that explicit then?   i.e.:

media::VideoCaptureRequest
media::VideoCaptureResponse

and have the Format struct as members as necessary?

[ncarter (slow), 2013/10/22 01:06:21]

The current patch set seems like a reasonable compromise for now. (Already the
latest patch set moves VideoCaptureParams away from inheriting from
VideoCaptureFormat; not clear if you'd seen that when you made this latest
comment?). Further discussion can go to http://crbug.com/297597 .

+      clients_[handler] = params;
       DCHECK_EQ(1ul, clients_.size());
-      video_type_ = capability.color;
-      int session_id = capture_format_.session_id;
-      DCHECK_EQ(capability.session_id, 0);
-      capture_format_ = capability;
-      capture_format_.session_id = session_id;
-      if (capture_format_.frame_rate > media::limits::kMaxFramesPerSecond)
-        capture_format_.frame_rate = media::limits::kMaxFramesPerSecond;
+      requested_params_ = params;
+      requested_params_.session_id = session_id_;
+      if (requested_params_.frame_rate > media::limits::kMaxFramesPerSecond)
+        requested_params_.frame_rate = media::limits::kMaxFramesPerSecond;
       DVLOG(1) << "StartCapture: starting with first resolution ("
-               << capture_format_.width << "," << capture_format_.height << ")";
+               << requested_params_.width << ","
+               << requested_params_.height << ")";
 
       StartCaptureInternal();
     }
   }
 }
 
 void VideoCaptureImpl::DoStopCaptureOnCaptureThread(
     media::VideoCapture::EventHandler* handler) {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
@@ skipping 17 matching lines @@
     int length, int buffer_id) {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
   // In case client calls StopCapture before the arrival of created buffer,
   // just close this buffer and return.
   if (state_ != VIDEO_CAPTURE_STATE_STARTED) {
     base::SharedMemory::CloseHandle(handle);
     return;
   }
 
-  DCHECK(device_info_available_);
-
   scoped_ptr<base::SharedMemory> shm(new base::SharedMemory(handle, false));
   if (!shm->Map(length)) {
     DLOG(ERROR) << "DoBufferCreatedOnCaptureThread: Map() failed.";
     return;
   }
 
   bool inserted =
       client_buffers_.insert(std::make_pair(
                                  buffer_id,
                                  new ClientBuffer(shm.Pass(),
-                                                  length,
-                                                  device_info_.width,
-                                                  device_info_.height,
-                                                  device_info_.width))).second;
+                                                  length))).second;
   DCHECK(inserted);
 }
 
+void VideoCaptureImpl::DoBufferDestroyedOnCaptureThread(int buffer_id) {
+  DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
+
+  ClientBufferMap::iterator iter = client_buffers_.find(buffer_id);
+  if (iter == client_buffers_.end())
+    return;
+
+  DCHECK(!iter->second || iter->second->HasOneRef())
+      << "Instructed to delete buffer we are still using.";
+  client_buffers_.erase(iter);
+}
+
 void VideoCaptureImpl::DoBufferReceivedOnCaptureThread(
-    int buffer_id, base::Time timestamp) {
+    int buffer_id,
+    base::Time timestamp,
+    const media::VideoCaptureFormat& format) {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
   if (state_ != VIDEO_CAPTURE_STATE_STARTED || suspended_) {
     Send(new VideoCaptureHostMsg_BufferReady(device_id_, buffer_id));
     return;
   }
 
+  // Synthesize deviceinfo notifications for listeners when the resolution
+  // changes.
+  if (!last_device_info_.IsValid()) {
+    for (ClientInfo::iterator i = clients_.begin(); i != clients_.end(); ++i) {
+      i->first->OnDeviceInfoReceived(this, format);
+    }
+  } else if (last_device_info_.width != format.width ||
+             last_device_info_.height != format.height) {
+    for (ClientInfo::iterator i = clients_.begin(); i != clients_.end(); ++i) {
+      i->first->OnDeviceInfoChanged(this, format);
+    }
+  }
+  last_device_info_ = format;
+
+  gfx::Size size(last_device_info_.width, last_device_info_.height);
+
   ClientBufferMap::iterator iter = client_buffers_.find(buffer_id);
   DCHECK(iter != client_buffers_.end());
   scoped_refptr<ClientBuffer> buffer = iter->second;
   scoped_refptr<media::VideoFrame> frame =
       media::VideoFrame::WrapExternalSharedMemory(
           media::VideoFrame::I420,
-          gfx::Size(buffer->frame_stride, buffer->frame_height),
-          gfx::Rect(0, 0, buffer->frame_width, buffer->frame_height),
-          gfx::Size(buffer->frame_width, buffer->frame_height),
+          size, gfx::Rect(size), size,
           reinterpret_cast<uint8*>(buffer->buffer->memory()),
           buffer->buffer_size,
           buffer->buffer->handle(),
           // TODO(sheu): convert VideoCaptureMessageFilter::Delegate to use
           // base::TimeTicks instead of base::Time.  http://crbug.com/249215
           timestamp - base::Time::UnixEpoch(),
           media::BindToLoop(
               capture_message_loop_proxy_,
               base::Bind(
                   &VideoCaptureImpl::DoClientBufferFinishedOnCaptureThread,
@@ skipping 50 matching lines @@
         it->first->OnRemoved(this);
       }
       clients_.clear();
       state_ = VIDEO_CAPTURE_STATE_ENDED;
       break;
     default:
       break;
   }
 }
 
-void VideoCaptureImpl::DoDeviceInfoReceivedOnCaptureThread(
-    const media::VideoCaptureParams& device_info) {
-  DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
-  DCHECK(client_buffers_.empty());
-
-  device_info_ = device_info;
-  device_info_available_ = true;
-  for (ClientInfo::iterator it = clients_.begin(); it != clients_.end(); ++it) {
-    it->first->OnDeviceInfoReceived(this, device_info);
-  }
-}
-
-void VideoCaptureImpl::DoDeviceInfoChangedOnCaptureThread(
-    const media::VideoCaptureParams& device_info) {
-  DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
-
-  for (ClientInfo::iterator it = clients_.begin(); it != clients_.end(); ++it) {
-    it->first->OnDeviceInfoChanged(this, device_info);
-  }
-}
-
 void VideoCaptureImpl::DoDelegateAddedOnCaptureThread(int32 device_id) {
   DVLOG(1) << "DoDelegateAdded: device_id " << device_id;
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
   device_id_ = device_id;
   for (ClientInfo::iterator it = clients_pending_on_filter_.begin();
        it != clients_pending_on_filter_.end(); ) {
     media::VideoCapture::EventHandler* handler = it->first;
-    const media::VideoCaptureCapability capability = it->second;
+    const media::VideoCaptureParams params = it->second;
     clients_pending_on_filter_.erase(it++);
-    StartCapture(handler, capability);
+    StartCapture(handler, params);
   }
 }
 
 void VideoCaptureImpl::DoSuspendCaptureOnCaptureThread(bool suspend) {
   DVLOG(1) << "DoSuspendCapture: suspend " << (suspend ? "yes" : "no");
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
   suspended_ = suspend;
 }
 
 void VideoCaptureImpl::StopDevice() {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
 
-  device_info_available_ = false;
+  last_device_info_ = media::VideoCaptureParams();
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
     state_ = VIDEO_CAPTURE_STATE_STOPPING;
     Send(new VideoCaptureHostMsg_Stop(device_id_));
-    capture_format_.width = capture_format_.height = 0;
+    requested_params_.width = requested_params_.height = 0;
   }
 }
 
 void VideoCaptureImpl::RestartCapture() {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
   DCHECK_EQ(state_, VIDEO_CAPTURE_STATE_STOPPED);
 
   int width = 0;
   int height = 0;
   for (ClientInfo::iterator it = clients_.begin();
        it != clients_.end(); ++it) {
     width = std::max(width, it->second.width);
     height = std::max(height, it->second.height);
   }
   for (ClientInfo::iterator it = clients_pending_on_restart_.begin();
        it != clients_pending_on_restart_.end(); ) {
     width = std::max(width, it->second.width);
     height = std::max(height, it->second.height);
     clients_[it->first] = it->second;
     clients_pending_on_restart_.erase(it++);
   }
-  capture_format_.width = width;
-  capture_format_.height = height;
-  DVLOG(1) << "RestartCapture, " << capture_format_.width << ", "
-           << capture_format_.height;
+  requested_params_.width = width;
+  requested_params_.height = height;
+  DVLOG(1) << "RestartCapture, " << requested_params_.width << ", "
+           << requested_params_.height;
   StartCaptureInternal();
 }
 
 void VideoCaptureImpl::StartCaptureInternal() {
   DCHECK(capture_message_loop_proxy_->BelongsToCurrentThread());
   DCHECK(device_id_);
 
-  media::VideoCaptureParams capability_as_params_copy;
-  capability_as_params_copy.width = capture_format_.width;
-  capability_as_params_copy.height = capture_format_.height;
-  capability_as_params_copy.frame_rate = capture_format_.frame_rate;
-  capability_as_params_copy.session_id = capture_format_.session_id;
-  capability_as_params_copy.frame_size_type = capture_format_.frame_size_type;
-  Send(new VideoCaptureHostMsg_Start(device_id_, capability_as_params_copy));
+  Send(new VideoCaptureHostMsg_Start(device_id_, requested_params_));
   state_ = VIDEO_CAPTURE_STATE_STARTED;
 }
 
 void VideoCaptureImpl::AddDelegateOnIOThread() {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
   message_filter_->AddDelegate(this);
 }
 
 void VideoCaptureImpl::RemoveDelegateOnIOThread(base::Closure task) {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
@@ skipping 17 matching lines @@
   if (it != clients->end()) {
     handler->OnStopped(this);
     handler->OnRemoved(this);
     clients->erase(it);
     found = true;
   }
   return found;
 }
 
 }  // namespace content