getUserMeida: report device starting states

media/capture/video/video_capture_device.h

 // 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.
 //
 // VideoCaptureDevice is the abstract base class for realizing video capture
 // device support in Chromium. It provides the interface for OS dependent
 // implementations.
 // The class is created and functions are invoked on a thread owned by
 // VideoCaptureManager. Capturing is done on other threads, depending on the OS
 // specific implementation.
@@ skipping 196 matching lines @@
     // be StopAndDeAllocate()-ed. |reason| is a text description of the error.
     virtual void OnError(const tracked_objects::Location& from_here,
                          const std::string& reason) = 0;
 
     // VideoCaptureDevice requests the |message| to be logged.
     virtual void OnLog(const std::string& message) {}
 
     // Returns the current buffer pool utilization, in the range 0.0 (no buffers
     // are in use by producers or consumers) to 1.0 (all buffers are in use).
     virtual double GetBufferPoolUtilization() const = 0;
+
+    // VideoCaptureDevice reports it's successfully started.
+    // This method will be called either synchronously or asynchronously by any
+    // thread. This is OK because it will be posted to IO thread and handled
+    // there by state machines.
+    virtual void OnStarted() = 0;

[chfremer, 2017/02/14 19:21:23]

A couple of recommendations here:

1.) I recommend removing the sentence "This is OK because ...".
Reason: 
The description for this method is supposed to define a contract between 
implementations of VideoCaptureDevice and implementations of 
VideoCaptureDevice::Client. This contract should be independent of information 
regarding what implementations we happen to use downstream and how they behave 
(e.g. VideoCaptureImpl). It should only state what guarantees it makes. 

2.) When stating that it may be invoked synchronously, let us make it clear 
where the synchronous call would originate from, see example below.

3.) As part of the contract, we should specify whether or not we guarantee that 
OnStarted() gets called before any video frame is sent. Silently relying on 
VideoCaptureImpl being used downstream and assuming it (and everything in 
between) correctly handles the case where video frames are sent before 
OnStarted() gets called is dangerous, because even if it may be true today this 
can easily change and cause things to break in unexpected ways.

4.) Allowing video frames to get sent before OnStarted() makes the contract much
more complicated than if we would guarantee that OnStarted() is always called 
before any frame is sent. It would also raise the question if it can happen that
the OnStarted() call arrives concurrently with the delivery of a frame. 
I highly recommend going for the simpler contract.

Example:
VideoCaptureDevice reports it's successfully started.
This method may be called either synchronously from an invocation of 
VideoCaptureDevice::AllocateAndStart() or asynchronously from any thread after 
AllocateAndStart() has returned. It is guaranteed to get called before any 
video frames are delivered.

[braveyao, 2017/02/15 21:47:16]

Done.

Basically, there is hundreds of milliseconds delay after we start the capture
device to get the first captured frame. Plus the existing state machine in VCI,
it won't be a problem if we call OnStarted synchronously.
So here only Video/Sceen capture on Android and Video capture on Linux/CrOS,
which will call OnStarted asynchronously, need to be taken care of. The simple
process is to set the capture flag after OnStarted event and forward capture
frame only after the flag is set.

[chfremer, 2017/02/16 01:00:50]

I looked at the device implementations and had some questions regarding
potential race conditions. Please look at my other comments.

Note, that for this it does not matter how many milliseconds of delay there
typically is.

[braveyao, 2017/02/17 20:37:21]

Acknowledged.
From this point of view, it is the thing good to have.
I went through all the device implementations. Please have a look to the changes
there.

   };
 
   ~VideoCaptureDevice() override;
 
   // Prepares the video capturer for use. StopAndDeAllocate() must be called
   // before the object is deleted.
   virtual void AllocateAndStart(const VideoCaptureParams& params,
                                 std::unique_ptr<Client> client) = 0;
 
   // In cases where the video capturer self-pauses (e.g., a screen capturer
@@ skipping 75 matching lines @@
 
  private:
   // Gets the power line frequency from the current system time zone if this is
   // defined, otherwise returns 0.
   PowerLineFrequency GetPowerLineFrequencyForLocation() const;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_H_